Skip to content

fix: handle stale redirected matches during view transitions#7158

Open
schiller-manuel wants to merge 2 commits intomainfrom
fix-7120
Open

fix: handle stale redirected matches during view transitions#7158
schiller-manuel wants to merge 2 commits intomainfrom
fix-7120

Conversation

@schiller-manuel
Copy link
Copy Markdown
Contributor

@schiller-manuel schiller-manuel commented Apr 11, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a client-side crash where a redirect during navigation could blank the UI when view transitions and lazy routes were enabled.
    • Ensured pending UI displays reliably during such redirects instead of leaving the screen empty.
  • Tests

    • Added an end-to-end reproduction test and fixture to prevent regressions and validate the pending UI/redirect behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

This PR prevents a client-side crash when a root beforeLoad redirect races with pending UI and a lazy-loaded target while defaultViewTransition is enabled by introducing a retained-pending mechanism and updating Match logic across React, Solid, and Vue; adds an e2e fixture reproducing the regression.

Changes

Cohort / File(s) Summary
E2E Fixture & Metadata
/.changeset/bright-cars-lie.md, e2e/react-router/issue-7120/index.html, e2e/react-router/issue-7120/package.json, e2e/react-router/issue-7120/playwright.config.ts, e2e/react-router/issue-7120/tsconfig.json, e2e/react-router/issue-7120/vite.config.js
New end-to-end test fixture for issue-7120: changeset, app host files, Playwright config/global setup/teardown wiring, and build/dev tooling config.
E2E App & Tests
e2e/react-router/issue-7120/src/main.tsx, e2e/react-router/issue-7120/src/posts.lazy.tsx, e2e/react-router/issue-7120/src/posts.ts, e2e/react-router/issue-7120/src/styles.css, e2e/react-router/issue-7120/tests/issue-7120.repro.spec.ts, e2e/react-router/issue-7120/tests/setup/global.setup.ts, e2e/react-router/issue-7120/tests/setup/global.teardown.ts
Adds a React Router repro app with a root beforeLoad redirect to a lazy /posts route, fetching posts (switchable to local dummy server), CSS/Tailwind, and a Playwright test that asserts no crash and correct pending UI behavior.
Router Core: retained-pending support
packages/router-core/src/Matches.ts, packages/router-core/src/index.ts, packages/router-core/src/load-matches.ts
Introduces retainedPendingPromise field and markMatchPendingVisible() export; adds clearing of retained pending promise in multiple load/redirect/error finalization paths.
React Router Match updates
packages/react-router/src/Match.tsx, packages/react-router/tests/redirect.test.tsx
Adds PendingRouteMatch wrapper to mark matches pending-visible; refactors pending/redirect suspension logic to prefer loader promises but fall back to retained pending; updates test to enable defaultViewTransition and assert pending UI appears.
Solid Router Match updates
packages/solid-router/src/Match.tsx, packages/solid-router/tests/redirect.test.tsx
Adds PendingRouteMatch, retained-pending handling, and revised promise selection (including router.latestLoadPromise fallback); adjusts pending rendering and test assertions to cover view-transition case.
Vue Router Match updates
packages/vue-router/src/Match.tsx, packages/vue-router/tests/redirect.test.tsx
Adds PendingRouteMatch, simplifies MatchInner shape passing, generalizes promise lookups with router.latestLoadPromise fallback and retained-pending use for redirects; test updated to check pending UI during redirect.

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant FrameworkMatch as Match Component
  participant RouterCore
  participant LazyRoute as Lazy Loader
  Browser->>FrameworkMatch: Navigate (root)
  FrameworkMatch->>RouterCore: beforeLoad runs -> redirect to /posts
  Browser->>FrameworkMatch: Render pendingComponent (view transition starts)
  FrameworkMatch->>RouterCore: markMatchPendingVisible(matchId)
  RouterCore->>LazyRoute: start loading /posts (async)
  alt Lazy resolves before redirect finalizes
    LazyRoute-->>FrameworkMatch: component ready
    FrameworkMatch->>RouterCore: consume load result -> finish navigation
  else Redirect/transition races and match becomes stale
    RouterCore-->>FrameworkMatch: provide retainedPendingPromise fallback
    FrameworkMatch->>Browser: keep/clear pending UI safely (no crash)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • tannerlinsley

