-
Notifications
You must be signed in to change notification settings - Fork 1
fix: convert sync loader errors to rejected promises #147
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,16 @@ import type { | |
| InternalRouteDefinition, | ||
| } from "../types.js"; | ||
|
|
||
| /** | ||
| * Wrapper for synchronous errors thrown by loaders. | ||
| * Cached instead of the raw error so the Router's useMemo doesn't throw. | ||
| * RouteRenderer checks for this class and re-throws the original error | ||
| * during rendering, where Error Boundaries can catch it. | ||
| */ | ||
| export class LoaderError { | ||
| constructor(public readonly error: unknown) {} | ||
| } | ||
|
|
||
| /** | ||
| * Cache for loader results. | ||
| * Key format: `${entryId}:${matchIndex}` | ||
|
|
@@ -28,7 +38,14 @@ function getOrCreateLoaderResult( | |
| const cacheKey = `${entryId}:${matchIndex}`; | ||
|
|
||
| if (!loaderCache.has(cacheKey)) { | ||
| loaderCache.set(cacheKey, route.loader(args)); | ||
| 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)); | ||
| } | ||
|
Comment on lines
+41
to
+48
|
||
| } | ||
|
|
||
| return loaderCache.get(cacheKey); | ||
|
|
||
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.
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).