Skip to content

Unexpected reactive statement call #5731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ganigeorgiev opened this issue Nov 28, 2020 · 4 comments
Closed

Unexpected reactive statement call #5731

ganigeorgiev opened this issue Nov 28, 2020 · 4 comments

Comments

@ganigeorgiev
Copy link

ganigeorgiev commented Nov 28, 2020

Describe the bug
There is an unexpected reactive statement call (aka. $: ) on variable assignment in other conditional reactive statement. This is happening only when there is an assignment inside reactive statement that is expected to be triggered if an exported object prop changes (which is also checked in another reactive statement).

To Reproduce
Please check the following repl - https://svelte.dev/repl/58570a9e05a240f591a76b4eeab09598?version=3.30.0
It consists of 4 different scenarios inside 4 separate components. Only TEST 1 fails (check the console output).

Expected behavior
I expect the first reactive statement to not be triggered by unrelated assignments in another reactive statement.
Or at least, please provide an explanation why is behaving like that.

Information about your Svelte project:

  • Fedora 33
  • Chrome 87.0.4280.66 (Official Build) (64-bit)
  • Svelte ^3.30.0
  • Rollup ^2.33.3

Severity
The bug is not easily detected and it took me a while before I was able to isolate it. I've already refactored several of my components by using the approach similar to TEST 4 from the repl.

@PaulMaly
Copy link
Contributor

PaulMaly commented Dec 4, 2020

Hi, thanks for the report!

Actually, it's really tricky in there. I'm not sure that it works correctly, but I can explain why it's happening in all those cases. Definitely, I agree with you what it's a little bit confusing in general, and maybe would be nice to improve it somehow.

At the moment, the reason for this behavior in 3 things:

  1. model is a prop
  2. model is an object
  3. model invalidating in onchange handler like this:
const change_handler = e => {
  if (e.target.checked) {
    selected.push(1);
  } else {
    selected.splice(0, 1);
  }

  ($$invalidate(0, selected), $$invalidate(1, model));
};

I'm not sure about the third thing. Is this a bug or a feature? But it is. The quick fix for Test 1 just turn the immutable option in true:

<svelte:options immutable={true} />

Btw, I always advise any guys who ask that it's better to use immutable=true in your projects. Svelte optimize only a DOM updates for complex types which can't be safely checked by !==. But reactive statements are not optimized an can be triggered superfluous times. To fix that, you can enable an option above and always change the reference to the object after assignments.

Test 1 - Step by step guide:

Because model is a prop it could be changed at any time so we should handle this case. Because model is an object when it invalidating it always marking as dirty if immutable=false. Because model is invalidating in onchange handler (we don't know why) all reactive statements depending on it will be triggered.

Test 2 - Step by step guide:

Because model is NOT a prop here and there's not any other place in the component where it could be changed, compiler won't generate a code for this. Actually, in this example reactive statements won't be generated as reactive because there's no cases they should be triggered again. So, both reactive statements will be performed only once.

Test 3 - Step by step guide:

This example looks the same as Test 1, but with the key difference - model is NOT an object, but a primitive value. So, in spite of the fact that it still invalidating in onchange handler for some reason, it won't trigger a reactive statement because the value not actually changed and the inequality check fails.

Test 4 - Step by step guide:

Using this approach, you just hide selected from dependencies of reactive statement and, for some reason, model stops invalidating in onchange handler:

const change_handler = e => {
  if (e.target.checked) {
    selected.push(1);
  } else {
    selected.splice(0, 1);
  }

  $$invalidate(0, selected);
};

>>> So, the only question for all these cases - should the model be invalidated in onchange handler or not. <<<

Can anyone help us with this question please: @Rich-Harris @Conduitry @tanhauhau

UPDATE:

I believe it related to 2694

UPDATE 2:

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.

@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Jun 26, 2021
@stale stale bot removed the stale-bot label Jun 26, 2021
@stale stale bot removed the stale-bot label Jun 27, 2021
@ganigeorgiev
Copy link
Author

The bug still persists with the latest version - https://svelte.dev/repl/58570a9e05a240f591a76b4eeab09598?version=3.38.3

@tanhauhau
Copy link
Member

it's the same root issue as reported in #4933, i'm going to close this instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants