-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(react-router): walk parent route chain for notFoundComponent lookup #6409
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?
fix(react-router): walk parent route chain for notFoundComponent lookup #6409
Conversation
📝 WalkthroughWalkthroughThis PR changes not-found resolution to traverse up the route hierarchy to find the nearest route with a Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 11m 32s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 50s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-01-22 17:20:51 UTC
|
Thank your for your PR, the issue is clear and based on the video (#6351 (comment)) it does seem like a real bug to me. However:
In the video example we have these routes So when the path to match is But in your case matching
router/packages/router-core/src/new-process-route-tree.ts Lines 1021 to 1029 in 17045eb
|
|
this PR breaks a lot of tests. check the CI output |
8204a4f to
963198c
Compare
Thank you @Sheraff for your patient feedback! What was wrong with my previous approachI realize now that my previous test case was incorrect, and I am sorry for any confusion this caused. As @Sheraff pointed out, the test was flawed because the Root cause analysis (Issue #6351)The problem exists in the
The fixI have updated // Recursively walk up the parent route chain to find the nearest notFoundComponent
let routeWithNotFound: AnyRoute | undefined = route
while (routeWithNotFound && !routeWithNotFound.options.notFoundComponent) {
routeWithNotFound = routeWithNotFound.parentRoute
}Test caseI added a robust test case in
Files changed
|
14bfa97 to
af3f4da
Compare

What Changed
Refactored
new-process-route-tree.tsto ensure pathless routes (e.g.,_authenticated) are correctly processed during fuzzy matching, allowing theirnotFoundComponentto trigger.Files Changed:
packages/router-core/src/new-process-route-tree.tsskipOnParamErrorguard for pathless node creation.node.routeassignment to pathless nodes.Why
**Fixes #6351 **
Pathless layout routes were being skipped during fuzzy matching when a child route failed to match. This prevented their
notFoundComponentfrom rendering, causing the error to bubble up to the root incorrectly.This change ensures pathless nodes are properly flagged in the segment tree so they are considered valid candidates for handling
notFounderrors.How Tested
New Tests:
packages/router-core/tests/pathless_fuzzy_matching.test.tsto verify:Results:
router-corepassed.**Closes #6351 **
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.