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

Inconsistent ownership warning #15488

Open
benmccann opened this issue Mar 10, 2025 · 5 comments
Open

Inconsistent ownership warning #15488

benmccann opened this issue Mar 10, 2025 · 5 comments
Labels

Comments

@benmccann
Copy link
Member

benmccann commented Mar 10, 2025

Discussed in #15481

Originally posted by midzelis March 9, 2025
Is this a bug or inconsistent behavior?
https://svelte.dev/playground/3057c1db35e745f39d4a73061a9c1e22?version=5.22.6

When passing a class with $state to a component with bindable, if the component binds to a nested property, an ownership failure is logged. However, if the component directly binds to a $state variable then no ownership failure is logged. Playground link shows bug by default. Open console under result in playground to see console logs.

In playground, the rendering/output still worked - however, in my complex real world app, it did not work as expected.

Reply from @brunnerh:

This looks like a bug to me. The warning should appear in both cases because the demo property is not actually bound (setting $bindable is just a prerequisite to that).

@benmccann benmccann added the bug label Mar 10, 2025
@paoloricciuti
Copy link
Member

Mmm interesting...the problem arise because we only check the ownership in proxies...so the reason why the $state(0) change doesn't trigger an invalid mutation warning is because $state(0) is not proxified.

@dummdidumm
Copy link
Member

This is arguably the correct behavior:

  • declaring something public on a class means that people can reassign it from anywhere as it's considered public API
  • you don't need to explicitly bind in that case because things go through the class, it's not a POJO that's mutated
  • on the other hand, if you mutate the state of the class, you're now mutating something you probably shouldn't, hence the warning

One could argue it also shouldn't warn on the class mutation because it's on the class, but that might be hard to achieve.

@huntabyte
Copy link
Member

This seems related to: #13607 as well right?

@brunnerh
Copy link
Member

declaring something public on a class means that people can reassign it from anywhere as it's considered public API

This does not make sense to me. Properties on plain objects can also be considered their public API, I don't see why properties on classes should be treated differently in that regard. An object is being modified without binding.

@benmccann
Copy link
Member Author

if you mutate the state of the class, you're now mutating something you probably shouldn't, hence the warning

How can we make that assumption?

One could argue it also shouldn't warn on the class mutation because it's on the class, but that might be hard to achieve.

I'm not sure where the warning lives, but perhaps we could skip the warning whenever we see a mutation happening to a something containing a . in the name?

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

No branches or pull requests

5 participants