-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Fix/@sveltejs/svelte/16114 #16263
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
base: main
Are you sure you want to change the base?
Fix/@sveltejs/svelte/16114 #16263
Conversation
…rectly assigned sources when reading in teardown functions
🦋 Changeset detectedLatest commit: b213de2 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 |
|
Thank you. Unfortunately this isn't the right fix, since it depends on when the derived was last calculated — you can see that in this example:
In fact I think this is just #16262 in a slightly different form. There's a mechanism in #15844 that would fix this — |
Fixes: #16114
When a
$state
or a$derived
is directly set by the user, we properly add it to theold_values
Map
.However when a derived is updated as a result of the
check_dirtiness
check duringflush_queued_effects
, we do not save the old value, hence the value read in a teardown function of a$effect
is not the old value but the new one, this is incosistent with the "old value behavior" that was recently introduced.In this PR i save the old value when the
$derived
is updated, so that it will be used by teardown functions of$effect
s.However while working on this i've noticed that there's an edge case when working with child components.
While passing a prop in certain conditions (either a legacy bindable or a prop accessed via
$.prop
because it has a setter somewhere in the code) the original$state
is wrapped in aderived
. When the child component is destroyed the teardown functions might read$.prop
values that did not got update because thay were not read (perhaps in the template). This means that is possible that during the execution of the teardown the values of the derived are eithernull
because they were never initialized because nothing ever read from it, or they are outdated. I've opened a separate issue for this: #16262.This bug is present both before and after the changes of this PR.
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 test
and lint the project withpnpm lint