Poem

🐰 A hop, a pending promise held so tight,

Redirects raced the view transition’s light.
Retained and marked, no stale match will fall,
Three frameworks shelter one graceful call.
Paws on keys — regressions fixed tonight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: handle stale redirected matches during view transitions' accurately and concisely describes the main change: handling stale redirected matches that occur during view transitions when a redirect races with lazy-loaded routes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-7120

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud bot commented Apr 11, 2026

View your CI Pipeline Execution ↗ for commit afbe97a

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ❌ Failed 17m 23s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 5s View ↗

☁️ Nx Cloud last updated this comment at 2026-04-11 23:59:59 UTC

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 11, 2026

🚀 Changeset Version Preview

1 package(s) bumped directly, 4 bumped as dependents.

🟩 Patch bumps

Package Version Reason
@tanstack/react-router 1.168.18 → 1.168.19 Changeset
@tanstack/react-start 1.167.31 → 1.167.32 Dependent
@tanstack/react-start-client 1.166.35 → 1.166.36 Dependent
@tanstack/react-start-rsc 0.0.11 → 0.0.12 Dependent
@tanstack/react-start-server 1.166.36 → 1.166.37 Dependent

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 11, 2026

Bundle Size Benchmarks

  • Commit: c3900c832804
  • Measured at: 2026-04-11T23:35:07.316Z
  • Baseline source: history:0e2c9003c18a
  • Dashboard: bundle-size history
Scenario Current (gzip) Delta vs baseline Raw Brotli Trend
react-router.minimal 87.48 KiB +146 B (+0.16%) 275.12 KiB 76.03 KiB █▇▇▇▄▁▁▁▁▁▁▇
react-router.full 90.76 KiB +149 B (+0.16%) 286.19 KiB 78.92 KiB ████▄▁▁▁▁▁▁▇
solid-router.minimal 35.67 KiB +140 B (+0.38%) 107.22 KiB 32.08 KiB ▅▅▅▅▃▁▁▁▁▁▁█
solid-router.full 40.14 KiB +159 B (+0.39%) 120.71 KiB 36.03 KiB ▅▅▅▅▃▁▁▁▁▁▁█
vue-router.minimal 53.43 KiB +119 B (+0.22%) 152.60 KiB 47.99 KiB ████▄▁▁▁▁▁▁▆
vue-router.full 58.31 KiB +106 B (+0.18%) 168.02 KiB 52.23 KiB ████▄▁▁▁▁▁▁▆
react-start.minimal 101.88 KiB +141 B (+0.14%) 322.85 KiB 88.06 KiB ██▅▅▃▁▁▁▁▁▁▅
react-start.full 105.32 KiB +136 B (+0.13%) 333.17 KiB 91.03 KiB ██▆▆▃▁▁▁▁▁▁▅
solid-start.minimal 49.70 KiB +174 B (+0.34%) 153.03 KiB 43.80 KiB ██▅▅▃▁▁▁▁▁▁█
solid-start.full 55.20 KiB +159 B (+0.28%) 169.23 KiB 48.53 KiB ██▅▅▄▁▁▁▁▁▁▇

Trend sparkline is historical gzip bytes ending with this PR measurement; lower is better.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 11, 2026

More templates

@tanstack/arktype-adapter

npm i https://pkg.pr.new/@tanstack/arktype-adapter@7158

@tanstack/eslint-plugin-router

npm i https://pkg.pr.new/@tanstack/eslint-plugin-router@7158

@tanstack/eslint-plugin-start

npm i https://pkg.pr.new/@tanstack/eslint-plugin-start@7158

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@7158

@tanstack/nitro-v2-vite-plugin

npm i https://pkg.pr.new/@tanstack/nitro-v2-vite-plugin@7158

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@7158

@tanstack/react-router-devtools

npm i https://pkg.pr.new/@tanstack/react-router-devtools@7158

@tanstack/react-router-ssr-query

