Skip to content
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: widen ownership when sub state is assigned to new state #11217

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

dummdidumm
Copy link
Member

Ownership was not widened when assigning a sub state to a different top level state. The set of owners for the state was zero because the owner was on the original parent, but that one was reset to null because it's now the top level of a different state. That meant that there was no owner but also no parent to check for the owner, which is an invalid combination resulting in a nullpointer (and also potentially false positive warnings in other situations).

fixes #11204

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Ownership was not widened when assigning a sub state to a different top level state. The set of owners for the state was zero because the owner was on the original parent, but that one was reset to null because it's now the top level of a different state. That meant that there was no owner but also no parent to check for the owner, which is an invalid combination resulting in a nullpointer (and also potentially false positive warnings in other situations).

fixes #11204
Copy link

changeset-bot bot commented Apr 18, 2024

🦋 Changeset detected

Latest commit: 77d83ea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

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

@dummdidumm dummdidumm merged commit 4b59ef3 into main Apr 18, 2024
8 checks passed
@dummdidumm dummdidumm deleted the ownership-widen-fix branch April 18, 2024 08:00
@chainlist
Copy link

I have retested with the RPL following your changes @dummdidumm.

No errors anymore but just a warning. And I don't really know if this type of warning is really relevant here (in the context of the REPL).

Should passing a rune that stores an object to another component and in that component editing a property of that object trigger a warning? It seems to me like that's something that is commonly done, no?

Maybe I am misunderstanding something.

@Rich-Harris
Copy link
Member

Should passing a rune that stores an object to another component and in that component editing a property of that object trigger a warning?

Yes, that's the whole point of the warning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Svelte 5: TypeError: null is not an object (evaluating 'metadata.parent') when assigning value to $bindable
3 participants