-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Svelte 5: $effect is unusable (produce circular dependencies and endless updates) #9944
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
Would something like this work? |
I had same idea - but as you see it dosn't work (now). edit: see invalid values not filtered out |
Reposting what I said in Discord: You are mutating state in an effect that reruns every time that state changes, of course it'll provide infinite loops. Similar to how a recursive function needs a stopgap (I forgot the technical term), these types of effects need a check to determine if they should run again or not (reassign again, in this case). I used the lightweight lib dequal for this, although you could implement an equality check yourself if you wanted to. |
The other issue is passing in I'd even consider breaking out this logic further – having the filtering of the array in the parent that uses a common utility function that can do this for you. Then you likely won't need so much indirection at all. The trick here is to not think of these patterns with a Svelte 4 mind, but to look at the problem differently given the new primitives you have in Svelte 5. |
@trueadm - thank you, but the functionality of your solution is different and filtering is moved to parent. @TGlide, you solved it perfectly - I've tried this for DAYS with v5! But this issue point it out, that $effect ist HARD to understand and the result is not intuitive expectable. I post some part of the code to see, what we need to do today: v4:
equivalent in v5 (updated & simplified):
|
@dm-de We don't think The things you're stumbling across here are more likely related to how things are done with Svelte 4 in regards to bound props. My point I was trying to make is that in scenarios like this, it's actually simpler to redesign the parent -> child relationship in terms of their dataflow. Simplifying it like I showed above can make it easier to debug and reason with. Granted that this isn't how things worked in Svelte 4, but that's actually a design flaw in Svelte 4 and something we've been actively trying to tackle. For the cases where you need to have to do this, we offer If you use $effect(() => {
if (disabled || !value) value = []
});
$effect(() => {
value = filtered_items.filter(
item => !item.disabled && untrack(() => value).includes(item.value)
).map(
item => item.value
)
}) As shown here. |
@trueadm The main idea was to have a checkbox group that bind always valid values (and correct if necessary) and do not delegate this job to parent. Let's say you use 10 times this checkbox group everywhere - you will repeat it every time. It is simular, if you have a date picker and only valid date-ranges are allowed. Should your parent do this job (please no!) or your date picker? This is a real world example from my code - and I struggled with it a lot. I believe, if $effect stay as it is (without error/warning from compiler) - this will be a unpredictable nightmare in Svelte 5. |
@dm-de I posted the old URL, my bad. Check it again :) |
@trueadm you see disabled checkboxes (they can not be valid) and invalid value: |
Ah okay. I mistook the bug there, this should work better then? That has got me wondering if there's something we can do here to guide folks towards this path or another better one. Leave it with me. |
No... you see... this is try and error - it is not expectable Even changing last line from working (TGlide) code in $effect produce different results: |
@dm-de If you de-couple the effects it's fine. I was avoiding the untrack here. I updated the link. |
Ha ha - this is a beast! it do not work |
That's because |
It's not unpredictable. The effect runs anytime a variable inside it changes. Value is changed inside the effect, but it is also read from inside it, so it is quite clear why an infinite loop happens. Having automatic untracks or similar approaches are not predictable. We've seen this with the current approach to state in Svelte (screenshot below). ![]() |
Nonetheless, I'll try and make it simpler. But just because something has more LOC, doesn't mean its worse, especially when we get the whole context behind these decisions. |
This is, IMO, simpler than the Svelte 4 example, easier to reason about (as in Svelte 4, it's not clear why the reactive block does not run infinitely), safer, while maintaining almost the same amount of LOC. Comparison: // Svelte 4
$: if (disabled || !value) value = []
$: value = items2.filter(
item => !item.disabled && value.includes(item.value)
).map(
item => item.value
) // Svelte 5
const is_valid_key = (key) => !!filtered_items.find(i => i.value === key && !i.disabled)
$effect(() => {
if (value?.length === 0) return
if (disabled || !value) return value = []
if (value.some(v => !is_valid_key(v))) {
value = value.filter(is_valid_key)
}
}) If you exclude the start and end of the effect ( |
ufff.... I thought first But without this line, it will not work. |
It's a dependency tracking issue. If there were no read of With this logic, first the |
This would be one of those use cases for a property change hook which I mentioned elsewhere. The point of this logic is to ensure a consistent internal component state, when properties are changed from the outside. There is no way to make this distinction and thus an If there were an external change hook, which would not be an effect but just an event, you should be able to simply use the Svelte 4 logic: onPropChange(() => {
if (disabled || !value)
value = [];
value = items
.filter(i => !i.disabled && value.includes(i.value))
.map(i => i.value);
}); (Could be more granular if the handler is supplied information about what property was changed.) |
I found out another working solution, which is more like original v4 code. important part from this is:
but i'm still not happy with it. Bad part is that even reading state can cause loops: |
This is my expected code, that should run - but it's wrong:
Value is watched in this $effect - and set at the same time = endless loop. Here are ideas to solve this in Svelte:
This feels BAD
|
See my previous comment. |
Reading though this, I must agree with OP. While nothing is logically wrong with My concerns isn't so much about the right or wrongs of Svelte has always brought in programmers at all different experiences because "it just works", simple logic like "when a value changes do this" and it working is what has brought joy to programmers of all experiences. I worry that adding this complexity would be a net negative to Svelte for DX. I personally worry about people seeing these changes happening to appease enterprise programmers while forgetting about the hobbyist. |
The problem is that the static analysis that the reactivity system relied on previously was catching everyone off-guard, beginner and seasoned programmers alike. Reactive blocks would not run at times, or the order would that they are stated would drastically alter the output. There are several issues others have raised in GH that get fixed with runes (e.g. #6732). Sure, compared to something that tried and "magically" do things for you, you're now forced to come up with more responsibility, and it may feel odd. But it's not out of the blue, things weren't working out as intended, reactivity was quite leaky at times. |
I want to share more information, about behavior in some other frameworks for comparison. Summary: This is an identical, but "useless" example to test endless loops. Svelte 4: Svelte 5: React:
Vue: Solid-JS: |
@dm-de One important point you're missing though. The loop "prevention" can be actively harmful. |
I find that the use cases where $effect in the current state is beneficial, are very limited. In svelte 4, the same result can be achieved with a setTimeout which in my opinion is much less of a hassle than having to write additional checks to avoid loops. I already loathe the time where I will upgrade all my apps to runes and be greeted by infinite loops. |
These are rare codes that do this. There are currently 8 effects in my library. My other effect code - I naively assumed it was analogous to Svelte 4 - also triggered endless loops for me. There I had simply reset an object to default. Like this:
|
For the simple use case I demonstrated, that may work. But this is about predictability, which is important to ensure your code is reliable. The Also, what if it gets more complicated, with multiple setTimeouts? EDIT: Of course these effects are wonky. They're just to show that a setTimeout yields different results. |
After looking into the issue, I think I did something similar to the obj before in my previous codes in React. The component was probably running in the loop. However, I never noticed that it updated itself. If an error occurs (like in Svelte 5 or SolidJS), then I find that better than executing a loop unnoticed. Even better if the compiler recognizes it and warns you. Later... we need a collection of pitfalls in the docs, |
That is true, setTimeout is not a true alternative. Maybe instead we could have another variant of $effect which follows the naming scheme as $effect.pre, In that case we won`t need the debate anymore. |
That could be quite the API pollution IMO. It would be super confusing to have two separate effects functions that do almost the same thing. At least active and root serve a different, and pre works the same, just happens at a different point in time. Still, maybe it could be valuable. Up to the maintainers :) |
You can avoid using |
The problem with relying on the getter, is that the state doesn't actually change, so if you go, say, from config 1 to config 2 and back again, To produce the same results you'd have to do something like this |
Not really a problem with getter, but just with me understanding the requirements 😄 It looks like your version uses getters and works just fine! |
That's fair! 😄 Yeah, I said to mean that the bulk of the calculation can't be only on the getter, but you're right that it depends on the requirements. Nonetheless it's a great example. |
I'm just curious - sorry to jump in here - so feel free to tell me to go away ;) But relating to the reactive class example Is there a reason to use getters over exposed derived (other than the fact that it allows set() customisation)? I.e. rather than get items() {
return (this.#items ?? []).filter(i => !i.hidden && i.value)
} have this: items = $derived(this.#items ?? []).filter(i => !i.hidden && i.value) |
After playing around with runes for a bit, I am also a bit confused. It looks super weird to me that let uuid = $state('');
$effect(() => {
uuid = Math.random().toString(36).substring(7);
if (uuid.includes("x")) console.log("X included")
}); Sure, I could go with $effect(() => {
dependency1;
dependency2;
dependency3;
untrack(() => {
// complicated code goes here
})
}) I would assume that in most cases you would like to not have infinite update loops and wouldn't it be easy to detect those cases: If the Am I missing something or am I the only one thinking this is a bit clunky? |
|
@dummdidumm... Can you please post an example how you can solve described problem with $derived? |
Here it is with |
Not sure if this post is still active, the $effect should behave the same way as |
I think it's too late to change anything here. It was once an attempt to point out that $effect is completely different. But apparently the new function is intended that way. I have changed my code so that the inner processing is not reflected upwards, but the path goes from top to bottom. After reading the documentation, it becomes clear what $effect is supposed to do. It should not change the state, it should change more e.g. HTML code. |
Uh oh!
There was an error while loading. Please reload this page.
Describe the bug
This is a serious issue with $effect - it produces a lot of problems.
See working example Svelte 4 and broken Svelte 5 version.
It is impossible for me to so solve it with v5.
In Checks.svelte you will find a commented out $effect code, that I expect to work. But it does not work.
Then i added another $effect code below - that works, but not fully.
I must use untrack - but it makes no sense here. Otherwise you will get endless iterations.
The main problem seems to be with
value.includes()
.With this half working code, you can see 3 issues in REPL.
Additionally:
one line is still commented out - I can not get it working.
//if (!value) { value = []; return } //STILL DON'T WORK!!!
It is REALLY hard to use $effect as it is (specially compared with
$:
in v4).It needs to be improved, otherwise it will frustrate many people.
Reproduction
Svelte 4:
https://svelte.dev/repl/9c8fbbe0824e4f4598a6c32e85119483?version=4.2.8
Svelte 5:
LINK
Logs
No response
System Info
Severity
blocking all usage of svelte
The text was updated successfully, but these errors were encountered: