Disallow useLoaderData in errorElement #9735
Conversation
🦋 Changeset detectedLatest commit: cf23c80 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| value={this.state.error} | ||
| children={this.props.component} | ||
| /> | ||
| </RouteContext.Provider> |
There was a problem hiding this comment.
Need to put the error boundary inside the RouteContext so it reflects the right matches, otherwise it thinks it' at the parent layer in the nested hierarchy 😬
| function useRouteContext(hookName: DataRouterStateHook) { | ||
| let route = React.useContext(RouteContext); | ||
| invariant(route, getDataRouterConsoleError(hookName)); | ||
| return route; | ||
| } | ||
|
|
||
| function useCurrentRouteId(hookName: DataRouterStateHook) { | ||
| let route = useRouteContext(hookName); | ||
| let thisRoute = route.matches[route.matches.length - 1]; | ||
| invariant( | ||
| thisRoute.route.id, | ||
| `${hookName} can only be used on routes that contain a unique "id"` | ||
| ); | ||
| return thisRoute.route.id; | ||
| } | ||
|
|
There was a problem hiding this comment.
Private utility hooks
| if (state.errors && state.errors[routeId] != null) { | ||
| console.error( | ||
| `You cannot \`useLoaderData\` in an errorElement (routeId: ${routeId})` | ||
| ); | ||
| return undefined; | ||
| } | ||
| return state.loaderData[routeId]; |
There was a problem hiding this comment.
Brought over from Remix - if we're calling this from the currently rendered error boundary match, return undefined
| @@ -2306,6 +2306,44 @@ describe("a router", () => { | |||
| child: new Error("Kaboom!"), | |||
| }); | |||
| }); | |||
|
|
|||
| it("clears previous loaderData at that route", async () => { | |||
There was a problem hiding this comment.
Added a handful of new tests because while in here I found that in some cases the router wasn't properly cleaning up stale actionData/loaderData upon a subsequent error
| @@ -1142,10 +1151,10 @@ export function createRouter(init: RouterInit): Router { | |||
| if (matchesToLoad.length === 0 && revalidatingFetchers.length === 0) { | |||
| completeNavigation(location, { | |||
| matches, | |||
| loaderData: mergeLoaderData(state.loaderData, {}, matches), | |||
| loaderData: {}, | |||
There was a problem hiding this comment.
This is a duplicative call, we can just let the empty object be merged inside completeNavigation now
| @@ -2535,7 +2550,7 @@ function getMatchesToLoad( | |||
| ? Object.values(pendingError)[0] | |||
| : pendingActionData | |||
| ? Object.values(pendingActionData)[0] | |||
| : null; | |||
| : undefined; | |||
There was a problem hiding this comment.
Changed to undefined to ensure it properly reflects "no action data" as compared to an action that returned null
| @@ -2899,6 +2914,9 @@ function processRouteLoaderData( | |||
| errors[boundaryMatch.route.id] = error; | |||
| } | |||
|
|
|||
| // Clear our any prior loaderData for the throwing route | |||
| loaderData[id] = undefined; | |||
There was a problem hiding this comment.
If this loader threw an error - we mark this to ensure that we don't hang onto it's prior loaderData during the merge
| if (pendingError) { | ||
| errors = pendingError; | ||
| loaderData[Object.keys(pendingError)[0]] = undefined; |
There was a problem hiding this comment.
If a route threw in an action, we wouldn't have run it's loaders so we don't hit it in the results loop above, so also ensure we clear out it's prior loaderData
Matches Remix behavior since
loaderDatais not reliable inerrorElementssince we haven't revalidated the data