-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Reactive declaration on a store, caused by a change in another store from another component is not called #5365
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
Comments
Though: This appears to imply that Svelte will wait for the next process? |
A more visual REPL demonstrating the issue |
The ordering inside the compiled <script>
let amount = 2
let price = 1.99;
let total = 0
$: formattedTotal = total.toFixed(2)
$: calculate(amount) // $: calculate doesn't work when placed below the $: formattedTotal
function calculate(n) {
total = price * n
console.log('calculated '+ total)
}
</script>
{price} x <input type="number" bind:value={amount} /> = {formattedTotal} This makes the reactivity system unreliable, i'll see if I can fix the issue or provide a meaningful warning message. |
Hi Monsieur Bob, thanks for your reply, but, I may have been mistaken, is this not a bug at all and instead how svelte handles the micro-processes? PS I was just reading a stack-overflow answer on Svelte, from last month regarding #key and I was thinking this sounds weird but awesome, I'm hoping it was written by an authority, and hey, it was you! |
Definitely a bug. when you write: I do expect resistance to a solution. That will need some iterations to get right. |
Another repro in the REPL: As you can see, the moment a function is defined reactive with a parameter, it does not trigger any subsequent reactive updates. While executing the function the moment the initial reactive variable is changed does trickle down all subsequent changes. |
@bfanger Maybe change the title of this issue to reflect the actual bug? |
@guidobouman I did not report the issue, I did figure out why it happens and created an RFC which outlines a possible solution. |
Both the REPL by @guidobouman and the issue outlined in the RFC by @bfanger are actually expected behavior given how reactive statements currently work. The key is that the code is analyzed beforehand and only the values immediately part of the declaration are taken into account for ordering. This is explained in more detail in this issue and this section of the community docs. The latter also explains why this behavior actually is desirable for some people because you might want to hide specific values from reactivity tracking. |
@dummdidumm This does not seem like intended behavior. Especially when it starts working the moment you change the order of the reactive declarations. |
This at the moment is the intended behavior, because as described in the issue linked above, you hide the variable from the ordering algorithm because it's inside the function. You need to make it a direct dependency of the |
For my understanding: Then where does the If the compiler adds the
|
The |
Okay, I hoped that the PS: The RFC explains the issue nicely:
|
Uhmm... It gets reordered anyhow, right? This would not change that. Actually, I kinda agree with the request from #5905. But combined with deeper dependency tracking from this issue. The one thing I want for in every platform / framework / codebase is predictability. How easy is it to explain it to a newcomer without resorting to caveats. Magic sorting is quite a caveat. Especially when it goes against the line by line execution known to nearly all programming languages, including JavaScript. If you build invisible reactivity, it needs to work in every case. It seems reasonable to want to shield code from reactivity but still react on some variables. A function could still provide that. Could this maybe be achieved if you only track variables in functions that are referenced in the actual reactive statements? |
I also just ran in this "bug"... you can check an oversimplified REPL of my code (I use a custom store in real code) : https://svelte.dev/repl/cb8853b5fe804b9a84417704ba85f6db?version=3.38.3 I feel this is pretty unpredictable and create bugs that are hard to catch if the component is semi-complicated. In this case, I don't want to shield code from reactivity, I want to react when the value of a store changes, regardless of the order. |
It works as expected if you replace |
I used .set in my example for simplicity sake but the real code has a custom store and I don't expose .set. I only expose some state changing functions. So in this case |
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. |
I think this issue is still relevant. |
This will be working as expected in Svelte 5. Dependencies are tracked at runtime now, and with the |
Describe the bug
ComponentA setting the value of a store (call it store1), which calls a function in ComponentB as it is dependent on this store. The function in componentB sets the value of another store (call it store2) and has a reactive declaration for store2 (just a console log, or anything else for that matter), the reactive declaration is NOT called.
This appears to be because the initial trigger is from ComponentA. If I update store2 from ComponentB, it calls the reactive declaration on store2.
To Reproduce
Please click here:
https://svelte.dev/repl/f51251e28e1a42d292de15b1a6079476?version=3.24.1
Top.svelte sets setCategory (type the value in the text field and hit SetCat and it updates it). Annoying.svelte listens for this and calls on onSetCat, which console logs (to make sure it's running) and then sets setSearch, which should show an alert, but it does not (only shows on page load, when the value is set from stores.js. There is a button in Annoying.svelte, called try locally, and this DOES trigger the reactive declaration. I've included commented code for a traditional subscribe function, to show it does not work with this either.
Information about your Svelte project:
Version 85.0.4183.83 (Official Build) (64-bit)
Windows 10
Reproduced on repl which I believe is the latest?
Severity
Umm going to be honest, I like this bug, because it actually makes my live search function work a bit better, but hey, I'm an honest guy.
The text was updated successfully, but these errors were encountered: