-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix: don't set derived values during time traveling #17163
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
Conversation
|
|
|
This looks like it may have the same cause as #17111. |
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Rich-Harris
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
|
ah crap, forgot a changeset and have to run out now. will fix later if no-one else does |
|
Here we go #17200 |
Just a failing test because I tried to fix this without much success. This also causes SvelteKit to fail when preloading happens (I though #17147 would've fixed it, but the problem is with blocks that use deriveds).
The issue is that we update
derived.vand increase the version even if the state change happens in afork. However, this means that if in a later state update (be it in another fork or just normal state update) to determine if the block effect is dirty we try to update the derived and then check thewvof it. But since we already updated the value AND the version in the fork the block effect is not executed.IMHO we should reserve the same treatment we reserve to sources for deriveds and update the value in the batch and not in the derived itself but when I tried to do that a bunch of tests failed.
EDIT: oh my god I think I actually figured it out! I just pushed a commit to fix the issue, I'm just not sure if it's the right solution.
Basically, if
batch_valuesis defined we don't setderived.v, we just increase the version. Now what this means is that if we are changing the derived AND we are not in a tracking context we kinda wasted one computation because the next time around the derived will be re-computed (during the get) but if this time we are in a tracking contextbatch_valueswill be updated and thegetwill read that (the rest of the logic applies as usual withbatch_valuesso when we commit the value is the updated one).That said, there might be an issue when accessing a derived in a non tracking context when
batch_valuesis defined, but I wasn't able to think of a situation for that. Can you?Fixes sveltejs/kit#14827
Fixes #17190
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