fix(router-core, react-router): keep a departed match readable until its tree unmounts - #8576
Tarekkharsa wants to merge 3 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: TanStack/router/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughRouter stores now retain previous route IDs and the last matches for routes removed from the active match set. React bindings use retained matches when rendering an exiting route tree. ChangesDeparted route match handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change preserves departed-route rendering without relaxing inactive-route errors elsewhere. No actionable merge-blocking issue was established; merge after normal checks pass. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change retains prior route data to support a tree that has not finished unmounting. Active-tree lookups remain restricted, and no security bypass was established. Cancellation and overlapping navigation behavior remain partly unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ates
Server-render a nested route, keep its Suspense boundary dehydrated with a
child that suspends on the client, and click a root Link. On main the
departed route's Match reads a cleared store and throws "Cannot read
properties of undefined (reading 'routeId')".
Also assert that useMatch({ from }) still throws for a route that is no
longer active.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…its tree unmounts
setMatches clears a departed route's store in the same batch that commits
the navigation. When the route's Suspense boundary is still dehydrated,
React hydrates it once with the old children before it applies the
update, so MatchImpl, Outlet and useMatch read undefined.
Keep the previous route ids in stores.previousIds and the last match of
each route that left with that change in stores.departed. MatchImpl and
Outlet fall back to them, and Outlet no longer indexes with -1. useMatch
falls back only when its nearest route has departed, so useMatch({ from })
on an inactive route from a live tree still throws. Remove the now unused
dummyMatchContext.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…click Also document the lifetime of the `departed` and `previousIds` stores. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
5b180dd to
2ee81aa
Compare
🎯 Changes
Fixes #8198. Related: #8306.
Supersedes #8485 (moved from my work account).
Reproducer: https://github.com/Tarekkharsa/tanstack-router-departed-match-repro. On the current release,
pnpm i && pnpm testexits 1.Problem
In an SSR app, the user can click a
<Link>in the header while the route content's Suspense boundary is still dehydrated. This happens often on React 19, because React hydrates that boundary later than the header. React discards the hydration and logs #422/#520, and in trees with nested outlets the recovery render throws again (MatchViewrouteIdTypeError /useMatchinvariant):Root cause
setMatchesclears the store of each route that the user leaves, in the same batch that commits the navigation. Before React unmounts the old tree, it hydrates the dehydrated boundary one time with the old children. In that render:MatchImplgivesundefinedtoMatchView.Outletreads_notFoundfrom the cleared parent store. It also usesids[ids.indexOf(routeId) + 1], andindexOfreturns-1, so it renders the root route inside itself.useMatchfinds no match and throws.Fix
router-core/stores.ts:setMatchessaves the previous ids instores.previousIds. It also keeps the last match of each route that left instores.departed(the same objects, no copies). Both are written in the batch that clears the departed stores and cleared at the next change ofids, so they hold at most one entry per route.react-router/Match.tsx:MatchImpland theOutletparent selector usestores.departedas a fallback. If no match exists,MatchImplrendersnull.Outletfinds the child id inpreviousIdswhen its route has left, so it never uses index-1.react-router/useMatch.tsx: usesstores.departedonly when the nearest route in context has itself left, that is, only inside the old tree. From a live tree,useMatch({ from })for an inactive route still throws, as before.useMatchnow always readsmatchContext, so the PR removes the unuseddummyMatchContext.Only the old tree reads the departed state, and only for the one render before it unmounts. The core change only adds state, so Solid and Vue do not change.
Why not just return
nullinMatchImpl? That stops the throw, but then the client renders nothing where the server HTML has content. React logs a recoverable hydration error and client-renders the boundary. The departed map keeps the one hydration render equal to the server HTML, soonRecoverableErrorstays empty.If the user navigates twice before the old tree hydrates, React tries one hydration per lane and then client-renders the new children; the
nullpath inMatchImplis only a guard for that case.Tests
packages/react-router/tests/departed-match-dehydrated-boundary.test.tsxuses only public APIs:/a/child, hydrates it with a child that stays suspended, and clicks a root link to/b. Before the click it asserts that/ahas not committed on the client, so the boundary is provably still dehydrated and the result does not depend on React scheduling. It then checks that/brenders one time and that no error callback fires. Onmain, this test fails with therouteIdTypeError.useMatch({ from: '/a' })inside/bstill throws.If you revert any one of the reader changes, a test fails.
Local runs on
mainat d35aab4 (after #8579), all exit 0:test:unit,test:eslint,test:types,build,test:buildforrouter-coreandreact-router, andtest:unitforsolid-routerandvue-router. With only thesrcchanges reverted, the new test fails with therouteIdTypeError. Bundle size ofreact-router.minimal: +103 bytes gzip (85965 to 86068).Follow-up: in the departed render,
useParentMatches/useChildMatchesreturn a wrong slice (no throw). Not changed here.✅ Checklist
🚀 Release Impact
Summary by CodeRabbit
useMatch({ from })continues to report an error when the requested route is no longer active, rather than returning a match from a different page.