npm i https://pkg.pr.new/@tanstack/react-router-ssr-query@7158

@tanstack/react-start

npm i https://pkg.pr.new/@tanstack/react-start@7158

@tanstack/react-start-client

npm i https://pkg.pr.new/@tanstack/react-start-client@7158

@tanstack/react-start-rsc

npm i https://pkg.pr.new/@tanstack/react-start-rsc@7158

@tanstack/react-start-server

npm i https://pkg.pr.new/@tanstack/react-start-server@7158

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@7158

@tanstack/router-core

npm i https://pkg.pr.new/@tanstack/router-core@7158

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@7158

@tanstack/router-devtools-core

npm i https://pkg.pr.new/@tanstack/router-devtools-core@7158

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@7158

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@7158

@tanstack/router-ssr-query-core

npm i https://pkg.pr.new/@tanstack/router-ssr-query-core@7158

@tanstack/router-utils

npm i https://pkg.pr.new/@tanstack/router-utils@7158

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@7158

@tanstack/solid-router

npm i https://pkg.pr.new/@tanstack/solid-router@7158

@tanstack/solid-router-devtools

npm i https://pkg.pr.new/@tanstack/solid-router-devtools@7158

@tanstack/solid-router-ssr-query

npm i https://pkg.pr.new/@tanstack/solid-router-ssr-query@7158

@tanstack/solid-start

npm i https://pkg.pr.new/@tanstack/solid-start@7158

@tanstack/solid-start-client

npm i https://pkg.pr.new/@tanstack/solid-start-client@7158

@tanstack/solid-start-server

npm i https://pkg.pr.new/@tanstack/solid-start-server@7158

@tanstack/start-client-core

npm i https://pkg.pr.new/@tanstack/start-client-core@7158

@tanstack/start-fn-stubs

npm i https://pkg.pr.new/@tanstack/start-fn-stubs@7158

@tanstack/start-plugin-core

npm i https://pkg.pr.new/@tanstack/start-plugin-core@7158

@tanstack/start-server-core

npm i https://pkg.pr.new/@tanstack/start-server-core@7158

@tanstack/start-static-server-functions

npm i https://pkg.pr.new/@tanstack/start-static-server-functions@7158

@tanstack/start-storage-context

npm i https://pkg.pr.new/@tanstack/start-storage-context@7158

@tanstack/valibot-adapter

npm i https://pkg.pr.new/@tanstack/valibot-adapter@7158

@tanstack/virtual-file-routes

npm i https://pkg.pr.new/@tanstack/virtual-file-routes@7158

@tanstack/vue-router

npm i https://pkg.pr.new/@tanstack/vue-router@7158

@tanstack/vue-router-devtools

npm i https://pkg.pr.new/@tanstack/vue-router-devtools@7158

@tanstack/vue-router-ssr-query

npm i https://pkg.pr.new/@tanstack/vue-router-ssr-query@7158

@tanstack/vue-start

npm i https://pkg.pr.new/@tanstack/vue-start@7158

@tanstack/vue-start-client

npm i https://pkg.pr.new/@tanstack/vue-start-client@7158

@tanstack/vue-start-server

npm i https://pkg.pr.new/@tanstack/vue-start-server@7158

@tanstack/zod-adapter

npm i https://pkg.pr.new/@tanstack/zod-adapter@7158

commit: afbe97a

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
e2e/react-router/issue-7120/src/posts.ts (1)

9-19: Prefer an explicit return type for fetchPosts for stronger type contracts.

This helps keep call-site expectations stable if response typing changes later.

