fix: ensure legacy run utility does not cause cycles #13643
Conversation
🦋 Changeset detectedLatest commit: e2530ca The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| var effect = /** @type {import('#client').Effect} */ (active_effect); | ||
| // If the effect is immediately made dirty again, mark it as maybe dirty to emulate legacy behaviour | ||
| if ((effect.f & DIRTY) !== 0) { | ||
| set_signal_status(effect, MAYBE_DIRTY); |
There was a problem hiding this comment.
Would be cool to warn in dev mode so people don't migrate towards effect and only then run into an infinite loop
There was a problem hiding this comment.
What should the warning say here?
There was a problem hiding this comment.
I added a warning but unsure of the exact messaging. Maybe @Rich-Harris has a clearer idea on the wording.
There was a problem hiding this comment.
message seems fine but it would be cool to include details of where the statement is. see how we do it in e.g. binding_property_non_reactive
There was a problem hiding this comment.
How do we know the name of the reactive signal? It could be a binding, or a local variable or even a proxy signal.
There was a problem hiding this comment.
Ah sorry I got confused — morning brain — thought that this warning applied to $: rather than run in which case we can just pass the location information from the compiler to the runtime. But yeah that doesn't make sense in this context
Fixes #13635. Not sure why the
runfunction didn't already do this given it's surely meant to emulate Svelte 4 behaviour and not have cycles. @dummdidumm does this make sense?