fix: convert sync loader errors to rejected promises#147
Conversation
When a loader throws a synchronous error, the exception previously propagated through the Router's useMemo, crashing the entire <Router> component. This converts sync errors to rejected promises so they behave identically to async loader errors — surfaced via React's use() hook and catchable by Error Boundaries. Also adds FAQ documentation recommending the root layout error boundary pattern for handling loader errors. https://claude.ai/code/session_01PDeppPqxy1CVE8oWJFiHLM
Instead of converting sync loader errors to rejected promises (which changes the type contract), cache them as LoaderError instances. RouteRenderer checks for LoaderError and re-throws the original error during rendering, where Error Boundaries can catch it. This preserves the expected data types for components while ensuring sync loader errors don't crash the entire <Router>. https://claude.ai/code/session_01PDeppPqxy1CVE8oWJFiHLM
Move the error boundary recommendation from the FAQ to the "How Loaders Run" learn page where it fits naturally alongside other loader behavior documentation. https://claude.ai/code/session_01PDeppPqxy1CVE8oWJFiHLM
13c8fb7 to
2cf4eba
Compare
There was a problem hiding this comment.
Pull request overview
Fixes Router crashes caused by synchronous loader exceptions by preventing errors from escaping the Router’s useMemo, and instead re-throwing them during route rendering so React Error Boundaries can handle them. Also adds tests and documentation guidance for error-boundary placement with loaders.
Changes:
- Wrap synchronous loader exceptions in a cached sentinel (
LoaderError) instead of letting them throw during loader execution. - Re-throw wrapped synchronous loader errors during
RouteRendererrender to enable Error Boundary handling. - Add loader error tests and extend loader docs with an error-handling FAQ section.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/router/src/core/loaderCache.ts | Catches sync loader throws and caches a wrapper sentinel instead of throwing through Router memoization. |
| packages/router/src/Router/RouteRenderer.tsx | Detects the wrapper sentinel and throws the original error during render for Error Boundaries. |
| packages/router/src/tests/loader.test.tsx | Adds coverage ensuring sync loader errors are handled by Error Boundaries (root and nested). |
| packages/docs/src/pages/LearnLoadersPage.tsx | Documents recommended Error Boundary placement for loader error handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| loaderCache.set(cacheKey, route.loader(args)); | ||
| } catch (error) { | ||
| // Wrap synchronous loader errors so they don't crash the Router | ||
| // during useMemo. RouteRenderer will unwrap and re-throw during | ||
| // rendering, where Error Boundaries can catch them. | ||
| loaderCache.set(cacheKey, new LoaderError(error)); | ||
| } |
There was a problem hiding this comment.
PR title/description say sync loader errors are converted to rejected promises, but this change instead caches a LoaderError sentinel and relies on RouteRenderer to throw during render. To avoid confusion for reviewers/users, either update the PR metadata (and any docs wording) to match this implementation, or change the cache value here to a rejected Promise (and handle it consistently downstream).
| <code>{"<Outlet />"}</code>. This catches errors from any loader in | ||
| the route tree while keeping the root layout (header, navigation, | ||
| etc.) intact: |
There was a problem hiding this comment.
This doc implies a root layout Error Boundary wrapping will catch errors from "any loader in the route tree", but it will not catch errors from the root layout route's own loader (because that boundary renders only if RootLayout renders). Consider clarifying that it catches descendant route loader errors, and mention that to catch the root route loader error you need an Error Boundary above (or avoid a loader on the root layout).
| <code>{"<Outlet />"}</code>. This catches errors from any loader in | |
| the route tree while keeping the root layout (header, navigation, | |
| etc.) intact: | |
| <code>{"<Outlet />"}</code>. This lets you catch errors from | |
| descendant route loaders while keeping the root layout (header, | |
| navigation, etc.) intact. To catch errors from the root layout | |
| route’s own loader, place an error boundary above{" "} | |
| <code>{"<Router />"}</code> (or avoid using a loader on the root | |
| layout route): |
When a loader throws a synchronous error, the exception previously
propagated through the Router's useMemo, crashing the entire
component. This converts sync errors to rejected promises so they
behave identically to async loader errors — surfaced via React's
use() hook and catchable by Error Boundaries.
Also adds FAQ documentation recommending the root layout error
boundary pattern for handling loader errors.
https://claude.ai/code/session_01PDeppPqxy1CVE8oWJFiHLM