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

False positive ownership_invalid_binding? #13607

Open
huntabyte opened this issue Oct 15, 2024 · 17 comments
Open

False positive ownership_invalid_binding? #13607

huntabyte opened this issue Oct 15, 2024 · 17 comments
Labels
awaiting submitter needs a reproduction, or clarification bug
Milestone

Comments

@huntabyte
Copy link
Member

huntabyte commented Oct 15, 2024

Describe the bug

I'm getting the following message, telling me to consider bind: between the two components. However, I am, in fact, binding between the two.

CleanShot 2024-10-14 at 22 02 50@2x

Reproduction

https://svelte.dev/playground/c27c1016f1f2423489568d9fb2271921?version=5.2.5

Logs

[svelte] ownership_invalid_bindingsrc/lib/calendar-with-select.svelte passed a value to node_modules/.pnpm/[email protected][email protected]/node_modules/bits-ui/dist/bits/calendar/components/calendar.svelte with `bind:`, but the value is owned by node_modules/.pnpm/[email protected][email protected]/node_modules/bits-ui/dist/bits/calendar/components/calendar.svelte. Consider creating a binding between node_modules/.pnpm/[email protected][email protected]/node_modules/bits-ui/dist/bits/calendar/components/calendar.svelte and src/lib/calendar-with-select.svelte

System Info

System:
    OS: macOS 15.0
    CPU: (12) arm64 Apple M2 Max
    Memory: 92.23 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.15.1 - ~/.nvm/versions/node/v20.15.1/bin/node
    npm: 10.7.0 - ~/.nvm/versions/node/v20.15.1/bin/npm
    pnpm: 9.6.0 - ~/Library/pnpm/pnpm
    bun: 1.0.25 - ~/.bun/bin/bun
  Browsers:
    Edge: 129.0.2792.89
    Safari: 18.0
  npmPackages:
    svelte: ^5.0.0-next.1 => 5.0.0-next.265

Severity

annoyance

@trueadm trueadm added the bug label Oct 15, 2024
@trueadm trueadm added this to the 5.x milestone Oct 15, 2024
@paoloricciuti
Copy link
Member

Cloning the repo locally i can't seem to reproduce...what are the reproduction steps?

@paoloricciuti paoloricciuti added the awaiting submitter needs a reproduction, or clarification label Oct 15, 2024
@someone2080
Copy link

someone2080 commented Oct 17, 2024

This one is probably a duplicate of 13423

@someone2080
Copy link

So, I was able to reproduce it, but I'm not sure if it's the same reproduction as in this issue.

Here's a REPL

This happens when state is stored in a separate file from where it was declared/instantiated. I hope REPL will make it clear enough.

I wonder if it's actually a 'legit' svelte coding pattern...

@huntabyte
Copy link
Member Author

Thanks for the REPL repro @someone2080 ! That's exactly when it appears to be happening when it shouldn't.

@huntabyte
Copy link
Member Author

Hey team, are there any plans to allow users to shut some of these warnings up?

I understand they only run during DEV, but they can still be quite annoying when they don't impact the functionality. I know we can ignore the compiler warnings via the svelte.config.js, but I don't see a way to ignore these runtime logs.

@coryvirok
Copy link
Contributor

So, I was able to reproduce it, but I'm not sure if it's the same reproduction as in this issue.

Here's a REPL

This happens when state is stored in a separate file from where it was declared/instantiated. I hope REPL will make it clear enough.

I wonder if it's actually a 'legit' svelte coding pattern...

Isn't one of the intended benefits of Svelte 5 runes that you can store state in any non-svelte component and use it elsewhere? If this isn't the intended behavior, what would be the correct way to distribute components that take state as props and modify them?

@someone2080
Copy link

@coryvirok Yes, you are right. I'm using this method, too. Runes in this mode are very powerful; they remind me of SwiftUI state management. The issue here is that this also opened a rabbit hole.

I'm not a JS expert, but this warning was meant to prevent us from doing crazy things like in this REPL, we can do this thanks to the fact that JS objects are passed by reference.

