fix: properly add owners to function bindings #14960
Closed
+93
−11
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #14956
The ownership stuff was specifically guarded against the
SequenceExpression
so i'm not sure if this is has any downside but for the moment let's open the PR so we can eventually discuss. I've added the owner to every stateful identifier in theget
function only since that's the only part the child component could mutate in some way (i wonder if we should actually just consider the identifiers returned from the function instead of every identifier).Also also i discovered that we were adding the same identifier multiple times in situations like this
there were two
add_owner_effect
's added which is wasteful. I did that fix in a separate commit so if we decide against this change we can always merge revert it and merge the fix for multiple effects separately.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