sveltejs / svelte

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

Svelte 5: `$:` generates wrong effect dependencies causing infinite loops. #9954

Closed brunnerh closed 4 months ago

brunnerh commented 8 months ago

Describe the bug

When setting a property on an object in a reactive statement, the object becomes a dependency. This can easily cause infinite loop issues if multiple such statements exist, breaking compatibility with v4.

(Found by @kitschpatrol, relevant to svelte-tweakpane-ui)

Reproduction

<script>
    let button = { title: '', label: '' };
    let title = '';
    let label = '';

    title = label = ''; // to add dependencies/generate update block

    $: button.title = title;
    $: button.label = label;
</script>

REPL

Generated code:

$.pre_effect(() => {
    ($.get(button), $.get(title));
    $.untrack(() => {
        $.mutate(button, $.get(button).title = $.get(title));
    });
});
$.pre_effect(() => {
    ($.get(button), $.get(label));
    $.untrack(() => {
        $.mutate(button, $.get(button).label = $.get(label));
    });
});

Note the button dependency that is being added.


Same code in v4: REPL

$$self.$$.update = () => {
    if ($$self.$$.dirty & /*title*/ 1) {
        $: button.title = title;
    }
    if ($$self.$$.dirty & /*label*/ 2) {
        $: button.label = label;
    }
};

Logs

Uncaught Error: ERR_SVELTE_TOO_MANY_UPDATES: Maximum update depth exceeded. This can happen when a reactive block or effect repeatedly sets a new value. Svelte limits the number of nested updates to prevent infinite loops.
    at infinite_loop_guard (playground:output:445:10)
    at flush_queued_effects (playground:output:462:4)
    at process_microtask (playground:output:489:3)

System Info

REPL - v5.0.0-next.26

Severity

blocking an upgrade

kitschpatrol commented 8 months ago

Thanks for this fix!

The simple test case is resolved by #10128, but I'm still seeing infinite loops in my library against 5.0.0-next.31. It seems like the loops are coming from existence checks in reactive statements.

Here's a slightly more complex test case with an existence check to only update the object's members if the object itself is defined:

<script>
    // Button object is initially undefined
    let button;
    let title = '';
    let label = '';

    // Simulate button becoming defined
    // (In practice, this would be be set elsewhere, e.g by a wrapped vanilla JS library)
    button = { label: 'foo', title: 'bar' };

    // Reactive statements need to check for button's existence before setting keys
    // works fine in Svelte 4, `ERR_SVELTE_TOO_MANY_UPDATES` in Svelte 5
    $: button && (button.title = title);
    $: button && (button.label = label);
    $: console.log(button);
</script>

Svelte 5 REPL vs Svelte 4 REPL

It seems like maybe button is firing a reactive event on the object when its members change, even if the identity of the object itself is unchanged.

dummdidumm commented 4 months ago

Given REPL now works.