sveltejs / svelte

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

svelte 5 : with reactive set `has` invokes in effects where it shouldn't #11515

Closed FoHoOV closed 3 months ago

FoHoOV commented 4 months ago

Describe the bug

With ReactiveSet everything is fine grained, for instance doing this:

<script>
    import {Set} from "svelte/reactivity";  

    const data = [1,2];
    const reactiveSet = new Set(data);

    $effect(() => {
        console.log("has 1", reactiveSet.has(1));
    });

    $effect(() => {
        console.log("has 2", reactiveSet.has(2));
    });

</script>

<button onclick={()=>{
    reactiveSet.delete(2);
}}>
    delete 2
</button>

<button onclick={()=>{
    reactiveSet.add(2);
}}>
    add 2
</button>

If you press the "delete 2" button, it only logs "has 2: false". But if the value is not there to begin with?

<script>
    import {Set} from "svelte/reactivity";  

    const data = [1,2];
    const reactiveSet = new Set(data);

    $effect(() => {
        console.log("has 1", reactiveSet.has(1));
    });

    $effect(() => {
        console.log("has 2", reactiveSet.has(2));
    });

    $effect(() => {
        console.log("has 3", reactiveSet.has(3));
    });

    $inspect(reactiveSet.has(2));
</script>

<button onclick={()=>{
    reactiveSet.delete(2);
}}>
    delete 2
</button>

<button onclick={()=>{
    reactiveSet.add(2);
}}>
    add 2
</button>

now if you press the "delete 2" button, both "has 2: false" AND "has 3: false" will get logged, it also happens when you add something new.

It might be because of these lines? When a change occures we increase the version, which makes the call to "has 3: false" to be invoked.

Reproduction

1- go this REPL 2- press on "delete 2" button 3- see the logs which also logs "has 3: false" 4- press on "add 2" button 5- see the logs which also logs "has 3: false"

Logs

No response

System Info

REPL (svelte5.next123)

Severity

annoyance

FoHoOV commented 4 months ago

If it doesn't matter can we consider #11504?

Azarattum commented 4 months ago

https://github.com/sveltejs/svelte/pull/11287 also fixes the problem described here.

FoHoOV commented 4 months ago

I want to expand on this issue. see this REPL:

  1. press on delete 2 button - you will see has 2: false and has 3: false as currently mentioned has 3: false shouln't be logged
  2. press on add 3 button, you will see has 2: false and has 3: true- nowhas 2: false` is having the same problem explained earlier.

it seems like tracking version here is wrong.

FoHoOV commented 3 months ago

a better description is provided in #11727