Reactive statement re-run even though dependency (exported prop) didn't change

[TylerRick]

Describe the bug

See my repro (based on actual problem I ran into):
https://svelte.dev/repl/ff6e69e975df44f3821cc4ed956881f8

<script>
  export let filters;

  export function fetchItems(filters) {
    return [{name: 'B'}, {name: 'A'}]
  }

  let items = []
  $: console.log('filters changed?', filters)
  $: {
    items = fetchItems(filters)
    console.log('re-fetched, overwriting sorted array with unsorted array')
  }
  $: console.log('items is now:', items.map(el => el.name))  

  function handleSort() {
    items.sort((a,b) => -1)
    console.log('items is now sorted:', items.map(el => el.name))
    items = items
  }
</script>

{#each items as item}
  <ol>
    <li>{item.name}</li>
  </ol>
{/each}

<button on:click={handleSort}>Sort!</button>

Expected behavior: When handleSort is called, it should sort items, and render the updated list with [A, B]. Because the $: items = fetchItems(filters) block depends only on filters and filters has not changed, it should not be re-run.

Actual behavior: When handleSort updates items = items, it re-runs the $: items = fetchItems(filters) block (why???), causing an extra unnecessary fetch, and worse, causing the results of the sort to be lost and replaced with unsorted list, [B, A].

You can see from the console that after it sorts the items, it erroneously triggers both of these to re-run, even though filters has not — and could not have — changed.

  $: console.log('filters changed?', filters)
  $: items = fetchItems(filters)

Similar issues

Looks similar to Test 1 from https://svelte.dev/repl/58570a9e05a240f591a76b4eeab09598?version=3.46.1 (#5731).

Just like in that issue, I only expect my reactive statement to be triggered (re-fetch data based on filters) if the exported filters prop changes.

But unlike in that issue, where it was put forth (but not confirmed that this was the reason) that

Seems, Svelte sees these two statements together and decide that model depending on selected assignment.

$: if (model) { <-- this 
  selected = []; <-- and this in the same expression
}

After that, when Svelte finds any selected assignments it will invalidates the model as well.

, I didn't see anything quite the same in my example.

---

Looks similar to #7045 . As in that case, if I run my repro on Svelte 3.2.0 (https://svelte.dev/repl/ff6e69e975df44f3821cc4ed956881f8?version=3.2.0) pre-#2444, it works as intended.

---

And of course it looks similar to #4933, which seems to be the canonical issue for all bugs like this... but is also too broad and confusing and possibly out-of-date (the linked-to REPL doesn't actually reproduce any bug). I'm guessing my issue would fall under Condition 1 of #4933?

---

Is the issue like @Rich-Harris said in https://stackoverflow.com/questions/61730267/reactive-statements-triggered-too-often, that

Objects are treated differently from primitives meaning Svelte will assume that they could have been mutated

So I'm guessing somehow it invalidates "too much" because it pessimistically assumes that filters may have changed? —

Even though it seems abundantly clear from any possible static analysis of my repro code that filters cannot have changed simply by triggering handleSort. The only way for filters to have changed is if the exported prop changed, and it didn't. (So how do I prevent it from thinking it did?)

[TylerRick]

And in the meantime, what would be best/simplest/most concise way to work around this (get my REPL to work as intended)? I can't just not export that prop. And simply throwing a <svelte:options immutable={true}/> at the top didn't seem to fix my particular issue. Thanks. I'm guessing somebody will say "stores".

[TylerRick]

Here is a workaround using stores, but I don't love it because it destroys the beautiful one-line simplicity that is supposed to be reactive statements. Does anyone have a cleaner solution?

Instead of this 1-liner:

  $: items = fetchItems(filters)

, we now have this boilerplate-y eyesore:

  const filtersStore = storeChanges(deepclone(filters))
  $: filtersStore.set(filters)
  const unsubscribe = filtersStore.subscribe((filters) => {
    console.log('re-fetched; filters changed:', filters)
    items = fetchItems(filters)
  })
  onDestroy(unsubscribe);	

It seems like we ought to at least be able to make the main statement a one-liner still using auto-subscribe:

  $: items = fetchItems($filtersStore)

but for reasons I don't understand, this exhibits the original bug, and gets re-run even when $filterStore hasn't changed its value. Why???

[robertadamsonsmith]

And in the meantime, what would be best/simplest/most concise way to work around this (get my REPL to work as intended)? I can't just not export that prop. And simply throwing a <svelte:options immutable={true}/> at the top didn't seem to fix my particular issue. Thanks. I'm guessing somebody will say "stores".

The immutable option isn't working as you intended, because you are mutating the array instead of creating a new one. Changing line 26 create a new array makes it work correctly.

items = [...items]

Strangely, that change also makes everything work correctly, even when immutable={false}.

Back to the main issue. I think that it would be good if some of the svelte reactivity which is designed to handle more complex cases was made not the default behaviour, until it is made to work in a more predictable way (I think it is great that work is and has been done to handle more advanced types of reactive behaviour, but it isn't quite there yet and so I think it causes more problems than it solves for now)

[TylerRick]

The immutable option isn't working as you intended, because you are mutating the array instead of creating a new one. Changing line 26 create a new array makes it work correctly.

items = [...items]

Strangely, that change also makes everything work correctly, even when immutable={false}.

Ah, thanks for pointing that out! I guess I don't understand the immutable option very well. (It would help if the docs explained some of these things. 😄)

I thought the rule was that items = items (any assignment) was always supposed to be enough to trigger reactive statements that use the variable as a dependency. So I sure wouldn't expect this to be treated any differently:

items = [...items]

What about that makes it work? Is it specifically looking for assignments that would change the LHS — so any assignment with a RHS other than the variable name by itself? If so, that seems wrong and breaks the expectations set by the docs.

this workaround with immutable={true}

REPL

While that change did fix the "reactive statement called when it shouldn't be", it also unfortunately made it so it didn't get called when it should be (didn't re-fetch when filters was changed). (Press Sort, then change "Sort by". It should reset to "B, A")

this workaround with immutable={false}

REPL

This variation works perfectly! Which, I agree, is strange, because I don't understand why.

[7nik]

For some reason, Svelte thinks that filters get changed in the handleSort though it even isn't used in the function:

// original
function handleSort() {
  items.sort((a,b) => -1)
  console.log('items is now sorted:', items.map(el => el.name))
  items = items
}
// compiled
function handleSort() {
  items.sort((a, b) => -1);
  console.log('items is now sorted:', items.map(el => el.name));
  ($$invalidate(0, items), $$invalidate(2, filters));
}

And thus it triggers re-fetching.

It's definitely a bug.

[robertadamsonsmith]

Ah, thanks for pointing that out! I guess I don't understand the immutable option very well. (It would help if the docs explained some of these things. 😄)

I thought the rule was that items = items (any assignment) was always supposed to be enough to trigger reactive statements that use the variable as a dependency. So I sure wouldn't expect this to be treated any differently:

items = [...items]

What about that makes it work? Is it specifically looking for assignments that would change the LHS — so any assignment with a RHS other than the variable name by itself? If so, that seems wrong and breaks the expectations set by the docs.

Immutability, means that you are stating that no object (or array) will be altered. If you need part of an object/array to be altered, you will instead create a new object/array. The compiled code then only needs to check if an object's identity has changed to know if it has been altered (rather than having to check the object/arrays contents). This also means that if you do alter (mutate) part of an object or array, the code will act as though nothing has altered.

this workaround with immutable={true}

REPL

While that change did fix the "reactive statement called when it shouldn't be", it also unfortunately made it so it didn't get called when it should be (didn't re-fetch when filters was changed). (Press Sort, then change "Sort by". It should reset to "B, A")

