fix: re-process batch if new root effects were scheduled
#17895
Closing issue
Describe the bug
A store getter that lazily writes $state on first read deadlocks the scheduler when read inside a template binding during the flush/commit phase. The UI freezes permanently — no errors, no stack overflow, no crash.
Works in: 5.53.7
Broken in: 5.53.8, 5.53.9
Likely cause: #17805 "simplify scheduling logic"
Reproduction
https://github.com/vdg/svelte-5.53.8-scheduler-deadlock
git clone https://github.com/vdg/svelte-5.53.8-scheduler-deadlock
cd svelte-5.53.8-scheduler-deadlock
npm install
npm run dev
- Click "Open panel"
- Click "Counter" — counter does not update, UI is frozen
- No console errors
Change to [email protected] → works fine.
The pattern
A module store with a getter that writes $state on first read:
let panelWidth = $state(null)
export default {
get panelWidth () {
if (panelWidth === null) panelWidth = DEFAULT_WIDTH // writes $state
return panelWidth
}
}
Read by a template binding:
<div style:width={store.panelWidth ? `${store.panelWidth}px` : null}>
What happens
- User action triggers a flush
- During commit, the
style:widthbinding readsstore.panelWidthfor the first time - The getter writes
$state(lazy init) - In 5.53.8+, this stalls the scheduler — no further updates are processed
Workaround
Avoid writing $state inside getters:
get panelWidth () {
return panelWidth ?? DEFAULT_WIDTH // no state write
}
Severity
Silent — no errors or warnings. The page appears frozen but JS is still running. Very hard to diagnose without knowing to look for this specific pattern.
Pull request
In some cases a new branch might create effects which via reading/writing reschedule an effect, causing this.#roots to become populated again. In this case we need to re-process the batch. Most of the time this will just result in a cleanup of the dirtied branches since other work is already handled via running the effects etc. - it's still crucial, else the reactive graph becomes frozen since no new root effects are scheduled.
Fixes #17891
Info
🦋 Changeset detected
Latest commit: 123ff93
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| svelte | Patch |
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
pnpm add https://pkg.pr.new/svelte@17895This isn't right either — if a new batch has already been created,
(honestly, setting state during render like this is a really bad idea. maybe we should forbid it in v6?)
(honestly, setting state during render like this is a really bad idea. maybe we should forbid it in v6?)
In general yes, sure. However this specific pattern is very useful and we also rely on it a lot. Personally I think that setting state as long as the result is idempotent should be fine with the framework. If we think in terms of end developer using the framework what harm does it do?
In our case we use it for get-or-default functionality on our caches (read SvelteMaps) which helps us prepopulate cache lazily with a default value once the item is requested. Doing it on read is very handy, because it requires writing code only in one place.
#14784 could help us get there - e.g. we get more strict but you can use unsafe in cases where you know what you're doing. Solid 2.0 will have something related to mark sources as pure: https://github.com/solidjs/solid/blob/next/documentation/solid-2.0/01-reactivity-batching-effects.md#no-writes-under-owned-scope
#14784 could help us get there - e.g. we get more strict but you can use
unsafein cases where you know what you're doing. Solid 2.0 will have something related to mark sources as pure: https://github.com/solidjs/solid/blob/next/documentation/solid-2.0/01-reactivity-batching-effects.md#no-writes-under-owned-scope
Yeah, as long as there is at least some way to do it, should be fine. Currently we are wrapping all such cases in untrack
If we think there are valid use cases (and it sounds like there are, even if they weren't represented in the test suite!) and we can make it work without unleashing Zalgo, then unsafe is probably pointless hectoring. I feel pretty good about the fix in this PR
In some cases a new branch might create effects which via reading/writing reschedule an effect, causing `this.#roots` to become populated again. In this case we need to re-process the batch. Most of the time this will just result in a cleanup of the dirtied branches since other work is already handled via running the effects etc. - it's still crucial, else the reactive graph becomes frozen since no new root effects are scheduled. Fixes #17891
Pro tip: You can prefix GitHub URLs of issues, PRs or discussions with svcl.dev/ to view them on this page! Also try it on a GitHub release ;)