-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix: better binding interop between runes/non-runes components #12123
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
…s correctly. This is done by also using the `prop(..)` variant for a property if it's mutated in runes mode, and then figuring out at runtime whether or not the parent should be notified or not fixes #12032
🦋 Changeset detectedLatest commit: 5415cc5 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 |
IIUC in the mutation case we're always calling the prop function with the value of calling itself: stuff(stuff().blah += 1, stuff()); Given that the prop already knows its own value, would it be simpler to do this? stuff(stuff().blah += 1, true); Then the function returned from return function (/** @type {any} */ value, mutation) {
if (arguments.length > 0) {
// We don't want to notify if the value was mutated and the parent is in runes mode.
// In that case the state proxy (if it exists) should take care of the notification.
// If the parent is not in runes mode, we need to notify on mutation, too, that the prop
// has changed because the parent will not be able to detect the change otherwise.
if (!runes || !mutation || legacy_parent) {
// upon reassignment only the first argument is used
/** @type {Function} */ (setter)(mutation ? getter() : value);
}
return value;
}
return getter();
}; |
good catch, simplified |
…ejs#12123) * Ensure binding from legacy component passed to runes component updates correctly. This is done by also using the `prop(..)` variant for a property if it's mutated in runes mode, and then figuring out at runtime whether or not the parent should be notified or not fixes sveltejs#12032 * fix adjacent bug around wrong value getting return upon mutation * deduplicate * changeset * simplify * move comment * rename --------- Co-authored-by: Rich Harris <[email protected]>
Ensure binding from legacy component passed to runes component updates correctly. This is done by also using the
prop(..)
variant for a property if it's mutated in runes mode, and then figuring out at runtime whether or not the parent should be notified or notfixes #12032
Also:
console.log(foo.bar = true)
did not logtrue
rest
onlet { foo, ...rest } = $props()
contained private properties like$$slots
etcprop(..)
prop" into a common functionBefore submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint