fix: keep dependencies of $state.eager/pending#18108
Conversation
When an eager effect reruns, its version source is destroyed and recreated. That means all dependencies are lost. This breaks when the following is all true: - `$state.eager` is (indirectly) used in a block effect - it is scheduled to rerun - another source (or the same as within `$state.eager`) is (also) dirty and scheduled to rerun in a microtask _before_ `flush_eager` runs ... then `flush_eager` runs the version sources with outdated (or rather, `null`ed) reactions, resulting in nothing being rerun. The fix is to have a stable `version` source across `eager` executions. Fixes #18095
🦋 Changeset detectedLatest commit: 65e035d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
The trade-off here is that in a case like this, where a |
|
Found a way to solve that case by having a Map of branch_effect->version but it also falls down in this case. So yeah the question is how bad is it really. |
|
Is there a reason we can't just use the immediate parent reaction along with a global map? Opened #18218 |
…ach) (#18218) #18108, with two differences: - we use a global map - we use the parent reaction as the key, rather than traversing upwards for a branch I think this has the same outcome? ### Before submitting the PR, please make sure you do the following - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`. - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. - [x] If this PR changes code within `packages/svelte/src`, add a changeset (`npx changeset`). ### Tests and linting - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` --------- Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com> Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
|
closing in favor of #18218 |
When an eager effect reruns, its version source is destroyed and recreated. That means all dependencies are lost. This breaks when the following is all true:
$state.eageris (indirectly) used in a block effect$state.eager) is (also) dirty and scheduled to rerun in a microtask beforeflush_eagerruns... then
flush_eagerruns the version sources with outdated (or rather,nulled) reactions, resulting in nothing being rerun.The fix is to have a stable
versionsource acrosseagerexecutions. Fixes #18095