Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify computed system #1267

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Simplify computed system #1267

wants to merge 6 commits into from

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented May 16, 2024

This PR is working to:

  • remove async computeds, because we think we can cover their use case with explicit relationships instead and it will greatly simplify the computed system
  • refactor the computed internals to work more like glimmer/tracking, such that arbitrary consumption of tracked fields causes automatic reactivity, no matter how "far apart" via arbitrary JS the producer and consumer are.

It's unused, and one of only two async computeds that exist.
Copy link

github-actions bot commented May 16, 2024

Host Tests Test Results

    1 files  ±    0      1 suites  ±0   7m 40s ⏱️ - 6m 30s
270 tests  - 346  229 ✔️  - 377  6 💤  - 4    1 +  1  34 🔥 +34 
270 runs   - 346  195 ✔️  - 411  6 💤  - 4  35 +35  34 🔥 +34 

For more details on these failures and errors, see this check.

Results for commit 2e5dff9. ± Comparison against base commit 12995c4.

This pull request removes 347 and adds 1 tests. Note that renamed tests count towards both.
Chrome 125.0 ‑ Integration | Component | RoomMessage: it does not show an error when last streaming chunk is still within reasonable time limit
Chrome 125.0 ‑ Integration | Component | RoomMessage: it shows an error when AI bot message streaming timeouts
Chrome 125.0 ‑ Integration | card-basics > cards allowed to be edited: catalog entry isField indicates if the catalog entry is a field card
Chrome 125.0 ‑ Integration | card-basics > cards allowed to be edited: catalog entry isField indicates if the catalog entry references a card descended from FieldDef
Chrome 125.0 ‑ Integration | computeds: can indirectly render an asynchronous computed field
Chrome 125.0 ‑ Integration | computeds: can maintain data consistency for async computed fields
Chrome 125.0 ‑ Integration | computeds: can render a async computed that depends on an async computed: consumer field is first
Chrome 125.0 ‑ Integration | computeds: can render a nested asynchronous computed field
Chrome 125.0 ‑ Integration | computeds: can render an asynchronous computed composite field
Chrome 125.0 ‑ Integration | computeds: can render an asynchronous computed field
…
Chrome 125.0 ‑ Global error: Uncaught Error: attempted to set read-only computed field created at http://localhost:7357/assets/chunk.c25b25c4bc5b3fd95551.js, line 561933
 While executing test: Integration | operator-mode > matrix: it maintains status of apply buttons during a session when switching between rooms

♻️ This comment has been updated with latest results.

@ef4 ef4 force-pushed the cs-6858-sync-computed branch from 123cf42 to b0fb6da Compare May 20, 2024 17:39
ef4 and others added 2 commits May 20, 2024 15:57
Copy link

Host Test Results

    1 files  ±  0      1 suites  ±0   13m 20s ⏱️ -14s
599 tests  - 38  552 ✔️  -   83  1 💤  - 1    1 +  1  45 🔥 +45 
603 runs   - 39  511 ✔️  - 129  1 💤  - 1  46 +46  45 🔥 +45 

For more details on these failures and errors, see this check.

Results for commit 4a0c5c0. ± Comparison against base commit 5716b6e.

This pull request removes 39 and adds 1 tests. Note that renamed tests count towards both.
Chrome 126.0 ‑ Integration | ai-assistant-panel: assures applied state displayed as a check mark even eventId in command payload is undefined
Chrome 126.0 ‑ Integration | ai-assistant-panel: button states only apply to a single button in a chat room
Chrome 126.0 ‑ Integration | ai-assistant-panel: can close past-sessions list on outside click
Chrome 126.0 ‑ Integration | ai-assistant-panel: displays message slightly muted when it is being sent
Chrome 126.0 ‑ Integration | ai-assistant-panel: displays retry button for message that failed to send
Chrome 126.0 ‑ Integration | ai-assistant-panel: it animates the sessions dropdown button when there are other sessions that have activity which was not seen by the user yet
Chrome 126.0 ‑ Integration | ai-assistant-panel: it can apply change to a linksTo field
Chrome 126.0 ‑ Integration | ai-assistant-panel: it can apply change to nested contains field
Chrome 126.0 ‑ Integration | ai-assistant-panel: it can handle an error during room creation
Chrome 126.0 ‑ Integration | ai-assistant-panel: it can handle an error in a card attached to a matrix message
…
Chrome 126.0 ‑ Global error: Uncaught Error: attempted to set read-only computed field created at http://localhost:7357/assets/chunk.ef0eb5edd6e7f294e317.js, line 564400  While executing test: Integration | ai-assistant-panel: it maintains status of apply buttons during a session when switching between rooms 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants