Change detection is overly aggressive where destructuring is used

[Rich-Harris]

Describe the bug

In a reactive declaration like this...

$: a = b(c);

...Svelte correctly wraps the assignment to a in a call to $$invalidate. b and c are treated as dependents (assuming they are themselves reactive), resulting in the following code if c is reactive:

$$self.$$set = $$props => {
  if ('c' in $$props) $$invalidate(1, c = $$props.c);
};

$$self.$$.update = () => {
  if ($$self.$$.dirty & /*c*/ 2) {
    $: $$invalidate(0, a = b(c));
  }
};

But if the assignment is destructured (and there are two or more destructured names)...

$: ({ x, y } = b(c));

...something rather odd happens:

$$self.$$set = $$props => {
  if ('b' in $$props) $$invalidate(2, b = $$props.b);
  if ('c' in $$props) $$invalidate(3, c = $$props.c);
};

$$self.$$.update = () => {
  if ($$self.$$.dirty & /*b, c*/ 12) {
    $: $$invalidate(1, { x, y } = b(c), x, (($$invalidate(0, y), $$invalidate(2, b)), $$invalidate(3, c)));
  }
};

In this case x and y are correctly invalidated, but b and c are as well.

It's easy enough to work around — just don't use destructuring — but it's also a bit of a bad bug. In our case it caused a major performance headache as something intended to run rarely was instead attempting to run at 60fps (which in turn made it more like 10fps).

Reproduction

The simplest repro I've come up with is this one. This more involved one shows a reactive statement re-running unnecessarily as a result of the bug.

Logs

No response

System Info

System:
    OS: macOS 10.15.7
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 8.12 GB / 64.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 16.5.0 - ~/.nvm/versions/node/v16.5.0/bin/node
    Yarn: 1.22.5 - /usr/local/bin/yarn
    npm: 7.19.1 - ~/.nvm/versions/node/v16.5.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.107
    Firefox: 90.0.1
    Safari: 13.1.3
  npmPackages:
    svelte: ^3.38.2 => 3.38.2 (but note that it happens in 3.41.0, per the REPL)

Severity

annoyance

[tanhauhau]

this root cause of this bug is introduced in #2444

which lead to a class of issues: #4933

what happened is that in #2444, where we fixed:

<script>
  let todos = [];
  $: filtered = todos.filter(...);
</script>
{#each filtered as todo}
    <input type="checkbox" bind:checked={todo.done}>
{/each}

{todos.filter(...).length}

whenever we invalidate filtered, we invalidate todos as well, (such that places in need of todos item value are updated, when modifying it through filtered.

NOTE: todos is a reactive dependency of filtered

so how does this relates to this bug?

when we invalidate y, we trigger the similar logic, thus invalidating b and c at the same time.

but why invalidating x or a does not invalidate b and c?

well, we have a bit of duplicated code over here (invalidate and renderer_invalidate), so, they are not going through the same function (which maybe they should, so that this should work )

[dummdidumm]

This comes down to $: standing for two things and it's not easy to distinguish:

• side effects that happen in reaction to something
• derived state that keeps values in sync

Svelte 5 fixes this by separating these into two distinct runes, $effect and $derived. That way it's much clearer what's going on and such edge cases are avoided entirely.
In this case, you would have a $state variable and either update that through the binding of from an $effect, resulting in the desired behavior.