Uh oh!
There was an error while loading. Please reload this page.
fix(react-router): render errorComponent for falsy thrown values - #8238
fix(react-router): render errorComponent for falsy thrown values#8238vladavoX wants to merge 1 commit into
Conversation
CatchBoundary decided whether it had caught an error from the truthiness of the thrown value. When a component, beforeLoad or loader threw a falsy value (`throw undefined`, `null` or `''`), the boundary stored it but the render check failed, so it re-rendered the same crashing children. React then escalated the error past that boundary, and since every route boundary and the global boundary ran the same check, the error skipped every errorComponent and unmounted the whole tree. Box the caught error in state, so the presence of the box marks the boundary as having caught something regardless of the thrown value. The default ErrorComponent and the global boundary's development warning now also read `message` defensively, since the value they receive can be `undefined` or `null`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesFalsy error handling
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk:⚪ Minimal · up to Catch boundaries now render configured or global error fallbacks for undefined, null, and empty-string throws instead of re-rendering failing content. The behavior is covered for component and loader failures, with no remaining merge-blocking risk identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
✨ 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 |
🎯 Changes
Fixes#8123.
CatchBoundarydecided whether it had caught an error from the truthiness of the thrown value. A falsy throw (throw undefined,null,'') was stored but the render check failed, so the boundary re-rendered the same crashing children and React escalated the error past it. Since every route boundary and the global boundary run the same check, the error skipped everyerrorComponentand unmounted the whole tree.CatchBoundarystate holds a boxedcaught: { error } | null, so the presence of the box marks "caught" independently of the value. ThegetDerivedStateFromPropsreset path andreset()clear the box.ErrorComponentreadserror?.message. With the boundary fixed it now receivesundefined/null, and the previouserror.messagethrew inside its own fallback — which escalated again and still blanked the page.String(error?.message || error)for the same reason. This keeps the oldtoString()fallback, sonew Error('')and error-shaped objects still print an identity.errorComponent.test.tsxcoverundefined,nulland''thrown from a route component, from a loader, and with noerrorComponentconfigured. All nine fail onmain.The same truthiness check exists in
lazyRouteComponent(an import rejected with a falsy reason retries forever instead of throwing) and invue-router'sCatchBoundary. Both are left out to keep this PR to the reported issue; happy to send follow-ups.✅ Checklist
🚀 Release Impact
Summary by CodeRabbit
null,undefined, or an empty string.