sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.21k stars 4.09k forks source link

Svelte5: Potential Ownership False Positive 'mutated a value owned...This is strongly discouraged...' #10649

Closed Mali-2 closed 6 months ago

Mali-2 commented 6 months ago

Describe the bug

Hello : ) I believe the below generates a false positive in terms of state ownership and mutation. This occurs in svelte next.68 - it did Not occur in next.61 - When creating an external state 'creator' like this: ~ createMyState.svelte.ts

type MyState = {
     a: number;
};
export type MyStateReturn = {
     myState: MyState;
     inc: () => void;
};

export function createMyState(): MyStateReturn {
     const myState = $state<MyState>({
          a: 0
     });

     function inc() {
          myState.a++;
     }

     return {
          myState,
          inc
     };
}

And using it like this with setContext(): ~ +page.svelte (Parent):

<script lang="ts">
    import Child from '$lib/components/Child.svelte';
    import { setContext } from 'svelte';
    import { createMyState, type MyStateReturn } from '$lib/components/createMyState.svelte';

    const myState: MyStateReturn = createMyState();
    setContext('my_state_context', myState);
</script>

<Child />

~Child:

<script lang="ts">
    import { getContext } from 'svelte';
    import type { MyStateReturn } from './createMyState.svelte';

    const { myState, inc }: MyStateReturn = getContext('my_state_context');
</script>

{myState.a}
<button onclick={() => inc()}>Inc MyState</button>

You get the following console error:

.../lib/components/Child.svelte mutated a value owned by .../src/routes/+page.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.

However, if the createMyState() function is written within the +page.svelte (Parent) directly - Not external - it works fine with no errors.

Reproduction

https://github.com/Mali-2/state

Logs

createMyState.svelte.ts:15 .../lib/components/Child.svelte mutated a value owned by .../src/routes/+page.svelte. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead.
warn    @   client.js?v=01b869fb:2587
check_ownership @   chunk-NEQIOBHS.js?v=01b869fb:159
set @   chunk-NEQIOBHS.js?v=01b869fb:404
(anonymous) @   createMyState.svelte.ts:15
inc @   createMyState.svelte.ts:15
on_click    @   Child.svelte:9
handle_event_propagation    @   chunk-VJRI4WPI.js?v=01b869fb:3198
Show less
createMyState.svelte.ts:15 console.trace
check_ownership @   chunk-NEQIOBHS.js?v=01b869fb:162
set @   chunk-NEQIOBHS.js?v=01b869fb:404
(anonymous) @   createMyState.svelte.ts:15
inc @   createMyState.svelte.ts:15
on_click    @   Child.svelte:9
handle_event_propagation    @   chunk-VJRI4WPI.js?v=01b869fb:3198
Show less

System Info

pnpm -v: 8.7.6
devDependencies:
@sveltejs/adapter-auto 3.1.1            @typescript-eslint/eslint-plugin 7.0.2  eslint-plugin-svelte 2.36.0-next.6      svelte-check 3.6.4                      
@sveltejs/kit 2.5.1                     @typescript-eslint/parser 7.0.2         prettier 3.2.5                          tslib 2.6.2                             
@sveltejs/vite-plugin-svelte 3.0.2      eslint 8.57.0                           prettier-plugin-svelte 3.2.1            typescript 5.3.3                        
@types/eslint 8.56.3                    eslint-config-prettier 9.1.0            svelte 5.0.0-next.68                    vite 5.1.4     
MacOs Sonoma: Version 14.2.1, Macbook Pro M1 
Browser: Chrome Version 121.0.6167.184 (Official Build) (arm64)

Severity

blocking an upgrade

dummdidumm commented 6 months ago

There are two false positives here:

kuechlerm commented 6 months ago

I am not sure what to make out of this. I think, I have the same problem and have a simpler example: REPL - see comment in Field.svelte

@dummdidumm do you mean that this error message is a bug or that I should find another way of solving this setup?

svelte-kit-so-good commented 6 months ago

@kuechlerm yea I think he means it is a bug since he's agreeing that they're false positives. Curiously with your example if you move list in Form.svelte like so:

<script context="module">
+   let list = $state([]);
</script>
<script>
    import { setContext } from 'svelte';
    let { children } = $props();
-   let list = $state([]); 
    setContext('list', list);

</script>

{@render children()}

... then the error message disappears.

pzerelles commented 2 months ago

@dummdidumm The problem comes back currently with HMR, when code is changed on a running page. I use getContext to get reactive state and there is no warning until HMR reloads a file.

dummdidumm commented 2 months ago

Please open a new issue with a minimum reproducible