This is because you are mutating the 'filters' object (via the bind:value={filters.sort} ), while passing the 'filters' object to your List component. Because the identity of the 'filters' object doesn't change, the List assumes that nothing has changed and no update is required.

this workaround with immutable={false}

REPL

This variation works perfectly! Which, I agree, is strange, because I don't understand why.

I think that is made more confusing by the misunderstanding over the immutable option. There is a strange behaviour here over how reactivity is being handled, but it doesn't really have anything to do with the immutable option (I think!)

[7nik]

I've simplified the example: https://svelte.dev/repl/c1855cfddccf42b288ae550a06f6eb81?version=3.46.1
Clicking the second button causes execution of the reactive block though it must not.

[robertadamsonsmith]

Obviously deleting line 14 makes it work fine, but changing it to this also works:

let foo = arr2;
arr2 = foo;

Alternatively, you can fix it by changing:

$: {
	arr2 = arr1;
	console.log("arr1 assigned to arr2");
}

to:

function set_arr2(v){arr2 = v}
$: {
	set_arr2(arr1);
	console.log("arr1 assigned to arr2");
}

Poking around in the compiler, I think that this is to do with how 'reactive_declarations' are determined and then used in '/compiler/compile/Component.ts' and '/compiler/compile/render_dom/index.ts' respectively. There is quite a lot of stuff going on in those files, so it might be worth putting in a load of console logging there to see exactly what is happening. I think that "$:{arr2=arr1}" means that arr2 is dependent on arr1, and that "arr2 = arr2" is triggering some reactive logic that can cause a invalidated variable to have its dependencies invalidated in turn, the idea being that since we know that "arr2=arr1", when we invalidate arr2 we must really mean that something it is based on has changed, and so if we invalidate those dependencies instead, then arr2 will be recalculated correctly. I am far from certain that I understand what is going on correctly though, so I could well have this all back to front.

Hopefully this behaviour is clear to somebody else, and if not then I'll investigate further.

[robertadamsonsmith]

Ok, this is strange. Commenting out line 10, so that reset1 does nothing, means that reset2 then works as expected. Even if the "Reset arr1" button is deleted, it seems that the mere existence of a function that could cause arr1 to be invalidated causes the unexpected behaviour. Note that this is only the case if arr1 and arr2 are arrays of objects - simple variables don't seem to be effected by this.

[7nik]

Replacing a variable in the reactive block with its setter or getter fixes the problem.
Removing reset1() also fixes.
Replacing self-assignment in the reset2() with almost anything also fixes. E.g. arr2 = arr2 || arr2, arr2 = (arr2, arr2), arr2 = getArr2();
Removing arr2 from outputting to the page also fixes it.

For primitive types, the problem remains but here Svelte checks that the value indeed was changed and if it didn't, Svelte doesn't execute reactive blocks and other stuff.

[7nik]

Duplicate of #4933

[dummdidumm]

Closing as duplicate of #4933 - Svelte 5 will fix this.