-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
fix: recurse into $derived
for ownership validation
#15166
Conversation
🦋 Changeset detectedLatest commit: 67dc579 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 |
|
- `$derived` can contain `$state` declarations so we cannot ignore them, so this reverts #14533 - instead, we add equality checks to not do this expensive work unnecessarily - this also adds a render effect similar to the class ownership addition when it detects a getter on a POJO during ownership addition fixes #15164
e35f074
to
f0bb9c5
Compare
Thanks for the quick fix. Could you have a look at this case as well? |
Mhm, honestly I think this is a case which we cannot solve. But also, if you're passing along a thunk, you may also pass an update function. |
packages/svelte/tests/runtime-runes/samples/non-local-mutation-with-binding-8/main.svelte
Outdated
Show resolved
Hide resolved
Did this PR go stale? |
No - I just didn't see Rich approved it; thanks for the reminder 😄 |
@dummdidumm |
Nevermind, sorry for pinging, needed to clear my node_modules/vite cache. |
$derived
can contain$state
declarations so we cannot ignore them, so this reverts fix: don't try to add owners to non-$state
class fields #14533fixes #15164
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