Disallow useLoaderData in errorElement (#9735)

.changeset/ninety-moles-bake.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/router": patch
+---
+
+Fix a few bugs where loader/action data wasn't properly cleared on errors

.changeset/rare-clouds-behave.md

@@ -0,0 +1,6 @@
+---
+"react-router": patch
+"@remix-run/router": patch
+---
+
+Prevent `useLoaderData` usage in `errorElement`

package.json

@@ -107,7 +107,7 @@
   },
   "filesize": {
     "packages/router/dist/router.umd.min.js": {
-      "none": "37 kB"
+      "none": "37.5 kB"
     },
     "packages/react-router/dist/react-router.production.min.js": {
       "none": "12.5 kB"

packages/react-router/__tests__/data-memory-router-test.tsx

@@ -2105,6 +2105,178 @@ describe("<DataMemoryRouter>", () => {
         </div>"
       `);
     });
+
+    it("does not allow loaderData usage in self-caught error boundaries", async () => {
+      let errorSpy = jest.spyOn(console, "error");
+
+      let { container } = render(
+        <DataMemoryRouter>
+          <Route path="/" element={<Layout />}>
+            <Route
+              path="foo"
+              loader={() => Promise.reject(new Error("Kaboom!"))}
+              element={<h1>Foo</h1>}
+              errorElement={<FooError />}
+            />
+          </Route>
+        </DataMemoryRouter>
+      );
+
+      function Layout() {
+        let navigation = useNavigation();
+        return (
+          <div>
+            <MemoryNavigate to="/foo">Link to Foo</MemoryNavigate>
+            <p>{navigation.state}</p>
+            <Outlet />
+          </div>
+        );
+      }
+
+      function FooError() {
+        let error = useRouteError();
+        let data = useLoaderData();
+        return (
+          <>
+            <p>
+              Foo Data:{data === undefined ? "undefined" : JSON.stringify(data)}
+            </p>
+            <p>Foo Error:{error.message}</p>
+          </>
+        );
+      }
+
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <div>
+            <a
+              href=\\"/foo\\"
+            >
+              Link to Foo
+            </a>
+            <p>
+              idle
+            </p>
+          </div>
+        </div>"
+      `);
+
+      fireEvent.click(screen.getByText("Link to Foo"));
+      await waitFor(() => screen.getByText("Foo Error:Kaboom!"));
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <div>
+            <a
+              href=\\"/foo\\"
+            >
+              Link to Foo
+            </a>
+            <p>
+              idle
+            </p>
+            <p>
+              Foo Data:
+              undefined
+            </p>
+            <p>
+              Foo Error:
+              Kaboom!
+            </p>
+          </div>
+        </div>"
+      `);
+
+      expect(errorSpy).toHaveBeenCalledWith(
+        "You cannot `useLoaderData` in an errorElement (routeId: 0-0)"
+      );
+      errorSpy.mockRestore();
+    });
+
+    it("does not allow useLoaderData usage in bubbled error boundaries", async () => {
+      let errorSpy = jest.spyOn(console, "error");
+
+      let { container } = render(
+        <DataMemoryRouter
+          hydrationData={{
+            loaderData: {
+              "0": "ROOT",
+            },
+          }}
+        >
+          <Route
+            path="/"
+            element={<Layout />}
+            loader={() => "ROOT"}
+            errorElement={<LayoutError />}
+          >
+            <Route
+              path="foo"
+              loader={() => Promise.reject(new Error("Kaboom!"))}
+              element={<h1>Foo</h1>}
+            />
+          </Route>
+        </DataMemoryRouter>
+      );
+
+      function Layout() {
+        let navigation = useNavigation();
+        return (
+          <div>
+            <MemoryNavigate to="/foo">Link to Foo</MemoryNavigate>
+            <p>{navigation.state}</p>
+            <Outlet />
+          </div>
+        );
+      }
+      function LayoutError() {
+        let data = useLoaderData();
+        let error = useRouteError();
+        return (
+          <>
+            <p>
+              Layout Data:
+              {data === undefined ? "undefined" : JSON.stringify(data)}
+            </p>
+            <p>Layout Error:{error.message}</p>
+          </>
+        );
+      }
+
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <div>
+            <a
+              href=\\"/foo\\"
+            >
+              Link to Foo
+            </a>
+            <p>
+              idle
+            </p>
+          </div>
+        </div>"
+      `);
+
+      fireEvent.click(screen.getByText("Link to Foo"));
+      await waitFor(() => screen.getByText("Layout Error:Kaboom!"));
+      expect(getHtml(container)).toMatchInlineSnapshot(`
+        "<div>
+          <p>
+            Layout Data:
+            undefined
+          </p>
+          <p>
+            Layout Error:
+            Kaboom!
+          </p>
+        </div>"
+      `);
+
+      expect(errorSpy).toHaveBeenCalledWith(
+        "You cannot `useLoaderData` in an errorElement (routeId: 0)"
+      );
+      errorSpy.mockRestore();
+    });
   });
 
   describe("defer", () => {

packages/react-router/lib/hooks.tsx

@@ -482,6 +482,7 @@ type RenderErrorBoundaryProps = React.PropsWithChildren<{
   location: Location;
   error: any;
   component: React.ReactNode;
+  routeContext: RouteContextObject;
 }>;
 
 type RenderErrorBoundaryState = {
@@ -544,10 +545,12 @@ export class RenderErrorBoundary extends React.Component<
 
   render() {
     return this.state.error ? (
-      <RouteErrorContext.Provider
-        value={this.state.error}
-        children={this.props.component}
-      />
+      <RouteContext.Provider value={this.props.routeContext}>
+        <RouteErrorContext.Provider
+          value={this.state.error}
+          children={this.props.component}
+        />
+      </RouteContext.Provider>
     ) : (
       this.props.children
     );
@@ -615,14 +618,9 @@ export function _renderMatches(
     let errorElement = dataRouterState
       ? match.route.errorElement || <DefaultErrorElement />
       : null;
+    let matches = parentMatches.concat(renderedMatches.slice(0, index + 1));
     let getChildren = () => (
-      <RenderedRoute
-        match={match}
-        routeContext={{
-          outlet,
-          matches: parentMatches.concat(renderedMatches.slice(0, index + 1)),
-        }}
-      >
+      <RenderedRoute match={match} routeContext={{ outlet, matches }}>
         {error
           ? errorElement
           : match.route.element !== undefined
@@ -639,6 +637,7 @@ export function _renderMatches(
         component={errorElement}
         error={error}
         children={getChildren()}
+        routeContext={{ outlet: null, matches }}
       />
     ) : (
       getChildren()
@@ -678,6 +677,22 @@ function useDataRouterState(hookName: DataRouterStateHook) {
   return state;
 }
 
+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;
+}
+
 /**
  * Returns the current navigation, defaulting to an "idle" navigation when
  * no navigation is in progress
@@ -732,17 +747,15 @@ export function useMatches() {
  */
 export function useLoaderData(): unknown {
   let state = useDataRouterState(DataRouterStateHook.UseLoaderData);
+  let routeId = useCurrentRouteId(DataRouterStateHook.UseLoaderData);
 
-  let route = React.useContext(RouteContext);
-  invariant(route, `useLoaderData must be used inside a RouteContext`);
-
-  let thisRoute = route.matches[route.matches.length - 1];
-  invariant(
-    thisRoute.route.id,
-    `useLoaderData can only be used on routes that contain a unique "id"`
-  );
-
-  return state.loaderData[thisRoute.route.id];
+  if (state.errors && state.errors[routeId] != null) {
+    console.error(
+      `You cannot \`useLoaderData\` in an errorElement (routeId: ${routeId})`
+    );
+    return undefined;
+  }
+  return state.loaderData[routeId];
 }
 
 /**
@@ -773,23 +786,16 @@ export function useActionData(): unknown {
 export function useRouteError(): unknown {
   let error = React.useContext(RouteErrorContext);
   let state = useDataRouterState(DataRouterStateHook.UseRouteError);
-  let route = React.useContext(RouteContext);
-  let thisRoute = route.matches[route.matches.length - 1];
+  let routeId = useCurrentRouteId(DataRouterStateHook.UseRouteError);
 
   // If this was a render error, we put it in a RouteError context inside
   // of RenderErrorBoundary
   if (error) {
     return error;
   }
 
-  invariant(route, `useRouteError must be used inside a RouteContext`);
-  invariant(
-    thisRoute.route.id,
-    `useRouteError can only be used on routes that contain a unique "id"`
-  );
-
   // Otherwise look for errors from our data router state
-  return state.errors?.[thisRoute.route.id];
+  return state.errors?.[routeId];
 }
 
 /**