Skip to content

[Svelte 5] False positive firefox ownership_invalid_mutation warning #13139

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

Closed
TUTOR03 opened this issue Sep 5, 2024 · 3 comments · Fixed by #15678
Closed

[Svelte 5] False positive firefox ownership_invalid_mutation warning #13139

TUTOR03 opened this issue Sep 5, 2024 · 3 comments · Fixed by #15678

Comments

@TUTOR03
Copy link

TUTOR03 commented Sep 5, 2024

Describe the bug

Got ownership_invalid_mutation warning on every $state rune update, if update was triggered from parent snippet and $state was created inside child component using function from external lib.

Reproduction

I have described the most detailed reproduction in this repository. There is also a full explanation of the cause of the bug.
Shortened reproduction:

  • Create a lib repo using npm create svelte@latest
  • Export from the repo any .svelte component and function that creates for example onclick handler that updates $state
  • Use this lib in another npm create svelte@latest app repo and make sure is is optimized with vite
  • Create .svelte component that accepts a snippet as a prop and passes return from lib funtion to it
  • Destructur props in snippet to the dom element
  • Click dom element and get warning

lib index.ts

export { default as Dummy } from '$lib/Dummy.svelte';
export * from '$lib/hook.svelte.js';

lib hook.svelte.ts

export function getPropsCreator() {
	const data = $state({ count: 0 });
	function changeData() {
		data.count += 1;
	}

	function createProps() {
		return {
			onclick() {
				changeData();
			}
		};
	}

	return createProps;
}

app +page.svelte

<script lang="ts">
	import Wrapper from '$lib/Wrapper.svelte';
</script>

{#snippet children(props: Record<string, unknown>)}
	<span {...props}>Snippet from App</span>
{/snippet}

<Wrapper {children} />

app Wrapper.svelte

<script lang="ts">
	import type { Snippet } from 'svelte';
	import { getPropsCreator } from 'dummy-lib';
	type Props = {
		children: Snippet<[Record<string, unknown>]>;
	};

	let { children }: Props = $props();

	const createProps = getPropsCreator();
</script>

{@render children(createProps())}

Bug happens due to how (check_ownership)[https://github.com/sveltejs/svelte/blob/main/packages/svelte/src/internal/client/dev/ownership.js#L246] function from svelte source code parses new Error().stack for firefox.
Full explanation in this repo.

Logs

No response

System Info

System:
    OS: Linux 6.10 Fedora Linux 40 (Workstation Edition)
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i7-1370P
    Memory: 18.32 GB / 30.93 GB
    Container: Yes
    Shell: 5.2.26 - /bin/bash
  Binaries:
    Node: 20.16.0 - ~/.nvm/versions/node/v20.16.0/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v20.16.0/bin/yarn
    npm: 10.8.1 - ~/.nvm/versions/node/v20.16.0/bin/npm
    pnpm: 9.8.0 - ~/.local/share/pnpm/pnpm
  Browsers:
    Chrome: 128.0.6613.113
    Mozilla Firefox: 129.0.2
  npmPackages:
    svelte: ^5.0.0-next.243 => 5.0.0-next.243

Severity

annoyance

@jamesst20
Copy link

jamesst20 commented Sep 5, 2024

When possible it's better to provide REPL.

Here is one : REPL

I can confirm this is only happening on firefox (tested with Svelte 5 next 144 + Firefox 130)
Edit: Firefox inspector must be opened prior loading the page otherwise the error never happens

@TUTOR03
Copy link
Author

TUTOR03 commented Sep 5, 2024

@jamesst20 you need to split everything to two separate packages. I thought it is impossible ot reproduce in playground. I am really surprised that you managed to reproduce it in playground

@jamesst20
Copy link

@jamesst20 you need to split everything to two separate packages. I thought it is impossible ot reproduce in playground. I am really surprised that you managed to reproduce it in playground

I though it was weird to require to have 2 separate project. At first I thought maybe it was SvelteKit related but doesn't appear so as it works in REPL. I am no experts in how Svelte works internally so I will let other investigate but it's odd it requires the inspector of Firefox to be opened prior loading the page

Screenshot 2024-09-05 at 9 06 58 AM

dummdidumm added a commit that referenced this issue Apr 3, 2025
Previously we were doing stack-based retrieval of the owner, which while catching more cases was costly (performance-wise) and prone to errors (as shown by many issues over the months).

This drastically simplifies the ownership validation - we now only do simple static analysis to check which props are mutated and wrap them with runtime checks to see if a binding was established.

Besides making the implementation simpler and more performant, this also follows an insight we had over the months: Most people don't really know what to do with this warning when it's shown beyond very simple cases. Either it's not actionable because they don't really know how to fix it or they question if they should at all (in some cases rightfully so). Now that the warning is only shown in simple and easy-to-reason-about cases, it has a much better signal-to-noise-ratio and will hopefully guide people in the right direction early on (learn from the obvious cases to not write spaghetti code in more complex cases).

closes #15532
closes #15210
closes #14893
closes #13607
closes #13139
closes #11861
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 a pull request may close this issue.

2 participants