Now, Svelte team has to find a way to track down where and how this Rune was declared and issue a valid warning, which to me looks like a huge lift.

I wonder if it's possible to remove this restriction altogether and use $bindable only for simple types if necessary (kinda crazy idea though).

@kpwelsh
Copy link

kpwelsh commented Dec 17, 2024

@someone2080

I am seeing the same issue with what I think is a reasonable design pattern. I have 2 relevant components: 1 that produces something and 1 that modifies it.
Here are two slightly different repros:

I thought the "svelte" way to do this would be to bind variables through all of the components. I should be able to fix this with some callbacks or a moderate restructure of the data, but it seemed to me like the bind solution should work.

Edit:
Here is a repro where all of the definitions are located in the same file: https://svelte.dev/playground/8787f944e5d54146b4f9d65999ac089f?version=5.14.2.

Its not clear to me how to have this kind of behavior without seeing this warning.

@someone2080
Copy link

@kpwelsh Yeah, that looks pretty reasonable. It's a false positive.
Regarding callbacks, I use them on a regular basis instead of bindings, and for state management, I use controllers(MVVM/MVC) in the form of classes (in .svelte.ts), so I don't have problems with this warning anymore.

@krabodyan
Copy link

i am using Tauri and getting this bug even when using this example from svelte playground. For some reason, there is no this warnings in firefox 134.0 and chromium 131.0.

Image

@paoloricciuti
Copy link
Member

i am using Tauri and getting this bug even when using this example from svelte playground. For some reason, there is no this warnings in firefox 134.0 and chromium 131.0.

Image

Are you sure you are literally only using that example? I think we need an actual reproduction

@krabodyan
Copy link

i am using Tauri and getting this bug even when using this example from svelte playground. For some reason, there is no this warnings in firefox 134.0 and chromium 131.0.
Image

Are you sure you are literally only using that example? I think we need an actual reproduction

I rechecked it and it happens only in tauri when the code is hot reloaded. If you change any code fragment and open the console after hot-reloading, you will get these warnings. And it seems there are no problems with a full restart. It seems to have something to do with tauri, because I can't get this behavior in browsers

@F2
Copy link

F2 commented Jan 17, 2025

This has nothing to do with Tauri.

It is a general svelte issue when you bind to an object, and then perform a hot reload on the component that has the bindable props.

I have made a minimal repro here: https://github.com/F2/svelte-bug-ownership_invalid_mutation

In the video here, you can see that entering text into the textbox does not throw any warnings. But after a hot reload, it starts throwing warnings.

svelte-bug.mp4

@pekeler
Copy link

pekeler commented Jan 20, 2025

I'm also struggling with false ownership_invalid_mutation warnings after HMR. In my case, they only show in Safari's dev console, not in Chrome or FF.

@Metehan-Altuntekin
Copy link

Happens to me too. I have created state in a component and chained it down with bind: for two levels. It shows that warning only on dev server, not on the preview server.

@AndreasHald
Copy link

I'm getting this too, In my case it's because I have 3 components using bind, in some cases I wan't to bind to selectedValue on grandParent, in other cases I do not wan't to. Is this bad practice? the code runs as I would expect.

<!-- +page.svelte -->

<script>
   import GrandParent from '...';
</script>

<GrandParent />
<!-- GrandParent.svelte -->

<script>
import Parent from '...';
let {
    selectedValue = $bindable()
} = $props();

</script>

<Parent bind:selectedValue />
<!-- Parent.svelte -->
<script>
import Child from '...';
let {
    selectedValue = $bindable()
} = $props();

</script>

<Child bind:selectedValue />
<!-- Child -->
<script>
let {
    selectedValue = $bindable()
} = $props();

</script>

<!--  Do something & mutate selectedValue -->

@huntabyte
Copy link
Member Author

You all can ignore this message if you wish by putting the following above the component that is doing the "invalid binding"

<!-- svelte-ignore ownership_invalid_binding -->
<Something bind:somethingElse />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification bug
Projects
None yet
Development

No branches or pull requests