-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: Fixed the defaultNotFoundComponent as guided by #5757 #5937
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
chore: Fixed the defaultNotFoundComponent as guided by #5757 #5937
Conversation
WalkthroughThe PR updates renderRouteNotFound implementations in React and Solid router packages by adding JSDoc documentation, changing the default not-found element from a div to a paragraph, adding a data parameter to the Solid implementation, and updating corresponding documentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 |
|
View your CI Pipeline Execution ↗ for commit f65d3e9
☁️ Nx Cloud last updated this comment at |
…' of github.com:DeTuksa/router into Fix-missing-notFoundComponent-error-message-TanStack#5757
…er and solid-router
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/solid-router/src/renderRouteNotFound.tsx (1)
16-16: All call sites properly updated for the newdataparameter.The breaking change has been successfully addressed. Both solid-router and react-router implementations have all call sites updated to pass the new parameter—either with error data or
undefined.Consider replacing
data: anywith a more specific type (e.g.,Record<string, unknown>) for improved type safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-router/src/renderRouteNotFound.tsx(2 hunks)packages/solid-router/src/renderRouteNotFound.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-router/src/renderRouteNotFound.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
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/solid-router/src/renderRouteNotFound.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (2)
packages/solid-router/src/renderRouteNotFound.tsx (2)
5-12: LGTM: Good documentation addition.The JSDoc comment properly documents the function's purpose, parameters, and return value, improving code maintainability.
26-26: LGTM: Fixes malformed HTML and aligns with documentation.The warning message now correctly uses
<p>Not Found</p>instead of the malformed<div>Not Found<div>, fixing both the HTML syntax error and aligning with the documented default component output.
| @@ -1,17 +1,20 @@ | |||
| /* eslint-disable */ | |||
| // Generated by Wrangler by running `wrangler types` (hash: b11df627d8b3c51b1bf3230a546b0f20) | |||
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.
unrelated file, please dont change it
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.
Got you! It was actually done by the autofix-ci[bot]. Would look to revert it now on my end
…' of github.com:DeTuksa/router into Fix-missing-notFoundComponent-error-message-TanStack#5757
|
Thanks! I think there's a mention in the |
Gotcha! I'll update it now to Do you think I should also correct the other places where |
|
@DeTuksa i think the remaining places are overrides - they can be left, it's allowed to do custom not found components. |
In
docs/router/framework/react/api/router/defaultGlobalNotFoundComponent.mdit shows that:DefaultGlobalNotFound returns
Summary by CodeRabbit
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.