Skip to content

fix: handle beforeLoad throwing notFound correctly#6811

Merged
schiller-manuel merged 7 commits intomainfrom
fix-6779
Mar 4, 2026
Merged

fix: handle beforeLoad throwing notFound correctly#6811
schiller-manuel merged 7 commits intomainfrom
fix-6779

Conversation

@schiller-manuel
Copy link
Contributor

@schiller-manuel schiller-manuel commented Mar 3, 2026

fixes #6779

Summary by CodeRabbit

  • New Features

    • Enhanced not-found behavior: beforeLoad can trigger targeted notFound boundaries (including root) while preserving parent loader data; per-route notFoundComponent support; new demo routes and navigation to exercise deep and parent-boundary cases; exported rootRouteId for targeted notFound.
  • Documentation

    • Updated guide and migration example to explain boundary resolution and show notFoundComponent usage instead of prior API.
  • Tests

    • Expanded E2E and unit tests across frameworks for beforeLoad not-found, routeId targeting, deep/parent-boundary scenarios, and hydration global-not-found checks.

@nx-cloud
Copy link

nx-cloud bot commented Mar 3, 2026

View your CI Pipeline Execution ↗ for commit 5617b69

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 9m 24s View ↗

☁️ Nx Cloud last updated this comment at 2026-03-04 20:14:33 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Refactors loadMatches to deterministically resolve notFound thrown in beforeLoad (including routeId-targeting and boundary selection), adds not-found test routes and tests across frameworks, updates docs and generated route trees, and adjusts SSR hydration/dehydration and prerender filters.

Changes

Cohort / File(s) Summary
Docs
docs/router/guide/not-found-errors.md
Expanded explanation of how notFound() thrown in beforeLoad is resolved; migration example updated to use notFoundComponent and removed notFoundRoute usage.
Core load orchestration
packages/router-core/src/load-matches.ts
Major refactor: add serialError, externalize matchPromises, replace _handleNotFound with getNotFoundBoundaryIndex, defer notFound finalization to boundary selection, adjust sequencing of beforeLoad/loader/head, and update internal helper signatures.
Core SSR hydration
packages/router-core/src/ssr/ssr-client.ts, packages/router-core/src/ssr/ssr-server.ts, packages/router-core/src/ssr/types.ts, packages/router-core/tests/hydrate.test.ts
Add dehydration flag g ↔ hydrate mapping to match.globalNotFound; add tests for propagation.
Core tests
packages/router-core/tests/load.test.ts
Add extensive tests for head execution, beforeLoad-notFound scenarios, routeId-targeting, nested boundary behavior, and error propagation.
Router runtime tests
packages/{react,solid,vue}-router/tests/not-found.test.tsx, packages/*/tests/store-updates-during-navigation.test.tsx
Add notFound beforeLoad tests (routeId-targeting), surface rootRouteId for tests, and adjust expected store-update counts.
E2E route-tree generation
e2e/*-start/basic/src/routeTree.gen.ts
Generated route-tree additions for new not-found families (parent-boundary, via-beforeLoad-target-root, deep) and expanded public typings (FileRoutesByFullPath/To/Id and module augmentations).
E2E routes (React)
e2e/react-start/basic/src/routes/not-found/*, e2e/react-start/basic/src/routes/__root.tsx
Add nested not-found routes (/deep/..., /parent-boundary/..., /via-beforeLoad-target-root) with beforeLoad/loader logic, notFoundComponents, and navigation links.
E2E routes (Solid & Vue)
e2e/solid-start/basic/src/routes/not-found/*, e2e/vue-start/basic/src/routes/not-found/*
Add analogous parent-boundary and via-beforeLoad-target-root routes and notFoundComponents for Solid and Vue E2E suites.
E2E tests (React/Solid/Vue)
e2e/*-start/basic/tests/not-found.spec.ts
Enable beforeLoad in test matrices, remove a whitelist hydration error string, and add direct-visit/navigation tests covering boundary selection and loader-data preservation.
E2E config (prerender)
e2e/*-start/basic/vite.config.ts, e2e/react-start/basic/vite.config.ts
Prerender filter broadened to exclude entire /not-found subtree instead of only specific subpaths.
Not-found UI/components
e2e/react-start/basic/src/routes/not-found/deep/**, .../deep/index.tsx, .../deep/b/**
Add nested route components with validateSearch, loaderDeps, beforeLoad throws that call notFound, per-route notFoundComponent implementations for deeper branches.
Parent-boundary routes
e2e/*-start/basic/src/routes/not-found/parent-boundary/**
Add parent-boundary route with loader providing parent data, plus child route that throws notFound with/without routeId to test boundary selection and loader-data availability.

Sequence Diagram(s)

sequenceDiagram
  participant Browser as Browser
  participant RouterCore as RouterCore (loadMatches)
  participant BeforeLoad as Route.beforeLoad
  participant Loader as Route.loader
  participant Boundary as NotFoundBoundary

  Browser->>RouterCore: navigate to /not-found/...
  RouterCore->>BeforeLoad: run beforeLoad hooks (ordered)
  alt beforeLoad throws notFound
    BeforeLoad-->>RouterCore: throw notFound({ routeId?, data? })
    RouterCore->>RouterCore: getNotFoundBoundaryIndex(err) → select boundary index
    RouterCore->>Loader: optionally run loaders up to/for boundary
    RouterCore->>Boundary: ensure boundary has notFoundComponent (use default if needed)
    Boundary-->>RouterCore: notFoundComponent (with loader data) ready
    RouterCore->>Browser: finalize navigation → render selected notFoundComponent
  else no notFound
    RouterCore->>Loader: run loaders
    Loader-->>RouterCore: loader data
    RouterCore->>Browser: render route component
  end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested reviewers

  • Sheraff
  • nlynzaad

Poem

"🐇 I hopped through beforeLoad and found a clue,
Boundaries whispered where NotFound flew,
Parent data kept safe, root answered the call,
Tests sing loud as mismatches fall,
Hooray — routes align, and I nibble a carrot 🥕"

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: handle beforeLoad throwing notFound correctly' directly and specifically describes the main change in the PR, which is fixing the issue of throwing notFound() from beforeLoad hooks.
Linked Issues check ✅ Passed The PR addresses all coding requirements from issue #6779: fixing hydration mismatches when throwing notFound() in beforeLoad by implementing boundary-centric notFound logic, updating SSR serialization (g flag), and excluding not-found routes from prerendering.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the beforeLoad notFound handling issue: core routing logic, test coverage, E2E test routes/suites, and prerender configurations directly support the fix objectives without introducing unrelated modifications.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-6779

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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

@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Bundle Size Benchmarks

  • Commit: 9f35318c41fc
  • Measured at: 2026-03-04T20:01:23.839Z
  • Baseline source: history:2217f7c3f19f
  • Dashboard: bundle-size history
Scenario Current (gzip) Delta vs baseline Raw Brotli Trend
react-router.minimal 86.84 KiB +269 B (+0.30%) 273.33 KiB 75.44 KiB ▁▁▁▁▁▁▁▁▁▁▁█
react-router.full 89.88 KiB +281 B (+0.31%) 283.67 KiB 78.18 KiB ▁▁▁▁▁▁▁▁▁▁▁█
solid-router.minimal 36.17 KiB +299 B (+0.81%) 108.44 KiB 32.49 KiB ▁▁▁▁▁▁▁▁▁▁▁█
solid-router.full 40.50 KiB +293 B (+0.71%) 121.49 KiB 36.40 KiB ▁▁▁▁▁▁▁▁▁▁▁█
vue-router.minimal 52.03 KiB +287 B (+0.54%) 148.42 KiB 46.71 KiB ▁▁▁▁▁▁▁▁▁▁▁█
vue-router.full 56.85 KiB +306 B (+0.53%) 164.01 KiB 51.04 KiB ▁▁▁▁▁▁▁▁▁▁▁█
react-start.minimal 99.40 KiB +295 B (+0.29%) 312.48 KiB 85.98 KiB ▁▁▁▁▁▁▁▁▁▁▁█
react-start.full 102.78 KiB +288 B (+0.27%) 322.29 KiB 88.82 KiB ▁▁▁▁▁▁▁▁▁▁▁█
solid-start.minimal 48.49 KiB +307 B (+0.62%) 146.03 KiB 42.85 KiB ▁▁▁▁▁▁▁▁▁▁▁█
solid-start.full 53.96 KiB +290 B (+0.53%) 161.98 KiB 47.51 KiB ▁▁▁▁▁▁▁▁▁▁▁█

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

Copy link
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: 6

🧹 Nitpick comments (2)
packages/router-core/src/load-matches.ts (1)

593-593: New as any casts in the core loading path reduce type safety.

Line 593 and Line 1001 introduce broad casts that can hide interface drift in loader and router option contracts.

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

Also applies to: 1001-1002

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

In `@packages/router-core/src/load-matches.ts` at line 593, The new broad casts
using "as any" on parentMatchPromise (from matchPromises) remove type safety;
replace them by tightening the matchPromises and parentMatchPromise types
instead of casting: update the declaration/type of matchPromises (and related
variables used around loadMatches / loadRouteModule / loadMatches functions) to
the correct generic Promise<RouteMatch<T> | undefined>[] (or the specific
RouteMatch<T> union your loader returns), propagate those generics through
loadMatches/loadRouteModule signatures, and where necessary use unknown + safe
type guards or Promise.resolve<T>() to narrow types rather than "as any";
specifically remove the "as any" on parentMatchPromise and the similar cast
later and ensure callers supply the appropriate generic types so the compiler
enforces the loader/router option interfaces.
packages/router-core/tests/load.test.ts (1)

893-895: Tighten helper signatures to avoid any in new tests.

The beforeLoadNotFoundFactory tuple and runLoadMatchesAndCapture(router: any) lose useful compile-time safety in a critical behavior test suite.

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

Also applies to: 994-1006

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

In `@packages/router-core/tests/load.test.ts` around lines 893 - 895, The helper
signatures use unsafe any types: replace the tuple element types and the router
parameter with concrete router types to restore compile-time safety. Change
routes: readonly [any, any, any, any] to a strongly typed tuple (e.g., readonly
[RouteRecordRaw, RouteRecordRaw, RouteRecordRaw, RouteRecordRaw] or the
project's equivalent route descriptor type) and update the return type still
using ReturnType<typeof notFound>; also change runLoadMatchesAndCapture(router:
any) to accept the proper Router/RouterInstance type used in the codebase (e.g.,
Router or RouterHistory/RouterInstance type) so tests and helpers
(beforeLoadNotFoundFactory and runLoadMatchesAndCapture) use explicit types;
apply the same replacements for the other occurrences around lines 994-1006.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@e2e/react-start/basic/src/routes/not-found/deep/b/c/d.tsx`:
- Around line 21-24: Replace the untyped props in the notFoundComponent with the
exported NotFoundRouteProps type: import NotFoundRouteProps from the router
types (or the module that exports it) and change the component signature from
(props: any) => ... to (props: NotFoundRouteProps) => ..., ensuring you update
the import and any references to props.data, props.isNotFound, or props.routeId
accordingly so the component uses the correct typed properties.

In `@e2e/react-start/basic/src/routes/not-found/deep/b/c/route.tsx`:
- Around line 12-15: Replace the loose any in the notFoundComponent declaration
with the proper NotFoundRouteProps type from `@tanstack/router-core`: add an
import for NotFoundRouteProps and change the component signature from (props:
any) => ... to (props: NotFoundRouteProps) => ... so the props.data,
props.isNotFound and props.routeId are correctly typed for the
notFoundComponent.

In `@e2e/react-start/basic/src/routes/not-found/deep/b/route.tsx`:
- Around line 12-15: Replace the loose any type on the notFoundComponent handler
with the proper NotFoundRouteProps type: import NotFoundRouteProps from
'@tanstack/react-router' (or add it to existing imports) and change the handler
signature in notFoundComponent from (props: any) => ... to (props:
NotFoundRouteProps) => ... so the component props are strictly typed; ensure the
import name matches the used symbol NotFoundRouteProps and update any usages of
props accordingly.

In `@e2e/react-start/basic/src/routes/not-found/parent-boundary/route.tsx`:
- Around line 19-27: Replace the unsafe any prop on
ParentBoundaryNotFoundComponent with a narrow typed shape: declare a
NotFoundProps (or NotFoundPayload) interface like { data?: { source?: string } }
and change the component signature from function
ParentBoundaryNotFoundComponent(props: any) to
ParentBoundaryNotFoundComponent(props: NotFoundProps); keep the existing
accessors (props?.data?.source) and Route.useLoaderData() unchanged so runtime
behavior is identical while restoring TypeScript strict safety.

In `@packages/react-router/tests/not-found.test.tsx`:
- Around line 315-317: Replace the hardcoded and cast route id in the beforeLoad
handler with the in-scope parentRoute.id: in the beforeLoad function where it
currently throws notFound({ routeId: '/parent' as never }), change the payload
to use parentRoute.id (i.e., throw notFound({ routeId: parentRoute.id })) so the
notFound call uses the typed route identifier instead of an unsafe string cast.

In `@packages/router-core/src/load-matches.ts`:
- Around line 951-971: The catch block currently awaits
Promise.allSettled(matchPromises) which can hang if any matcher promise never
settles; instead, on error inspect matchPromises immediately by attaching
short-lived rejection handlers to each promise to capture redirect/not-found
reasons without waiting for full settlement: iterate over matchPromises and for
each do p.catch(err => { if (!firstRedirect && isRedirect(err)) firstRedirect =
err; if (!firstNotFound && isNotFound(err)) firstNotFound = err; /* swallow to
avoid unhandledRejection */ return undefined }); then skip awaiting allSettled
and proceed to throw firstRedirect or firstNotFound as before (references:
matchPromises, firstRedirect, firstNotFound, isRedirect, isNotFound).

---

Nitpick comments:
In `@packages/router-core/src/load-matches.ts`:
- Line 593: The new broad casts using "as any" on parentMatchPromise (from
matchPromises) remove type safety; replace them by tightening the matchPromises
and parentMatchPromise types instead of casting: update the declaration/type of
matchPromises (and related variables used around loadMatches / loadRouteModule /
loadMatches functions) to the correct generic Promise<RouteMatch<T> |
undefined>[] (or the specific RouteMatch<T> union your loader returns),
propagate those generics through loadMatches/loadRouteModule signatures, and
where necessary use unknown + safe type guards or Promise.resolve<T>() to narrow
types rather than "as any"; specifically remove the "as any" on
parentMatchPromise and the similar cast later and ensure callers supply the
appropriate generic types so the compiler enforces the loader/router option
interfaces.

In `@packages/router-core/tests/load.test.ts`:
- Around line 893-895: The helper signatures use unsafe any types: replace the
tuple element types and the router parameter with concrete router types to
restore compile-time safety. Change routes: readonly [any, any, any, any] to a
strongly typed tuple (e.g., readonly [RouteRecordRaw, RouteRecordRaw,
RouteRecordRaw, RouteRecordRaw] or the project's equivalent route descriptor
type) and update the return type still using ReturnType<typeof notFound>; also
change runLoadMatchesAndCapture(router: any) to accept the proper
Router/RouterInstance type used in the codebase (e.g., Router or
RouterHistory/RouterInstance type) so tests and helpers
(beforeLoadNotFoundFactory and runLoadMatchesAndCapture) use explicit types;
apply the same replacements for the other occurrences around lines 994-1006.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2217f7c and 1820369.

📒 Files selected for processing (33)
  • docs/router/guide/not-found-errors.md
  • e2e/react-start/basic/src/routeTree.gen.ts
  • e2e/react-start/basic/src/routes/__root.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/b/c/d.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/b/c/route.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/b/route.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/index.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/route.tsx
  • e2e/react-start/basic/src/routes/not-found/index.tsx
  • e2e/react-start/basic/src/routes/not-found/parent-boundary/index.tsx
  • e2e/react-start/basic/src/routes/not-found/parent-boundary/route.tsx
  • e2e/react-start/basic/src/routes/not-found/parent-boundary/via-beforeLoad.tsx
  • e2e/react-start/basic/src/routes/not-found/via-beforeLoad-target-root.tsx
  • e2e/react-start/basic/tests/not-found.spec.ts
  • e2e/react-start/basic/vite.config.ts
  • e2e/solid-start/basic/src/routeTree.gen.ts
  • e2e/solid-start/basic/src/routes/not-found/parent-boundary/route.tsx
  • e2e/solid-start/basic/src/routes/not-found/parent-boundary/via-beforeLoad.tsx
  • e2e/solid-start/basic/src/routes/not-found/via-beforeLoad-target-root.tsx
  • e2e/solid-start/basic/tests/not-found.spec.ts
  • e2e/vue-start/basic/src/routeTree.gen.ts
  • e2e/vue-start/basic/src/routes/not-found/parent-boundary/route.tsx
  • e2e/vue-start/basic/src/routes/not-found/parent-boundary/via-beforeLoad.tsx
  • e2e/vue-start/basic/src/routes/not-found/via-beforeLoad-target-root.tsx
  • e2e/vue-start/basic/tests/not-found.spec.ts
  • packages/react-router/tests/not-found.test.tsx
  • packages/react-router/tests/store-updates-during-navigation.test.tsx
  • packages/router-core/src/load-matches.ts
  • packages/router-core/tests/load.test.ts
  • packages/solid-router/tests/not-found.test.tsx
  • packages/solid-router/tests/store-updates-during-navigation.test.tsx
  • packages/vue-router/tests/not-found.test.tsx
  • packages/vue-router/tests/store-updates-during-navigation.test.tsx

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 3, 2026

