-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(router-core,ssr-client): Catch errors thrown during hydrate #5417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a new /not-found/via-head route for React and Solid e2e apps, updates not-found UIs and tests to exercise head-triggered notFound, changes router-core notFound ancestor selection for BEFORE_LOAD, adds client-side hydrate error catching, and introduces hydrate tests; adjusts one react-router test expectation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as Browser
participant C as SSR_Client
participant R as Router
participant Rt as Route(/not-found/via-head)
U->>B: Navigate to /not-found/via-head
B->>C: hydrate()
C->>R: init hydration
rect rgba(200,230,255,0.6)
R->>Rt: call head()
Rt-->>R: throw notFound()
R-->>C: Promise rejects
C--x C: catch & log "Error during route context hydration:"
end
R->>R: map notFound to nearest ancestor with notFoundComponent
R-->>B: render notFoundComponent
sequenceDiagram
autonumber
participant M as Match(routeCursor)
participant H as _handleNotFound
participant A as AncestorRoute
M->>H: BEFORE_LOAD notFound(error with routeId)
alt matched route has notFoundComponent
H-->>M: handle on matched route
else matched route lacks notFoundComponent
loop ascend
H->>A: routeCursor = routeCursor.parentRoute
end
H-->>M: rewrite error.routeId = ancestor.id
end
note right of H: Ensures a renderable notFoundComponent is chosen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/router-core/tests/hydrate.test.ts (1)
1-254
: LGTM - Comprehensive test coverage.This test suite thoroughly validates the hydrate function's behavior, including the critical error handling during route context hydration (lines 215-254) which confirms the fix for issue #5322.
Consider addressing the import ordering for consistency:
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + notFound, +} from '../../react-router/dist/esm' import { hydrate } from '../src/ssr/ssr-client' import type { TsrSsrGlobal } from '../src/ssr/ssr-client' -import { - createMemoryHistory, - createRootRoute, - createRoute, - createRouter, - notFound, -} from '../../react-router/dist/esm' import type { AnyRouteMatch } from '../src'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
e2e/react-start/basic/src/routeTree.gen.ts
(10 hunks)e2e/react-start/basic/src/routes/not-found/index.tsx
(1 hunks)e2e/react-start/basic/src/routes/not-found/via-head.tsx
(1 hunks)e2e/react-start/basic/tests/not-found.spec.ts
(3 hunks)e2e/solid-start/basic/src/routeTree.gen.ts
(10 hunks)e2e/solid-start/basic/src/routes/not-found/index.tsx
(1 hunks)e2e/solid-start/basic/src/routes/not-found/via-head.tsx
(1 hunks)e2e/solid-start/basic/tests/not-found.spec.ts
(3 hunks)packages/react-router/tests/store-updates-during-navigation.test.tsx
(1 hunks)packages/router-core/src/load-matches.ts
(1 hunks)packages/router-core/src/ssr/ssr-client.ts
(1 hunks)packages/router-core/tests/hydrate.test.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
e2e/react-start/basic/src/routes/not-found/index.tsx
packages/router-core/src/ssr/ssr-client.ts
packages/router-core/tests/hydrate.test.ts
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
packages/router-core/src/load-matches.ts
e2e/solid-start/basic/tests/not-found.spec.ts
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routeTree.gen.ts
e2e/solid-start/basic/src/routeTree.gen.ts
packages/react-router/tests/store-updates-during-navigation.test.tsx
e2e/react-start/basic/tests/not-found.spec.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/react-start/basic/src/routes/not-found/index.tsx
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/react-start/basic/src/routes/not-found/index.tsx
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/tests/not-found.spec.ts
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routeTree.gen.ts
e2e/solid-start/basic/src/routeTree.gen.ts
e2e/react-start/basic/tests/not-found.spec.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/ssr/ssr-client.ts
packages/router-core/tests/hydrate.test.ts
packages/router-core/src/load-matches.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/store-updates-during-navigation.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.064Z
Learnt from: nlynzaad
PR: TanStack/router#5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.064Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/router-core/tests/hydrate.test.ts
🧬 Code graph analysis (5)
e2e/react-start/basic/src/routes/not-found/index.tsx (2)
e2e/react-start/basic/src/routes/not-found/via-head.tsx (1)
Route
(3-15)e2e/react-start/basic/src/routes/not-found/route.tsx (1)
Route
(4-8)
packages/router-core/tests/hydrate.test.ts (1)
packages/router-core/src/ssr/ssr-client.ts (1)
TsrSsrGlobal
(17-28)
e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
Route
(3-41)
e2e/react-start/basic/src/routes/not-found/via-head.tsx (1)
e2e/react-start/basic/src/routes/not-found/index.tsx (1)
Route
(3-41)
e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
Route
(3-15)
🪛 ESLint
packages/router-core/tests/hydrate.test.ts
[error] 1-1: Member 'expect' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 4-10: ../../react-router/dist/esm
import should occur before type import of ../src/ssr/ssr-client
(import/order)
🔇 Additional comments (8)
packages/react-router/tests/store-updates-during-navigation.test.tsx (1)
200-200
: LGTM - Optimization from improved error handling.The reduction from 7 to 6 updates reflects an optimization in the not-found handling flow introduced by this PR's BEFORE_LOAD improvements.
packages/router-core/src/ssr/ssr-client.ts (1)
213-215
: LGTM - Core fix for hydration error handling.This catch handler correctly prevents errors thrown during
head()
execution from causing unhandled promise rejections during hydration. The error is logged for debugging, and by this point, the error has already been assigned to the appropriate match (via the BEFORE_LOAD handling inload-matches.ts
), so thenotFoundComponent
will render correctly on the client.e2e/react-start/basic/src/routes/not-found/index.tsx (1)
28-37
: LGTM - Consistent navigation link addition.The new
via-head
link follows the established pattern and properly integrates with the test infrastructure.e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
28-37
: LGTM - Consistent navigation link addition.The new
via-head
link follows the established pattern for Solid Router and properly integrates with the test infrastructure.e2e/solid-start/basic/tests/not-found.spec.ts (2)
12-12
: LGTM - Necessary error whitelisting.This whitelisted error message corresponds to the console.error logging introduced in
packages/router-core/src/ssr/ssr-client.ts
when anotFound()
error occurs during route context hydration.
28-28
: LGTM - Test coverage expansion.Adding
'head'
to the thrower sequences enables comprehensive testing of the new not-found error handling path for errors thrown from thehead()
function.Also applies to: 57-57
packages/router-core/src/load-matches.ts (1)
56-66
: LGTM - Improved BEFORE_LOAD error handling.The new approach of walking up the route tree to find the nearest parent with a
notFoundComponent
is more robust than the previous implementation. The subsequent invariant check (lines 79-82) ensures that anotFoundComponent
is available, either from the route tree or viadefaultNotFoundComponent
.e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
1-23
: LGTM - Well-structured test route.This route correctly implements the test scenario for
notFound()
errors thrown from thehead()
function. The test-ids on both components enable proper E2E test verification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/router-core/tests/hydrate.test.ts (3)
1-11
: Fix import sort/order to satisfy lint rules.Alphabetize vitest specifiers and move the react-router import before internal type imports.
As per static analysis hints, apply:
-import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { hydrate } from '../src/ssr/ssr-client' -import type { TsrSsrGlobal } from '../src/ssr/ssr-client' -import { - createMemoryHistory, - createRootRoute, - createRoute, - createRouter, - notFound, -} from '../../react-router/dist/esm' -import type { AnyRouteMatch } from '../src' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + notFound, +} from '../../react-router/dist/esm' +import { hydrate } from '../src/ssr/ssr-client' +import type { TsrSsrGlobal } from '../src/ssr/ssr-client' +import type { AnyRouteMatch } from '../src'Based on static analysis hints
46-47
: Align test router with client context (optional).You’re hydrating on “client” paths; set isServer to false to avoid future coupling to server-only code paths.
- mockRouter = createRouter({ routeTree, history, isServer: true }) + mockRouter = createRouter({ routeTree, history, isServer: false })
215-254
: Make error assertion resilient and ensure spy cleanup.Use expect.objectContaining for partial match and restore the spy in a finally block.
- await hydrate(mockRouter) - - expect(consoleSpy).toHaveBeenCalledWith('Error during route context hydration:', { 'isNotFound': true }) - - consoleSpy.mockRestore() + try { + await hydrate(mockRouter) + expect(consoleSpy).toHaveBeenCalledWith( + 'Error during route context hydration:', + expect.objectContaining({ isNotFound: true }), + ) + } finally { + consoleSpy.mockRestore() + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
e2e/react-start/basic/src/routeTree.gen.ts
(10 hunks)e2e/react-start/basic/src/routes/not-found/index.tsx
(1 hunks)e2e/react-start/basic/src/routes/not-found/via-head.tsx
(1 hunks)e2e/react-start/basic/tests/not-found.spec.ts
(3 hunks)e2e/solid-start/basic/src/routeTree.gen.ts
(10 hunks)e2e/solid-start/basic/src/routes/not-found/index.tsx
(1 hunks)e2e/solid-start/basic/src/routes/not-found/via-head.tsx
(1 hunks)e2e/solid-start/basic/tests/not-found.spec.ts
(3 hunks)packages/react-router/tests/store-updates-during-navigation.test.tsx
(1 hunks)packages/router-core/src/load-matches.ts
(1 hunks)packages/router-core/src/ssr/ssr-client.ts
(1 hunks)packages/router-core/tests/hydrate.test.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/react-router/tests/store-updates-during-navigation.test.tsx
packages/router-core/src/load-matches.ts
packages/router-core/src/ssr/ssr-client.ts
e2e/solid-start/basic/src/routeTree.gen.ts
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/tests/not-found.spec.ts
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
e2e/react-start/basic/tests/not-found.spec.ts
packages/router-core/tests/hydrate.test.ts
e2e/react-start/basic/src/routeTree.gen.ts
packages/{react-router,solid-router}/**
📄 CodeRabbit inference engine (AGENTS.md)
Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Files:
packages/react-router/tests/store-updates-during-navigation.test.tsx
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/load-matches.ts
packages/router-core/src/ssr/ssr-client.ts
packages/router-core/tests/hydrate.test.ts
e2e/**
📄 CodeRabbit inference engine (AGENTS.md)
Store end-to-end tests under the e2e/ directory
Files:
e2e/solid-start/basic/src/routeTree.gen.ts
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/tests/not-found.spec.ts
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
e2e/react-start/basic/tests/not-found.spec.ts
e2e/react-start/basic/src/routeTree.gen.ts
**/src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Place file-based routes under src/routes/ directories
Files:
e2e/solid-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/index.tsx
e2e/react-start/basic/src/routes/not-found/via-head.tsx
e2e/solid-start/basic/src/routes/not-found/via-head.tsx
🧬 Code graph analysis (5)
e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
Route
(3-15)
e2e/react-start/basic/src/routes/not-found/index.tsx (2)
e2e/react-start/basic/src/routes/not-found/via-head.tsx (1)
Route
(3-15)e2e/react-start/basic/src/routes/not-found/route.tsx (1)
Route
(4-8)
e2e/react-start/basic/src/routes/not-found/via-head.tsx (1)
e2e/react-start/basic/src/routes/not-found/index.tsx (1)
Route
(3-41)
e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
Route
(3-41)
packages/router-core/tests/hydrate.test.ts (1)
packages/router-core/src/ssr/ssr-client.ts (1)
TsrSsrGlobal
(17-28)
🪛 ESLint
packages/router-core/tests/hydrate.test.ts
[error] 1-1: Member 'expect' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 4-10: ../../react-router/dist/esm
import should occur before type import of ../src/ssr/ssr-client
(import/order)
🔇 Additional comments (17)
packages/react-router/tests/store-updates-during-navigation.test.tsx (1)
200-200
: Confirm intentional reduction of store updates
- Change from
expect(updates).toBe(7)
to6
aligns with the optimized notFound BEFORE_LOAD traversal.- Validate no required state updates are skipped in other navigation scenarios.
- Run this test locally multiple times to ensure the count is stable and catches all expected updates.
e2e/solid-start/basic/src/routes/not-found/via-head.tsx (1)
1-23
: LGTM!The route implementation correctly demonstrates throwing
notFound()
from thehead
function. The structure follows TanStack Router conventions with proper test IDs for E2E testing, and the component includes a server-awareness attribute for debugging.e2e/react-start/basic/src/routes/not-found/index.tsx (1)
28-37
: LGTM!The new link follows the established pattern for the other navigation links and correctly uses the
preload
search parameter.packages/router-core/src/ssr/ssr-client.ts (1)
213-215
: Appropriate error handling for hydration failures.The catch handler correctly prevents client-side crashes when errors are thrown during route context hydration (including from
head()
functions). This aligns with the existing error handling pattern forrouter.load()
on line 232, where errors are logged but don't propagate to crash the application.The
notFoundComponent
will render correctly because_handleNotFound
is invoked before this point in the error flow, setting the appropriate match status.e2e/solid-start/basic/src/routes/not-found/index.tsx (1)
28-37
: LGTM!The new link correctly follows Solid conventions (using
class
instead ofclassName
and callingpreload()
as a function) and maintains consistency with the existing navigation links.e2e/react-start/basic/tests/not-found.spec.ts (2)
12-12
: Appropriate error whitelisting for hydration failures.The whitelisted error message matches the console.error output from the hydration catch handler in
packages/router-core/src/ssr/ssr-client.ts
(lines 213-214), preventing test failures from expected error logs.
28-28
: Expanded test coverage for head lifecycle.Adding
'head'
to the thrower sequences appropriately extends test coverage to verify thatnotFound()
thrown from thehead
function is handled correctly in both navigation and direct-visit scenarios.Also applies to: 58-58
e2e/solid-start/basic/tests/not-found.spec.ts (2)
12-12
: Appropriate error whitelisting for hydration failures.Matches the React test configuration, whitelisting the expected hydration error message from the catch handler.
28-28
: Expanded test coverage for head lifecycle.The thrower sequences now include
'head'
, maintaining test parity with the React implementation and ensuring comprehensive coverage of notFound handling across all lifecycle phases.Also applies to: 57-57
e2e/react-start/basic/src/routes/not-found/via-head.tsx (1)
1-23
: LGTM!The React route implementation correctly demonstrates throwing
notFound()
from thehead
function and follows React conventions (usingclassName
). The structure maintains parity with the Solid implementation.packages/router-core/src/load-matches.ts (1)
56-66
: Improved notFound handling for BEFORE_LOAD errors.The change from
const
tolet
forrouteCursor
enables traversal up the parent chain to find a suitable route with anotFoundComponent
when a BEFORE_LOAD error occurs. This ensures that notFound errors thrown duringbeforeLoad
are handled by the nearest ancestor that can render a not-found UI, rather than requiring each route to define its ownnotFoundComponent
.The logic correctly:
- Only applies to BEFORE_LOAD errors (line 60)
- Traverses parents while no
notFoundComponent
exists (line 61)- Updates the error's
routeId
to the handling route (line 65)- Falls back to the existing invariant check if no suitable parent is found (lines 69-82)
e2e/react-start/basic/src/routeTree.gen.ts (3)
37-37
: New route import looks correct.Import alias for NotFoundViaHead matches file path and conventions.
173-177
: Route wiring LGTM.Id/path '/via-head' under NotFoundRouteRoute aligns with siblings.
281-282
: Type tables and children updated consistently.All registries (fullPath/to/id unions, module declarations, and children interfaces/objects) include the new '/not-found/via-head' route. Looks coherent and generated as expected.
Also applies to: 317-318, 358-359, 401-402, 437-438, 477-478, 688-694, 819-820, 826-827
e2e/solid-start/basic/src/routeTree.gen.ts (3)
34-34
: New route import looks correct.Alias and path for NotFoundViaHead align with file layout.
160-164
: Route instance wiring LGTM.'/via-head' correctly attached under NotFoundRouteRoute.
253-254
: Type unions, module mappings, and children updated consistently.All necessary generated surfaces include '/not-found/via-head'. No gaps spotted.
Also applies to: 286-287, 325-326, 364-365, 397-398, 435-436, 634-640, 744-745, 751-752
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/router-core/src/load-matches.ts (1)
60-60
: Consider improving type safety for routerCode check.The type assertion
(err as any).routerCode
is not type-safe. Consider adding proper typing for error objects with router codes.If NotFoundError type can be extended, consider:
interface RouterError extends NotFoundError { routerCode?: string }Then check with proper typing:
- if ((err as any).routerCode === 'BEFORE_LOAD' && routeCursor.parentRoute) { + if ('routerCode' in err && err.routerCode === 'BEFORE_LOAD' && routeCursor.parentRoute) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-core/src/load-matches.ts
(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-core/src/load-matches.ts
packages/router-core/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep framework-agnostic core router logic in packages/router-core/
Files:
packages/router-core/src/load-matches.ts
🔇 Additional comments (2)
packages/router-core/src/load-matches.ts (2)
418-421
: LGTM!The async batch correctly awaits the resolve function, which is necessary to handle the asynchronous head execution added in the resolve function.
456-481
: LGTM!The async conversion of
updateContext
is correct and maintains proper sequencing. Allresolve()
calls are properly awaited, ensuring head execution completes before state updates.
let routeCursor = | ||
inner.router.routesById[err.routeId ?? ''] ?? inner.router.routeTree | ||
|
||
// For BEFORE_LOAD errors, find a parent route with a notFoundComponent that can handle the error | ||
if ((err as any).routerCode === 'BEFORE_LOAD' && routeCursor.parentRoute) { | ||
while (routeCursor.parentRoute && !routeCursor.options.notFoundComponent) { | ||
routeCursor = routeCursor.parentRoute | ||
} | ||
// Update the error to point to the error handling route | ||
err.routeId = routeCursor.id | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the NotFound ancestor routing behavior.
The new logic correctly climbs the route tree to find an ancestor with a notFoundComponent
when handling BEFORE_LOAD errors. This is a significant change in error handling behavior.
Run the following script to verify this logic is exercised in tests:
🏁 Script executed:
#!/bin/bash
# Description: Find test coverage for BEFORE_LOAD NotFound handling
# Search for tests that exercise NotFound errors during beforeLoad
rg -nP -A5 -B5 'BEFORE_LOAD|beforeLoad.*notFound|notFound.*beforeLoad' --type=ts --type=tsx -g '**/*test*' -g '**/*spec*'
# Search for routes with notFoundComponent to understand the hierarchy
ast-grep --pattern $'notFoundComponent: $_'
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Retry search for test coverage of BEFORE_LOAD NotFound handling without type filters
# Search for tests mentioning BEFORE_LOAD or notFound in .ts/.js test files
rg -nP -C3 'BEFORE_LOAD|beforeLoad|notFound' -g '**/*.{test,spec}.{ts,js}'
# Search for definitions of notFoundComponent across codebase
rg -nP 'notFoundComponent' -g '**/*.{ts,js}'
Length of output: 64948
Add unit tests for BEFORE_LOAD notFound ancestor routing.
Current tests don’t cover the case where a nested route’s beforeLoad
throws notFound()
and the logic climbs to a parent route with a notFoundComponent
; please add tests to assert that err.routeId
is updated to the correct ancestor.
const resolve = async () => { | ||
// Execute head during SSR | ||
if (inner.router.isServer) { | ||
try { | ||
const headResult = executeHead(inner, matchId, route) | ||
if (headResult) { | ||
const head = await headResult | ||
inner.updateMatch(matchId, (prev) => ({ | ||
...prev, | ||
...head, | ||
})) | ||
} | ||
} catch (err) { | ||
console.error('Error executing head:', err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle redirect and notFound errors from head execution.
The catch block only logs errors without proper handling. If head()
throws notFound()
or redirect()
, these should be handled via handleRedirectAndNotFound()
rather than being swallowed.
Apply this diff to properly handle errors:
if (inner.router.isServer) {
try {
const headResult = executeHead(inner, matchId, route)
if (headResult) {
const head = await headResult
inner.updateMatch(matchId, (prev) => ({
...prev,
...head,
}))
}
} catch (err) {
- console.error('Error executing head:', err)
+ if (isRedirect(err) || isNotFound(err)) {
+ throw err
+ }
+ console.error('Error executing head:', err)
}
}
📝 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.
const resolve = async () => { | |
// Execute head during SSR | |
if (inner.router.isServer) { | |
try { | |
const headResult = executeHead(inner, matchId, route) | |
if (headResult) { | |
const head = await headResult | |
inner.updateMatch(matchId, (prev) => ({ | |
...prev, | |
...head, | |
})) | |
} | |
} catch (err) { | |
console.error('Error executing head:', err) | |
} | |
} | |
const resolve = async () => { | |
// Execute head during SSR | |
if (inner.router.isServer) { | |
try { | |
const headResult = executeHead(inner, matchId, route) | |
if (headResult) { | |
const head = await headResult | |
inner.updateMatch(matchId, (prev) => ({ | |
...prev, | |
...head, | |
})) | |
} | |
} catch (err) { | |
if (isRedirect(err) || isNotFound(err)) { | |
throw err | |
} | |
console.error('Error executing head:', err) | |
} | |
} |
🤖 Prompt for AI Agents
In packages/router-core/src/load-matches.ts around lines 391 to 406, the catch
block for executeHead currently only logs errors and swallows redirect/notFound
signals; update the catch to call handleRedirectAndNotFound(err) so those
special errors are handled properly (and if handleRedirectAndNotFound does not
handle it, rethrow the error or return early), otherwise proceed to log
unexpected errors — ensure you invoke handleRedirectAndNotFound(err) before any
console.error and avoid suppressing redirects/notFound.
Fixes #5322
Summary by CodeRabbit
New Features
Bug Fixes
Tests