refactor: use local matches in adapter Match components#7052
refactor: use local matches in adapter Match components#7052
Conversation
Stop re-looking up matches by id inside the React, Solid, and Vue Match components. Each adapter now reads the promise and pending state from its existing local match source, and the Vue store update expectations are adjusted to reflect the lower update count from that change.
📝 WalkthroughWalkthroughRefactored Match components across React, Solid, and Vue router packages to access pending promises directly from current match store state rather than re-looking up via router. Simplified pending-minimum handling by removing intermediate router lookups. Adjusted test expectations for store update counts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 11m 12s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 1m 28s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-03-26 22:16:40 UTC
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
Bundle Size Benchmarks
Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97d98428e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| throw router.getMatch(match.id)?._nonReactive.loadPromise | ||
| throw match._nonReactive.loadPromise |
There was a problem hiding this comment.
Use of match._nonReactive.loadPromise here can suspend on a stale promise when a new navigation starts before the current pending navigation finishes and both active/pending pools contain the same match.id. In that overlap, router updates and promise resolution target the pending pool first (updateMatch/getMatch behavior in router-core), so the active-store promise can be canceled or left stale while the live pending promise is elsewhere; throwing the active promise can keep the fallback stuck until a later remount instead of tracking the in-flight navigation.
Useful? React with 👍 / 👎.
| await new Promise((r) => setTimeout(r, 0)) | ||
| return router.getMatch(currentMatch().id)?._nonReactive | ||
| .loadPromise | ||
| return match()?._nonReactive.loadPromise |
There was a problem hiding this comment.
This resource now reads match()?._nonReactive.loadPromise from the locally active match object, which has the same stale-promise problem during overlapping navigations with duplicate ids across active and pending pools. Because core load updates resolve promises on the pending pool entry first, binding the Suspense resource to the active object's promise can desynchronize the UI from the real in-flight load and prolong fallback rendering until the route tree is replaced.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-router/src/Match.tsx`:
- Around line 400-410: The timeout callback currently mutates the closed-over
match object by setting match._nonReactive.minPendingPromise = undefined, which
can leave stale state if the match is replaced; instead, when the timer fires
use the store/router API to clear the pending flag (e.g. call
matchStore.setState(...) or router.updateMatch(...) to set
_nonReactive.minPendingPromise to undefined for that match id) so the live store
is updated atomically rather than mutating the captured match object; locate the
timeout in the block that creates minPendingPromise and replace the direct
mutation with a store/router update using the match's identifier.
In `@packages/solid-router/src/Match.tsx`:
- Around line 301-316: The timeout callback is closing over the captured
matchState and clears matchState._nonReactive.minPendingPromise directly, which
can leave the live store holding stale state if the match object was replaced;
instead, when creating the minPendingPromise for matchState (from match()),
store its id and use router.updateMatch(matchId, updater) (or the store's update
method) inside the timeout to clear the minPendingPromise on the live match
object by id; locate the block around match(), createControlledPromise(), and
where _nonReactive.minPendingPromise is assigned and replace the direct mutation
in the setTimeout with a router.updateMatch keyed by matchState.id that sets
_nonReactive.minPendingPromise = undefined.
In `@packages/vue-router/src/Match.tsx`:
- Around line 422-437: The timeout currently mutates the captured
currentMatch._nonReactive.minPendingPromise directly which can leave stale state
if the router replaces the match; instead, inside the setTimeout callback
resolve the controlled promise but clear the minPendingPromise through the
router's match update API (e.g., call router.updateMatch(currentMatch.id, {
_nonReactive: { minPendingPromise: undefined } }) or use the match store update
function) using the currentMatch.id so the live store is updated atomically
rather than mutating the closed-over currentMatch object; keep creating and
assigning the controlled promise to currentMatch._nonReactive.minPendingPromise
before scheduling the timeout, but ensure the timeout uses router.updateMatch
(or equivalent) to clear it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f92cc9d0-f35f-4921-ab40-1f68c1ef2168
📒 Files selected for processing (4)
packages/react-router/src/Match.tsxpackages/solid-router/src/Match.tsxpackages/vue-router/src/Match.tsxpackages/vue-router/tests/store-updates-during-navigation.test.tsx
| if (!match._nonReactive.minPendingPromise) { | ||
| // Create a promise that will resolve after the minPendingMs | ||
| if (!(isServer ?? router.isServer)) { | ||
| const minPendingPromise = createControlledPromise<void>() | ||
|
|
||
| routerMatch._nonReactive.minPendingPromise = minPendingPromise | ||
| match._nonReactive.minPendingPromise = minPendingPromise | ||
|
|
||
| setTimeout(() => { | ||
| minPendingPromise.resolve() | ||
| // We've handled the minPendingPromise, so we can delete it | ||
| routerMatch._nonReactive.minPendingPromise = undefined | ||
| match._nonReactive.minPendingPromise = undefined |
There was a problem hiding this comment.
Clear minPendingPromise through the store in the timeout.
At Lines 407-410, the callback mutates the closed-over match object. Match stores replace their state object on updates, so if this match is replaced before the timer fires, the live store keeps stale min-pending state and can remain forced-pending. Please clear it through matchStore.setState(...) or router.updateMatch(...) instead of mutating the captured object. The analogous SSR hydrate path already does this.
Suggested fix
setTimeout(() => {
minPendingPromise.resolve()
// We've handled the minPendingPromise, so we can delete it
- match._nonReactive.minPendingPromise = undefined
+ matchStore.setState((prev) => {
+ if (prev._nonReactive.minPendingPromise === minPendingPromise) {
+ prev._nonReactive.minPendingPromise = undefined
+ return { ...prev }
+ }
+ return prev
+ })
}, pendingMinMs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/react-router/src/Match.tsx` around lines 400 - 410, The timeout
callback currently mutates the closed-over match object by setting
match._nonReactive.minPendingPromise = undefined, which can leave stale state if
the match is replaced; instead, when the timer fires use the store/router API to
clear the pending flag (e.g. call matchStore.setState(...) or
router.updateMatch(...) to set _nonReactive.minPendingPromise to undefined for
that match id) so the live store is updated atomically rather than mutating the
captured match object; locate the timeout in the block that creates
minPendingPromise and replace the direct mutation with a store/router update
using the match's identifier.
| const matchState = match() | ||
| if ( | ||
| routerMatch && | ||
| !routerMatch._nonReactive.minPendingPromise | ||
| matchState && | ||
| !matchState._nonReactive.minPendingPromise | ||
| ) { | ||
| // Create a promise that will resolve after the minPendingMs | ||
| if (!(isServer ?? router.isServer)) { | ||
| const minPendingPromise = createControlledPromise<void>() | ||
|
|
||
| routerMatch._nonReactive.minPendingPromise = | ||
| matchState._nonReactive.minPendingPromise = | ||
| minPendingPromise | ||
|
|
||
| setTimeout(() => { | ||
| minPendingPromise.resolve() | ||
| // We've handled the minPendingPromise, so we can delete it | ||
| routerMatch._nonReactive.minPendingPromise = undefined | ||
| matchState._nonReactive.minPendingPromise = undefined |
There was a problem hiding this comment.
Clear min-pending state against the live match, not the captured matchState.
At Lines 313-316, the timer closes over matchState and mutates _nonReactive directly. If the match store swaps to a newer state object before this fires, the cleanup hits an orphaned match while the live one keeps stale min-pending state. Please route this through router.updateMatch(...) (or the underlying store) keyed by matchState.id.
Suggested fix
if (!(isServer ?? router.isServer)) {
const minPendingPromise = createControlledPromise<void>()
+ const matchId = matchState.id
matchState._nonReactive.minPendingPromise =
minPendingPromise
setTimeout(() => {
minPendingPromise.resolve()
// We've handled the minPendingPromise, so we can delete it
- matchState._nonReactive.minPendingPromise = undefined
+ router.updateMatch(matchId, (prev) => {
+ if (
+ prev._nonReactive.minPendingPromise ===
+ minPendingPromise
+ ) {
+ prev._nonReactive.minPendingPromise = undefined
+ return { ...prev }
+ }
+ return prev
+ })
}, pendingMinMs)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/solid-router/src/Match.tsx` around lines 301 - 316, The timeout
callback is closing over the captured matchState and clears
matchState._nonReactive.minPendingPromise directly, which can leave the live
store holding stale state if the match object was replaced; instead, when
creating the minPendingPromise for matchState (from match()), store its id and
use router.updateMatch(matchId, updater) (or the store's update method) inside
the timeout to clear the minPendingPromise on the live match object by id;
locate the block around match(), createControlledPromise(), and where
_nonReactive.minPendingPromise is assigned and replace the direct mutation in
the setTimeout with a router.updateMatch keyed by matchState.id that sets
_nonReactive.minPendingPromise = undefined.
| if ( | ||
| pendingMinMs && | ||
| routerMatch && | ||
| !routerMatch._nonReactive.minPendingPromise | ||
| currentMatch && | ||
| !currentMatch._nonReactive.minPendingPromise | ||
| ) { | ||
| // Create a promise that will resolve after the minPendingMs | ||
| if (!(isServer ?? router.isServer)) { | ||
| const minPendingPromise = createControlledPromise<void>() | ||
|
|
||
| routerMatch._nonReactive.minPendingPromise = minPendingPromise | ||
| currentMatch._nonReactive.minPendingPromise = minPendingPromise | ||
|
|
||
| setTimeout(() => { | ||
| minPendingPromise.resolve() | ||
| // We've handled the minPendingPromise, so we can delete it | ||
| routerMatch._nonReactive.minPendingPromise = undefined | ||
| currentMatch._nonReactive.minPendingPromise = undefined | ||
| }, pendingMinMs) |
There was a problem hiding this comment.
Avoid clearing min-pending state on the captured currentMatch.
At Lines 433-436, the timeout mutates currentMatch._nonReactive directly. If the router replaces that match object before the timer fires, the live store keeps stale min-pending state. Please clear it via router.updateMatch(...) (or the match store) keyed by the current match id instead of mutating the closed-over object.
Suggested fix
if (
pendingMinMs &&
currentMatch &&
!currentMatch._nonReactive.minPendingPromise
) {
+ const matchId = currentMatch.id
// Create a promise that will resolve after the minPendingMs
if (!(isServer ?? router.isServer)) {
const minPendingPromise = createControlledPromise<void>()
currentMatch._nonReactive.minPendingPromise = minPendingPromise
setTimeout(() => {
minPendingPromise.resolve()
// We've handled the minPendingPromise, so we can delete it
- currentMatch._nonReactive.minPendingPromise = undefined
+ router.updateMatch(matchId, (prev) => {
+ if (prev._nonReactive.minPendingPromise === minPendingPromise) {
+ prev._nonReactive.minPendingPromise = undefined
+ return { ...prev }
+ }
+ return prev
+ })
}, pendingMinMs)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/vue-router/src/Match.tsx` around lines 422 - 437, The timeout
currently mutates the captured currentMatch._nonReactive.minPendingPromise
directly which can leave stale state if the router replaces the match; instead,
inside the setTimeout callback resolve the controlled promise but clear the
minPendingPromise through the router's match update API (e.g., call
router.updateMatch(currentMatch.id, { _nonReactive: { minPendingPromise:
undefined } }) or use the match store update function) using the currentMatch.id
so the live store is updated atomically rather than mutating the closed-over
currentMatch object; keep creating and assigning the controlled promise to
currentMatch._nonReactive.minPendingPromise before scheduling the timeout, but
ensure the timeout uses router.updateMatch (or equivalent) to clear it.
1 participant
Summary
Match.tsxadapters and use each adapter's existing local match source insteadTesting
CI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:unit --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:types --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/react-router:test:eslint --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/solid-router:test:unit --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/solid-router:test:types --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/solid-router:test:eslint --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/vue-router:test:unit --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/vue-router:test:types --outputStyle=stream --skipRemoteCacheCI=1 NX_DAEMON=false pnpm nx run @tanstack/vue-router:test:eslint --outputStyle=stream --skipRemoteCacheSummary by CodeRabbit
Refactor
Tests