Merging this PR will not alter performance

✅ 5 untouched benchmarks


Comparing fix-6779 (5617b69) with main (9f35318)

Open in CodSpeed

Comment on lines +969 to +970
if (firstRedirect) {
throw firstRedirect
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the redirection be honored if it was found "under" a notFound? I feel like whichever first error we see (notFound or redirect) is the one that should be used

Copy link
Member

Choose a reason for hiding this comment

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

I think first one in wins, especially here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the scenario where a loader throws a redirect after a child beforeLoad threw a notFound

i think we cannot honor the notFound since there is no loader data for this route and so we cannot render its route component (the types would not make the loader data optional if you throw a redirect)

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 3, 2026

More templates

@tanstack/arktype-adapter

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

@tanstack/eslint-plugin-router

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

@tanstack/history

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

@tanstack/nitro-v2-vite-plugin

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

@tanstack/react-router

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

@tanstack/react-router-devtools

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

@tanstack/react-router-ssr-query

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

@tanstack/react-start

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

@tanstack/react-start-client

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

@tanstack/react-start-server

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

@tanstack/router-cli

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

@tanstack/router-core

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

@tanstack/router-devtools

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

@tanstack/router-devtools-core

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

@tanstack/router-generator

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

@tanstack/router-plugin

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

@tanstack/router-ssr-query-core

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

@tanstack/router-utils

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

@tanstack/router-vite-plugin

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

@tanstack/solid-router

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

@tanstack/solid-router-devtools

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

@tanstack/solid-router-ssr-query

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

@tanstack/solid-start

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

@tanstack/solid-start-client

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

@tanstack/solid-start-server

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

@tanstack/start-client-core

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

@tanstack/start-fn-stubs

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

@tanstack/start-plugin-core

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

@tanstack/start-server-core

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

@tanstack/start-static-server-functions

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

@tanstack/start-storage-context

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

@tanstack/valibot-adapter

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

@tanstack/virtual-file-routes

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

@tanstack/vue-router

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

@tanstack/vue-router-devtools

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

@tanstack/vue-router-ssr-query

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

@tanstack/vue-start

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

@tanstack/vue-start-client

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

@tanstack/vue-start-server

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

@tanstack/zod-adapter

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

commit: bd5ed21

Copy link
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.

♻️ Duplicate comments (1)
packages/router-core/src/load-matches.ts (1)

951-975: ⚠️ Potential issue | 🟠 Major

Avoid allSettled blocking and enforce first terminal error precedence.

At Line 954, waiting on Promise.allSettled(matchPromises) can stall forever if any loader hangs, even after a redirect/notFound is already known. Also, Line 969 currently makes redirect always win over notFound once both exist; this can invert “first terminal error” behavior.

💡 Proposed fix
-  try {
-    await Promise.all(matchPromises)
-  } catch {
-    const settled = await Promise.allSettled(matchPromises)
-
-    for (const result of settled) {
-      if (result.status !== 'rejected') continue
-
-      const reason = result.reason
-      if (!firstRedirect && isRedirect(reason)) {
-        firstRedirect = reason
-      }
-      if (!firstNotFound && isNotFound(reason)) {
-        firstNotFound = reason
-      }
-    }
-  }
+  const trackedMatchPromises = matchPromises.map((p) =>
+    p.catch((reason) => {
+      // First terminal error wins
+      if (!firstRedirect && !firstNotFound) {
+        if (isRedirect(reason)) firstRedirect = reason
+        else if (isNotFound(reason)) firstNotFound = reason
+      }
+      throw reason
+    }),
+  )
+
+  try {
+    await Promise.all(trackedMatchPromises)
+  } catch {
+    // firstRedirect / firstNotFound already captured by the wrappers above
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/router-core/src/load-matches.ts` around lines 951 - 975, The current
catch block blocks on Promise.allSettled(matchPromises) and always prefers
redirect over notFound; instead, attach per-promise rejection handlers before
awaiting Promise.all so rejections are recorded immediately: for each promise in
matchPromises call .catch(reason => { if (!firstRedirect && isRedirect(reason))
firstRedirect = reason; if (!firstNotFound && isNotFound(reason)) firstNotFound
= reason; throw reason }) so the earliest terminal errors are captured in
chronological order without waiting for allSettled; after awaiting
Promise.all(matchPromises) keep the existing throw-firstRedirect logic but
compute notFoundToThrow using firstNotFound (which now reflects the earliest
notFound) and preserve the beforeLoadNotFound fallback only when inner.preload
is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/router-core/src/load-matches.ts`:
- Around line 951-975: The current catch block blocks on
Promise.allSettled(matchPromises) and always prefers redirect over notFound;
instead, attach per-promise rejection handlers before awaiting Promise.all so
rejections are recorded immediately: for each promise in matchPromises call
.catch(reason => { if (!firstRedirect && isRedirect(reason)) firstRedirect =
reason; if (!firstNotFound && isNotFound(reason)) firstNotFound = reason; throw
reason }) so the earliest terminal errors are captured in chronological order
without waiting for allSettled; after awaiting Promise.all(matchPromises) keep
the existing throw-firstRedirect logic but compute notFoundToThrow using
firstNotFound (which now reflects the earliest notFound) and preserve the
beforeLoadNotFound fallback only when inner.preload is false.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1820369 and 63465c5.

📒 Files selected for processing (1)
  • packages/router-core/src/load-matches.ts

Copy link
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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/router-core/src/ssr/ssr-server.ts (1)

45-56: ⚠️ Potential issue | 🟠 Major

Serialize g only when globalNotFound === true.

Line 54 currently serializes g for any non-undefined globalNotFound value. If globalNotFound is false, hydration can still treat g as present and incorrectly promote it to global not-found on the client.

Suggested fix
   const properties = [
     ['__beforeLoadContext', 'b'],
     ['loaderData', 'l'],
     ['error', 'e'],
     ['ssr', 'ssr'],
-    ['globalNotFound', 'g'],
   ] as const

   for (const [key, shorthand] of properties) {
     if (match[key] !== undefined) {
       dehydratedMatch[shorthand] = match[key]
     }
   }
+  if (match.globalNotFound === true) {
+    dehydratedMatch.g = true
+  }

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 `@packages/router-core/src/ssr/ssr-server.ts` around lines 45 - 56, The loop
that fills dehydratedMatch incorrectly serializes the 'g' shorthand for any
non-undefined match.globalNotFound; update the condition so that when key ===
'globalNotFound' you only assign dehydratedMatch['g'] if match.globalNotFound
=== true, otherwise keep the existing undefined check for other keys (i.e., for
keys !== 'globalNotFound' continue to use match[key] !== undefined); reference
the properties array, the loop for (const [key, shorthand] of properties),
match, and dehydratedMatch when making this change.
♻️ Duplicate comments (1)
packages/router-core/src/load-matches.ts (1)

950-954: ⚠️ Potential issue | 🟠 Major

Promise.allSettled in the failure path can still deadlock redirect/notFound finalization.

At Line 953, waiting for all promises to settle can block forever if any sibling loader hangs, even when another loader already rejected with a redirect/notFound.

💡 Minimal fast-path improvement
-  try {
-    await Promise.all(matchPromises)
-  } catch {
-    const settled = await Promise.allSettled(matchPromises)
+  try {
+    await Promise.all(matchPromises)
+  } catch (err) {
+    if (isRedirect(err)) {
+      throw err
+    }
+    const settled = await Promise.allSettled(matchPromises)

Also applies to: 959-964

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

In `@packages/router-core/src/load-matches.ts` around lines 950 - 954, The current
catch block awaits Promise.allSettled(matchPromises) which can hang if any
sibling loader never settles; change the logic so you short-circuit once a
decisive rejection (redirect/notFound) is observed instead of blindly waiting
for all promises. Concretely, wrap each entry in matchPromises with a small
helper that races the original promise against a "settled marker" (or a timeout)
so you can use Promise.race to detect the first rejection and then only await
Promise.allSettled for the already-wrapped promises (or immediately proceed)
rather than blocking indefinitely; update the catch block around await
Promise.all(matchPromises) / const settled = await
Promise.allSettled(matchPromises) to use this race/short-circuit approach with
the same matchPromises identifier so redirect/notFound finalization is not
deadlocked.
🧹 Nitpick comments (1)
packages/router-core/src/ssr/ssr-client.ts (1)

31-33: Normalize globalNotFound deterministically during hydration.

Line 31 only sets globalNotFound when g is present, but does not clear it when omitted. Prefer assigning from the sentinel explicitly to avoid stale state carryover if match objects are reused.

Suggested change
-  if (deyhydratedMatch.g !== undefined) {
-    match.globalNotFound = true
-  }
+  match.globalNotFound = deyhydratedMatch.g === true ? true : undefined

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 `@packages/router-core/src/ssr/ssr-client.ts` around lines 31 - 33, The
hydration currently only sets match.globalNotFound when deyhydratedMatch.g
exists, which can leave stale true values on reused match objects; change the
logic in the hydration code that handles deyhydratedMatch and match (the block
referencing deyhydratedMatch.g and match.globalNotFound) to deterministically
assign a boolean instead of conditionally setting it — e.g., set
match.globalNotFound to the result of the sentinel check (deyhydratedMatch.g !==
undefined) so the flag is explicitly true or false during hydration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/router-core/src/ssr/ssr-server.ts`:
- Around line 45-56: The loop that fills dehydratedMatch incorrectly serializes
the 'g' shorthand for any non-undefined match.globalNotFound; update the
condition so that when key === 'globalNotFound' you only assign
dehydratedMatch['g'] if match.globalNotFound === true, otherwise keep the
existing undefined check for other keys (i.e., for keys !== 'globalNotFound'
continue to use match[key] !== undefined); reference the properties array, the
loop for (const [key, shorthand] of properties), match, and dehydratedMatch when
making this change.

---

Duplicate comments:
In `@packages/router-core/src/load-matches.ts`:
- Around line 950-954: The current catch block awaits
Promise.allSettled(matchPromises) which can hang if any sibling loader never
settles; change the logic so you short-circuit once a decisive rejection
(redirect/notFound) is observed instead of blindly waiting for all promises.
Concretely, wrap each entry in matchPromises with a small helper that races the
original promise against a "settled marker" (or a timeout) so you can use
Promise.race to detect the first rejection and then only await
Promise.allSettled for the already-wrapped promises (or immediately proceed)
rather than blocking indefinitely; update the catch block around await
Promise.all(matchPromises) / const settled = await
Promise.allSettled(matchPromises) to use this race/short-circuit approach with
the same matchPromises identifier so redirect/notFound finalization is not
deadlocked.

---

Nitpick comments:
In `@packages/router-core/src/ssr/ssr-client.ts`:
- Around line 31-33: The hydration currently only sets match.globalNotFound when
deyhydratedMatch.g exists, which can leave stale true values on reused match
objects; change the logic in the hydration code that handles deyhydratedMatch
and match (the block referencing deyhydratedMatch.g and match.globalNotFound) to
deterministically assign a boolean instead of conditionally setting it — e.g.,
set match.globalNotFound to the result of the sentinel check (deyhydratedMatch.g
!== undefined) so the flag is explicitly true or false during hydration.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63465c5 and ca1f3bd.

📒 Files selected for processing (13)
  • e2e/react-start/basic/src/routes/not-found/deep/b/c/d.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/b/c/route.tsx
  • e2e/react-start/basic/src/routes/not-found/deep/b/route.tsx
  • e2e/react-start/basic/src/routes/not-found/parent-boundary/route.tsx
  • packages/react-router/tests/not-found.test.tsx
  • packages/router-core/src/load-matches.ts
  • packages/router-core/src/ssr/ssr-client.ts
  • packages/router-core/src/ssr/ssr-server.ts
  • packages/router-core/src/ssr/types.ts
  • packages/router-core/tests/hydrate.test.ts
  • packages/router-core/tests/load.test.ts
  • packages/solid-router/tests/not-found.test.tsx
  • packages/vue-router/tests/not-found.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • e2e/react-start/basic/src/routes/not-found/parent-boundary/route.tsx
  • packages/react-router/tests/not-found.test.tsx
  • packages/vue-router/tests/not-found.test.tsx

@schiller-manuel schiller-manuel merged commit 9c81d5a into main Mar 4, 2026
10 of 11 checks passed
@schiller-manuel schiller-manuel deleted the fix-6779 branch March 4, 2026 20:34
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.

Hydration error when throwing notFound() in beforeLoad of a child route

3 participants