Fixes#17033
Fixes#17050
This one's hell, and has revelead three different bugs:
Bug 1
commit in an each could happen long after the original context of the block effect is gone. That meansget_boundary() inrun might not get the correct active effect, maybe it's evennull at that moment. To fix it Ithink we need to capture then restore the context. Loads of tests failed with that approach, so I added another capture-restore within the commit function itself but there's still one test failing - still trying to find out why.
Bug 2
Each blocks get out of order because of race conditions and block effects not rescheduling when they should. Reproducible by clicking fast three times on the button in the reproduction of#17033. This is what happens:
- run batch 1
- run batch 2
- run batch
- batch 1 completes, wants to rebase batch 2/3.
- Does update and run batch 2 first.
That reruns each block because each blocks rely on the newer values (because of proxy each entry is a separate source).
Since each'sarray method is not time traveling it's always the value of whatever batch ran last. Rerun also causes reinit of Circle.svelte because each logic destroys the old one - Does NOT run batch 3 because no overlap according to our "depends on distinct values" logic (since each block just reran and now has more dependencies on it so false negative).
- batch 3 Circle.run completes, decrement 0, runs commit
- Does NOT rebase batch 2 because of same reasons as 4.
- batch 2 Circle.run completes, decrement 0, runs commit -> wrong end result
The reason for this is that block effects can change dependencies after a rerun and so a subsequent batch rebase could have a false positive or negative effect reschedule. To fix it we need to split up the rebase logic into two stages: First collect all effects of all batches that should rerun, then run them in a second loop.
Bug 3
each state is not properly time-travel-ready. Something goes wrong for keyed each blocks in#17033 still. The problem is that fine grained proxy means some array entries may appear undefined for subsequent runs when they shouldn't, but no further insights yet and no idea how to fix yet.
Before submitting the PR, please make sure you do the following
Tests and linting
Uh oh!
There was an error while loading.Please reload this page.
Fixes#17033
Fixes#17050
This one's hell, and has revelead three different bugs:
Bug 1
commitin an each could happen long after the original context of the block effect is gone. That meansget_boundary()inrunmight not get the correct active effect, maybe it's evennullat that moment. To fix it Ithink we need to capture then restore the context. Loads of tests failed with that approach, so I added another capture-restore within the commit function itself but there's still one test failing - still trying to find out why.Bug 2
Each blocks get out of order because of race conditions and block effects not rescheduling when they should. Reproducible by clicking fast three times on the button in the reproduction of#17033. This is what happens:
That reruns each block because each blocks rely on the newer values (because of proxy each entry is a separate source).
Since each's
arraymethod is not time traveling it's always the value of whatever batch ran last. Rerun also causes reinit of Circle.svelte because each logic destroys the old oneThe reason for this is that block effects can change dependencies after a rerun and so a subsequent batch rebase could have a false positive or negative effect reschedule. To fix it we need to split up the rebase logic into two stages: First collect all effects of all batches that should rerun, then run them in a second loop.
Bug 3
eachstate is not properly time-travel-ready. Something goes wrong for keyed each blocks in#17033 still. The problem is that fine grained proxy means some array entries may appear undefined for subsequent runs when they shouldn't, but no further insights yet and no idea how to fix yet.Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint