fix(routing): subscribe roots to broadcasts before any root broadcasts - #65
Merged
Merged
Conversation
`buildRootComponent` subscribed the root machine to the global emitter from a passive effect, and a routing root broadcasts the initial route from one too. Passive effects run in tree order, so whether a root receives the initial route depended on where it sat relative to the routing root: a root rendered before it was subscribed in time, a root rendered after it was not, and silently missed the event. Subscribe during the layout phase instead. Layout effects for a commit all run before any passive effect for that commit, so every root is attached before a passive effect can broadcast, regardless of render order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an order-dependent routing bug in buildRootComponent where non-slotted roots could miss the initial routing broadcast depending on JSX sibling order. It does so by moving the global broadcast subscription from a passive effect to a layout effect, ensuring all roots are subscribed before any passive-effect routing broadcasts run.
Changes:
- Subscribe roots to the global emitter in
useLayoutEffect(instead ofuseEffect) to eliminate sibling-order dependence for initial routing events. - Add a regression test that renders a routing root and a listener root in both sibling orders and asserts the listener receives the initial route event.
- Add an in-code rationale comment explaining the React effect ordering issue being addressed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/xstateTree.tsx |
Moves root broadcast subscription to layout effect to guarantee subscription precedes passive-effect broadcasts (e.g., initial routing). |
src/tests/rootSubscribesBeforeBroadcast.spec.tsx |
Adds a regression test covering both sibling orders to prevent reintroducing the missed-initial-route behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+408
to
+413
| // Layout, not passive. Effects run in tree order, so a root rendered after a | ||
| // routing root would otherwise still be unsubscribed when that routing root | ||
| // broadcasts the initial route from its own (passive) effect, and would miss | ||
| // it entirely. Subscribing during the layout phase gets every root attached | ||
| // before any broadcast a passive effect makes. | ||
| useLayoutEffect(() => { |
|
🎉 This PR is included in version 5.5.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
buildRootComponentsubscribes the root machine to the global emitter from a passive effect:A routing root broadcasts the initial route from a passive effect too (the mount effect that calls
handleLocationChange).Passive effects run in tree order. So whether a root receives the initial routing event depends entirely on where it renders relative to the routing root:
In the first case the routing root's mount effect has already broadcast by the time
ListenerRoot's subscribe effect runs. The event is emitted into an emitter nobody is listening on yet, and it's gone — there's no replay for a non-slotted root.Event delivery silently depending on JSX sibling order is not something callers can reasonably be expected to know about, and it fails in the direction that's hardest to debug: no error, just a machine that never got its route.
The fix
Subscribe during the layout phase.
React runs all layout effects for a commit before any passive effect for that commit, so every root is attached to the emitter before a passive effect can broadcast — regardless of render order. Broadcasting stays where it is; only the subscription moves.
Test
src/tests/rootSubscribesBeforeBroadcast.spec.tsxrenders a routing root and a bare root in both orders and asserts the bare root sees the initial routing event either way.On
masterthe "rendered after" case fails and the "rendered before" case passes — the order dependence, pinned. Both pass with this change.Context
Kawaka has been carrying exactly this change as a local pnpm patch (
common/pnpm-patches/@koordinates__xstate-tree@5.5.1.patch) since May. It was added during an XState v5 migration wave, alongside a spec comment describing a routing event being "lost mid-transition".The commit that introduced it describes the patch as being for "a
TestRoutingContextexport needed by the stories" — that's inaccurate;TestRoutingContextwas already a public export in stock 5.5.1, and the patch has only ever contained this one-line effect change. Upstreaming it so the patch can be dropped.Verification
npx jest— 129 passed, 28 suites, no regressions (three consecutive clean runs)npm run lint -- --fix— 0 errorsnpm run build— cleannpm run api-extractor -- --local— public API surface unchanged