From 23a82df638fcc24822da458769123f82d9a9fa8d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 28 Apr 2025 11:40:28 -0400 Subject: [PATCH] Fix cleared loaderData bug on thrown action errors --- .changeset/hip-laws-teach.md | 5 ++ .../router/should-revalidate-test.ts | 62 +++++++++++++++++++ packages/react-router/lib/router/router.ts | 24 +++++-- 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 .changeset/hip-laws-teach.md diff --git a/.changeset/hip-laws-teach.md b/.changeset/hip-laws-teach.md new file mode 100644 index 0000000000..eed1726d2f --- /dev/null +++ b/.changeset/hip-laws-teach.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix bug where bubbled action errors would result in `loaderData` being cleared at the handling `ErrorBoundary` route diff --git a/packages/react-router/__tests__/router/should-revalidate-test.ts b/packages/react-router/__tests__/router/should-revalidate-test.ts index 5ee006bfea..de35fab2d0 100644 --- a/packages/react-router/__tests__/router/should-revalidate-test.ts +++ b/packages/react-router/__tests__/router/should-revalidate-test.ts @@ -1170,4 +1170,66 @@ describe("shouldRevalidate", () => { router.dispose(); }); + + it("preserves ancestor loaderData on bubbled action errors when not revalidating with a custom data strategy", async () => { + let history = createMemoryHistory(); + let router = createRouter({ + history, + routes: [ + { + id: "root", + path: "/", + hasErrorBoundary: true, + loader: () => "NOPE", + children: [ + { + id: "index", + index: true, + action: () => { + throw new Response("ERROR 400", { status: 400 }); + }, + }, + ], + }, + ], + hydrationData: { + loaderData: { + root: "ROOT", + }, + }, + async dataStrategy({ request, matches }) { + let keyedResults = {}; + let matchesToLoad = matches.filter((match) => + match.unstable_shouldCallHandler( + request.method === "POST" + ? undefined + : !match.unstable_shouldRevalidateArgs?.actionStatus || + match.unstable_shouldRevalidateArgs.actionStatus < 400 + ) + ); + await Promise.all( + matchesToLoad.map(async (match) => { + keyedResults[match.route.id] = await match.resolve(); + }) + ); + return keyedResults; + }, + }); + router.initialize(); + + router.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), + }); + await tick(); + expect(router.state).toMatchObject({ + location: { pathname: "/" }, + navigation: { state: "idle" }, + loaderData: { root: "ROOT" }, + actionData: null, + errors: { root: new ErrorResponseImpl(400, "", "ERROR 400") }, + }); + + router.dispose(); + }); }); diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index b5585cb978..728ed73d2d 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -686,7 +686,15 @@ interface ShortCircuitable { shortCircuited?: boolean; } -type PendingActionResult = [string, SuccessResult | ErrorResult]; +// Track any pending errors from the action (or other pre-loader flows). +// The format is [bubbledRouteId, result, actionRouteId?] +// We should probably change this to an object now that we (optionally) track +// the original action route id so we can clear out loaderData in the right in +// processRouteLoaderData +// +type PendingActionResult = + | [string, SuccessResult | ErrorResult] + | [string, SuccessResult | ErrorResult, string]; interface HandleActionResult extends ShortCircuitable { /** @@ -1858,7 +1866,11 @@ export function createRouter(init: RouterInit): Router { return { matches, - pendingActionResult: [boundaryMatch.route.id, result], + pendingActionResult: [ + boundaryMatch.route.id, + result, + actionMatch.route.id, + ], }; } @@ -6047,11 +6059,13 @@ function processRouteLoaderData( }); // If we didn't consume the pending action error (i.e., all loaders - // resolved), then consume it here. Also clear out any loaderData for the - // throwing route + // resolved), then consume it here if (pendingError !== undefined && pendingActionResult) { errors = { [pendingActionResult[0]]: pendingError }; - loaderData[pendingActionResult[0]] = undefined; + // Clear out any loaderData for the throwing route + if (pendingActionResult[2]) { + loaderData[pendingActionResult[2]] = undefined; + } } return {