♻️ Suggested update
 export const fetchPosts = async () => {
+export const fetchPosts = async (): Promise<Array<PostType>> => {
   return axios
     .get<Array<PostType>>(`${queryURL}/posts`)
     .then((r) => r.data.slice(0, 10))
 }

As per coding guidelines, **/*.{ts,tsx}: Use TypeScript strict mode with extensive type safety.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/react-router/issue-7120/src/posts.ts` around lines 9 - 19, The function
fetchPosts lacks an explicit return type; update its signature to include a
strong TypeScript return type (e.g. export const fetchPosts = async ():
Promise<PostType[]> => { ... }) so callers know they receive an array of
PostType (the function still returns the first 10 items from queryURL via
axios.get<Array<PostType>>). Keep the implementation unchanged except for the
annotated return type (or use Promise<Array<PostType>> if you prefer that
notation).
🤖 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/solid-router/src/Match.tsx`:
- Around line 336-343: The helper getRetainedPendingPromise loses access to a
stale match's internal _nonReactive because rawMatchState().match returns a
stripped match; restore or preserve the original match object so its
_nonReactive is available as a fallback. Concretely, update rawMatchState (or
the code that produces fallbackMatch) to return the full match object including
_nonReactive (instead of stripping it), then change getRetainedPendingPromise to
prefer router.getMatch(matchId)?._nonReactive.retainedPendingPromise and fall
back to fallbackMatch?._nonReactive.retainedPendingPromise; apply the same
change to the second occurrence (the block around lines 455-462) so both places
can suspend on the retainedPendingPromise of a stale match.
- Around line 32-37: The current Solid.onMount call only runs once so when
PendingRouteMatch remains mounted and props.matchId changes the new match isn't
marked pending-visible; replace or supplement the onMount with a reactive
watcher (e.g., Solid.createEffect) that depends on props.matchId, call
router.getMatch(props.matchId) inside it and invoke
markMatchPendingVisible(match) whenever a non-null match is found; keep or
remove the original onMount as needed to preserve initial behavior, and
reference the existing functions Solid.onMount / Solid.createEffect,
router.getMatch, markMatchPendingVisible and the PendingRouteMatch component to
locate where to add the reactive effect.

In `@packages/vue-router/src/Match.tsx`:
- Around line 304-309: The component currently only calls
markMatchPendingVisible in Vue.onMounted, so when props.matchId changes in a
reused instance new matches aren't marked; add a watcher using Vue.watch(() =>
props.matchId, ...) (in addition to the existing Vue.onMounted call) that
retrieves the new match via router.getMatch(props.matchId) and invokes
markMatchPendingVisible(match) when a match exists, ensuring pending visibility
is re-applied on matchId updates.

---

Nitpick comments:
In `@e2e/react-router/issue-7120/src/posts.ts`:
- Around line 9-19: The function fetchPosts lacks an explicit return type;
update its signature to include a strong TypeScript return type (e.g. export
const fetchPosts = async (): Promise<PostType[]> => { ... }) so callers know
they receive an array of PostType (the function still returns the first 10 items
from queryURL via axios.get<Array<PostType>>). Keep the implementation unchanged
except for the annotated return type (or use Promise<Array<PostType>> if you
prefer that notation).
🪄 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: 58c421fc-673d-4c3f-88da-a66531f614e0

📥 Commits

Reviewing files that changed from the base of the PR and between c3900c8 and 60932dd.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (22)
  • .changeset/bright-cars-lie.md
  • e2e/react-router/issue-7120/index.html
  • e2e/react-router/issue-7120/package.json
  • e2e/react-router/issue-7120/playwright.config.ts
  • e2e/react-router/issue-7120/src/main.tsx
  • e2e/react-router/issue-7120/src/posts.lazy.tsx
  • e2e/react-router/issue-7120/src/posts.ts
  • e2e/react-router/issue-7120/src/styles.css
  • e2e/react-router/issue-7120/tests/issue-7120.repro.spec.ts
  • e2e/react-router/issue-7120/tests/setup/global.setup.ts
  • e2e/react-router/issue-7120/tests/setup/global.teardown.ts
  • e2e/react-router/issue-7120/tsconfig.json
  • e2e/react-router/issue-7120/vite.config.js
  • packages/react-router/src/Match.tsx
  • packages/react-router/tests/redirect.test.tsx
  • packages/router-core/src/Matches.ts
  • packages/router-core/src/index.ts
  • packages/router-core/src/load-matches.ts
  • packages/solid-router/src/Match.tsx
  • packages/solid-router/tests/redirect.test.tsx
  • packages/vue-router/src/Match.tsx
  • packages/vue-router/tests/redirect.test.tsx

Comment on lines +32 to +37
Solid.onMount(() => {
const match = router.getMatch(props.matchId)
if (match) {
markMatchPendingVisible(match)
}
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Re-run markMatchPendingVisible when Solid reuses the pending component.

onMount only fires once. If PendingRouteMatch stays mounted and props.matchId changes, the new match is never marked pending-visible, so later redirect/pending transitions won't retain the right promise.

Suggested fix
-  Solid.onMount(() => {
+  Solid.createEffect(() => {
     const match = router.getMatch(props.matchId)
     if (match) {
       markMatchPendingVisible(match)
     }
   })
🤖 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 32 - 37, The current
Solid.onMount call only runs once so when PendingRouteMatch remains mounted and
props.matchId changes the new match isn't marked pending-visible; replace or
supplement the onMount with a reactive watcher (e.g., Solid.createEffect) that
depends on props.matchId, call router.getMatch(props.matchId) inside it and
invoke markMatchPendingVisible(match) whenever a non-null match is found; keep
or remove the original onMount as needed to preserve initial behavior, and
reference the existing functions Solid.onMount / Solid.createEffect,
router.getMatch, markMatchPendingVisible and the PendingRouteMatch component to
locate where to add the reactive effect.

Comment on lines +336 to 343
const getRetainedPendingPromise = (
matchId: string,
fallbackMatch: AnyRouteMatch | undefined,
) => {
return (
router.getMatch(matchId)?._nonReactive.retainedPendingPromise ??
fallbackMatch?._nonReactive.retainedPendingPromise
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Keep the stale Solid match's _nonReactive state available.

These helpers only work while router.getMatch(matchId) still resolves. rawMatchState().match currently strips _nonReactive, so once the redirected match drops out of the router pool there is no retained promise left to suspend on and this branch falls through instead of preserving the pending UI.

Suggested fix
     return {
       key,
       routeId,
-      match: {
-        id: currentMatch.id,
-        status: currentMatch.status,
-        error: currentMatch.error,
-        _forcePending: currentMatch._forcePending ?? false,
-        _displayPending: currentMatch._displayPending ?? false,
-      },
+      match: currentMatch,
     }
-                const routerMatch = router.getMatch(matchId)
+                const fallbackMatch = currentMatch()

                 if (!isRedirect(currentMatch().error)) {
                   if (process.env.NODE_ENV !== 'production') {
                     throw new Error(
                       'Invariant failed: Expected a redirect error',
@@
                 const [loaderResult] = Solid.createResource(async () => {
                   await new Promise((r) => setTimeout(r, 0))
-                  return getLoadPromise(matchId, routerMatch)
+                  return getLoadPromise(matchId, fallbackMatch)
                 })

                 const [retainedPendingResult] = Solid.createResource(
                   async () => {
                     await new Promise((r) => setTimeout(r, 0))
-                    return getRetainedPendingPromise(matchId, routerMatch)
+                    return getRetainedPendingPromise(matchId, fallbackMatch)
                   },
                 )

Also applies to: 455-462

🤖 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 336 - 343, The helper
getRetainedPendingPromise loses access to a stale match's internal _nonReactive
because rawMatchState().match returns a stripped match; restore or preserve the
original match object so its _nonReactive is available as a fallback.
Concretely, update rawMatchState (or the code that produces fallbackMatch) to
return the full match object including _nonReactive (instead of stripping it),
then change getRetainedPendingPromise to prefer
router.getMatch(matchId)?._nonReactive.retainedPendingPromise and fall back to
fallbackMatch?._nonReactive.retainedPendingPromise; apply the same change to the
second occurrence (the block around lines 455-462) so both places can suspend on
the retainedPendingPromise of a stale match.

Comment on lines +304 to +309
Vue.onMounted(() => {
const match = router.getMatch(props.matchId)
if (match) {
markMatchPendingVisible(match)
}
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Re-run pending visibility marking when matchId changes.

PendingRouteMatch only calls markMatchPendingVisible on mount. Vue can reuse this component instance while patching a new matchId, so later pending/redirect transitions never mark the new match as visible and won't retain its pending promise.

Suggested fix
   setup(props) {
     const router = useRouter()

-    Vue.onMounted(() => {
-      const match = router.getMatch(props.matchId)
+    const markPendingVisible = (matchId: string) => {
+      const match = router.getMatch(matchId)
       if (match) {
         markMatchPendingVisible(match)
       }
-    })
+    }
+
+    Vue.onMounted(() => {
+      markPendingVisible(props.matchId)
+    })
+
+    Vue.watch(
+      () => props.matchId,
+      (matchId) => {
+        markPendingVisible(matchId)
+      },
+      { flush: 'post' },
+    )

     return () => (props.pendingComponent ? Vue.h(props.pendingComponent) : null)
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Vue.onMounted(() => {
const match = router.getMatch(props.matchId)
if (match) {
markMatchPendingVisible(match)
}
})
const markPendingVisible = (matchId: string) => {
const match = router.getMatch(matchId)
if (match) {
markMatchPendingVisible(match)
}
}
Vue.onMounted(() => {
markPendingVisible(props.matchId)
})
Vue.watch(
() => props.matchId,
(matchId) => {
markPendingVisible(matchId)
},
{ flush: 'post' },
)
🤖 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 304 - 309, The component
currently only calls markMatchPendingVisible in Vue.onMounted, so when
props.matchId changes in a reused instance new matches aren't marked; add a
watcher using Vue.watch(() => props.matchId, ...) (in addition to the existing
Vue.onMounted call) that retrieves the new match via
router.getMatch(props.matchId) and invokes markMatchPendingVisible(match) when a
match exists, ensuring pending visibility is re-applied on matchId updates.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 11, 2026

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing fix-7120 (afbe97a) with main (0e2c900)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (c3900c8) during the generation of this report, so 0e2c900 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

nx-cloud[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 444-467: Add a concise clarifying comment inside the
suspendOrKeepPending function (near the final "return null") explaining that
returning null is intentional: it covers the race where a resolved promise
cleared before retainedPendingPromise was set (i.e., there is simply no active
promise to throw/suspend on), so the caller should continue rendering normally;
reference the function name suspendOrKeepPending and variables promise and
retainedPendingPromise to indicate the exact fallback path being documented.
🪄 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: 95471f9b-c5d8-4377-b41c-233743a4f0ae

📥 Commits

Reviewing files that changed from the base of the PR and between 60932dd and afbe97a.

📒 Files selected for processing (1)
  • packages/react-router/src/Match.tsx

Comment on lines +444 to 467
const suspendOrKeepPending = (
key: 'displayPendingPromise' | 'minPendingPromise' | 'loadPromise',
) => {
const routerMatch = router.getMatch(match.id)

const promise =
routerMatch?._nonReactive[key] ??
match._nonReactive[key] ??
router.latestLoadPromise

if (promise) {
throw promise
}

const retainedPendingPromise =
routerMatch?._nonReactive.retainedPendingPromise ??
match._nonReactive.retainedPendingPromise

if (retainedPendingPromise) {
throw retainedPendingPromise
}

return null
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are any test cases or comments explaining when suspendOrKeepPending might return null

# Search for test coverage or comments about this behavior
rg -n -C3 "suspendOrKeepPending|return null" packages/react-router/src/

# Check if there's e2e coverage for this specific edge case
rg -n "retainedPendingPromise" e2e/

Repository: TanStack/router

Length of output: 6869


🏁 Script executed:

# Read the Match.tsx file around the suspendOrKeepPending function to understand the full context
sed -n '430,530p' packages/react-router/src/Match.tsx

Repository: TanStack/router

Length of output: 3287


🏁 Script executed:

# Search for where retainedPendingPromise is set and used
rg -n "retainedPendingPromise" packages/react-router/src/ packages/router-core/src/ --max-count=50

# Also check if there are comments explaining when suspendOrKeepPending might return null
rg -n -B5 "return null" packages/react-router/src/Match.tsx | head -100

Repository: TanStack/router

Length of output: 1488


🏁 Script executed:

# See the context around where retainedPendingPromise is set and cleared
sed -n '100,120p' packages/router-core/src/Matches.ts

# See the context where it's cleared
sed -n '40,55p' packages/router-core/src/load-matches.ts

# Search for tests related to this edge case
rg -n "suspendOrKeepPending|retainedPendingPromise" e2e/ --type tsx --type ts | head -30

Repository: TanStack/router

Length of output: 1049


🏁 Script executed:

# Search for e2e tests that cover pending/suspense behavior
find e2e -name "*.tsx" -o -name "*.ts" | head -20

# Look for tests related to pending or suspense
rg -n "pending|suspense" e2e/ --max-count=30 | grep -i "test\|it\|describe"

# Check the Matches.ts file for more context on how retainedPendingPromise is used
sed -n '100,180p' packages/router-core/src/Matches.ts

Repository: TanStack/router

Length of output: 21100


🏁 Script executed:

# Look at the full context of clearRetainedPending and when it's called
rg -n "clearRetainedPending" packages/router-core/src/ -A 5 -B 5

# Check the load-matches.ts to understand when promises are cleared
sed -n '30,60p' packages/router-core/src/load-matches.ts

Repository: TanStack/router

Length of output: 5198


🏁 Script executed:

# Look at the updateMatch callback and how match status changes
sed -n '220,280p' packages/router-core/src/load-matches.ts

Repository: TanStack/router

Length of output: 1589


🏁 Script executed:

# Search for where _displayPending and _forcePending are set
rg -n "_displayPending|_forcePending" packages/router-core/src/ --max-count=50

Repository: TanStack/router

Length of output: 637


🏁 Script executed:

# Look at the context where _displayPending and _forcePending are set and cleared
sed -n '110,130p' packages/router-core/src/ssr/ssr-client.ts
sed -n '280,310p' packages/router-core/src/ssr/ssr-client.ts

Repository: TanStack/router

Length of output: 1719


🏁 Script executed:

# Look for where markMatchPendingVisible is called to understand when retainedPendingPromise is set
rg -n "markMatchPendingVisible" packages/ --max-count=20

Repository: TanStack/router

Length of output: 687


🏁 Script executed:

# Look at the context around line 253 where markMatchPendingVisible is called
sed -n '240,280p' packages/react-router/src/Match.tsx

Repository: TanStack/router

Length of output: 1171


🏁 Script executed:

# Look at what renders PendingRouteMatch and when pendingElement is provided
sed -n '300,400p' packages/react-router/src/Match.tsx

Repository: TanStack/router

Length of output: 2895


🏁 Script executed:

# Continue reading the client-side rendering logic to see when PendingRouteMatch is rendered
sed -n '400,480p' packages/react-router/src/Match.tsx

Repository: TanStack/router

Length of output: 2442


🏁 Script executed:

# Look for any tests or code that might exercise the return null path
rg -n "return null" packages/react-router/src/Match.tsx -B 10 | head -40

# Check if there's any documentation about when return null is expected
rg -n "pending\|redirected" packages/react-router/src/Match.tsx | grep -i "comment\|todo\|fixme\|note"

Repository: TanStack/router

Length of output: 1269


🏁 Script executed:

# Search for any comments explaining the suspendOrKeepPending logic
sed -n '430,480p' packages/react-router/src/Match.tsx | cat -n

Repository: TanStack/router

Length of output: 1890


Consider adding a clarifying comment explaining the return null fallback path.

The return null at line 466 can be reached when a promise resolves before retainedPendingPromise is set (a race condition window that occurs when pending UI hasn't rendered yet). This is expected behavior, but adding a comment explaining that returning null is appropriate when there's no promise to suspend on would improve code clarity and prevent future maintainers from questioning this logic.

🤖 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 444 - 467, Add a concise
clarifying comment inside the suspendOrKeepPending function (near the final
"return null") explaining that returning null is intentional: it covers the race
where a resolved promise cleared before retainedPendingPromise was set (i.e.,
there is simply no active promise to throw/suspend on), so the caller should
continue rendering normally; reference the function name suspendOrKeepPending
and variables promise and retainedPendingPromise to indicate the exact fallback
path being documented.

Copy link
Copy Markdown
Contributor

@nx-cloud nx-cloud bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nx Cloud is proposing a fix for your failed CI:

We updated four Solid.createResource factory functions in packages/solid-router/src/Match.tsx to return Promise<null> instead of Promise<void>, fixing TypeScript 5.5 error TS1345 ("An expression of type 'void' cannot be tested for truthiness"). The factories now chain .then(() => null) on the underlying promises so their resolved type is null | undefined rather than void | undefined, making the ?? renderPending() expressions valid while preserving identical runtime suspension behavior.

Tip

We verified this fix by re-running @tanstack/solid-router:test:types.

Suggested Fix changes
diff --git a/packages/solid-router/src/Match.tsx b/packages/solid-router/src/Match.tsx
index 3f07c8889b..e43df877d3 100644
--- a/packages/solid-router/src/Match.tsx
+++ b/packages/solid-router/src/Match.tsx
@@ -354,9 +354,10 @@ export const MatchInner = (): any => {
             <Solid.Match when={currentMatch()._displayPending}>
               {(_) => {
                 const [displayPendingResult] = Solid.createResource(
-                  () =>
-                    router.getMatch(currentMatch().id)?._nonReactive
-                      .displayPendingPromise,
+                  (): Promise<null> | undefined =>
+                    router
+                      .getMatch(currentMatch().id)
+                      ?._nonReactive.displayPendingPromise?.then(() => null),
                 )
 
                 return <>{displayPendingResult() ?? renderPending()}</>
@@ -365,9 +366,10 @@ export const MatchInner = (): any => {
             <Solid.Match when={currentMatch()._forcePending}>
               {(_) => {
                 const [minPendingResult] = Solid.createResource(
-                  () =>
-                    router.getMatch(currentMatch().id)?._nonReactive
-                      .minPendingPromise,
+                  (): Promise<null> | undefined =>
+                    router
+                      .getMatch(currentMatch().id)
+                      ?._nonReactive.minPendingPromise?.then(() => null),
                 )
 
                 return <>{minPendingResult() ?? renderPending()}</>
@@ -401,11 +403,16 @@ export const MatchInner = (): any => {
                   }
                 }
 
-                const [loaderResult] = Solid.createResource(async () => {
-                  await new Promise((r) => setTimeout(r, 0))
-                  return router.getMatch(currentMatch().id)?._nonReactive
-                    .loadPromise
-                })
+                const [loaderResult] = Solid.createResource(
+                  (): Promise<null> =>
+                    new Promise<void>((r) => setTimeout(r, 0))
+                      .then(
+                        () =>
+                          router.getMatch(currentMatch().id)?._nonReactive
+                            .loadPromise,
+                      )
+                      .then(() => null),
+                )
 
                 return <>{loaderResult() ?? renderPending()}</>
               }}
@@ -447,16 +454,20 @@ export const MatchInner = (): any => {
                   invariant()
                 }
 
-                const [loaderResult] = Solid.createResource(async () => {
-                  await new Promise((r) => setTimeout(r, 0))
-                  return getLoadPromise(matchId, routerMatch)
-                })
+                const [loaderResult] = Solid.createResource(
+                  (): Promise<null> =>
+                    new Promise<void>((r) => setTimeout(r, 0))
+                      .then(() => getLoadPromise(matchId, routerMatch))
+                      .then(() => null),
+                )
 
                 const [retainedPendingResult] = Solid.createResource(
-                  async () => {
-                    await new Promise((r) => setTimeout(r, 0))
-                    return getRetainedPendingPromise(matchId, routerMatch)
-                  },
+                  (): Promise<null> =>
+                    new Promise<void>((r) => setTimeout(r, 0))
+                      .then(() =>
+                        getRetainedPendingPromise(matchId, routerMatch),
+                      )
+                      .then(() => null),
                 )
 
                 return <>{loaderResult() ?? retainedPendingResult()}</>

Apply fix via Nx Cloud  Reject fix via Nx Cloud


Or Apply changes locally with:

npx nx-cloud apply-locally jPV0-cpOm

Apply fix locally with your editor ↗   View interactive diff ↗



🎓 Learn more about Self-Healing CI on nx.dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant