-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Infinite recursion in $effect
#15568
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
I suspect that this is the expected behavior because you are reading and writing the same state in an effect, which should generally be avoided. You can prevent import { untrack } from 'svelte';
or function setY(newX){
const newValue = { a: 1 };
y = newValue;
console.log(newValue);
}; |
Correct, this is expected...thanks for reporting tho |
@brunnerh Thanks for you reply, and I agree, your explanation does make logical sense. But to me, turning a benign function into an infinite-loop-footgun just by adding a |
Effects should be used very sparingly and calling functions from them is inherently risky if their contents interact with state or do something outside your control. When setting one state in response to another within an |
I understand that, but let me explain the actual use case where I encountered this problem: I'm writing a date picker for incomplete dates, which may only consist of year, year and month, or year, month and day. The value passed to it needs to be parsed and converted from a string into its internal representation (an array of Int). Now, based on how many parts are present, the picker needs to display years, months, or days. So the process is roughly like this: let { value } = $props();
let internalValue = $state([0, 0, 0]);
let mode = $state('year');
$effect(() => parseValue(value));
function parseValue(){
internalValue = // ... parse string to array of ints
mode = modeFromInternalValue(internalValue);
}
function modeFromInternalValue(internalValue){
return isYear(internalValue) ? 'year' : 'month';
} This causes the infinite loop I reported. To me this code seems like the straight-forward way of updating the two So the question is this: Should Svelte be able to handle code like this (which IMHO is rather basic) gracefully, without looping infinitely, or should we as developers learn work-arounds for the quirks of the reactivity system. And if sticking to work-arounds is the answer, then at least better error messages would be useful. |
There's a PR open to allow writable deriveds #15570 but for the moment you can create state in deriveds to allow for a writable derived. const writable_der = $derived.by(()=>{
let ret = $state({ current: my_prop });
return ret;
});
writable_der.current = "blah"; Assigning state in effect is never a good idea, and even if you don't want to use this solution i would suggest wrapping the state in a setter so that you can update both states. |
@paoloricciuti Thanks, I'll have to think about how to apply your advice, and I'll keep an eye on that PR. |
@Rich-Harris This issue seems a lot like what #15553 is addressing to me, or am I misunderstanding the goal of the PR? In this issue there is also an "unsafe read" that doesn't modify any state but still causes an infinite loop. I tried running the REPL against 5.25.6 and it still loops, so the question is, is this the same kind of problem you were fixing and should the PR have fixed this issue as well? |
That PR is about state created within an |
@elpres |
Describe the bug
The effect in the following code is looping indefinitely:
Converting the first line in the function to
y.a = 1
fixes the problem. Commenting out the second line (the one withconsole.log
) also does.Reproduction
REPL
Logs
System Info
Severity
blocking all usage of svelte
The text was updated successfully, but these errors were encountered: