fix middleware when used with createRoutesStub (#14348)

Author: brophdawg11

.changeset/blue-masks-boil.md

@@ -0,0 +1,7 @@
+---
+"react-router": patch
+---
+
+- Update client-side router to run client `middleware` on initial load even if no loaders exist
+- Update `createRoutesStub` to run route middleware
+  - You will need to set the `<RoutesStub future={{ v8_middleware: true }} />` flag to enable the proper `context` type

packages/react-router/__tests__/dom/stub-test.tsx

@@ -1,5 +1,5 @@
 import * as React from "react";
-import { render, screen, waitFor } from "@testing-library/react";
+import { act, render, screen, waitFor } from "@testing-library/react";
 import user from "@testing-library/user-event";
 import {
   Form,
@@ -12,7 +12,11 @@ import {
   type LoaderFunctionArgs,
   useRouteError,
 } from "../../index";
-import { RouterContextProvider, createContext } from "../../lib/router/utils";
+import {
+  RouterContextProvider,
+  createContext,
+  redirect,
+} from "../../lib/router/utils";
 
 test("renders a route", () => {
   let RoutesStub = createRoutesStub([
@@ -53,6 +57,57 @@ test("renders a nested route", () => {
   expect(screen.getByText("INDEX")).toBeInTheDocument();
 });
 
+test("middleware works without loader", async () => {
+  let RoutesStub = createRoutesStub([
+    {
+      path: "/",
+      middleware: [
+        () => {
+          throw redirect("/target");
+        },
+      ],
+      HydrateFallback: () => null,
+      Component() {
+        return <pre>Home</pre>;
+      },
+    },
+    {
+      path: "/target",
+      Component() {
+        return <pre>Target</pre>;
+      },
+    },
+  ]);
+
+  act(() => {
+    render(<RoutesStub future={{ v8_middleware: true }} />);
+  });
+
+  await waitFor(() => screen.findByText("Target"));
+});
+
+test("middleware works with loader", async () => {
+  let stringContext = createContext<string>();
+  let RoutesStub = createRoutesStub([
+    {
+      path: "/",
+      HydrateFallback: () => null,
+      Component() {
+        let data = useLoaderData();
+        return <pre data-testid="data">Message: {data.message}</pre>;
+      },
+      middleware: [({ context }) => context.set(stringContext, "hello")],
+      loader({ context }) {
+        return { message: context.get(stringContext) };
+      },
+    },
+  ]);
+
+  render(<RoutesStub future={{ v8_middleware: true }} />);
+
+  await waitFor(() => screen.findByText("Message: hello"));
+});
+
 // eslint-disable-next-line jest/expect-expect
 test("loaders work", async () => {
   let RoutesStub = createRoutesStub([

packages/react-router/__tests__/router/router-test.ts

@@ -936,6 +936,42 @@ describe("a router", () => {
       });
     });
 
+    it("does not run middlewares when complete hydrationData exists", async () => {
+      let middlewareSpy = jest.fn();
+      let loaderSpy = jest.fn();
+      let router = createRouter({
+        history: createMemoryHistory(),
+        routes: [
+          {
+            id: "index",
+            path: "/",
+            middleware: [middlewareSpy],
+            loader: loaderSpy,
+          },
+        ],
+        hydrationData: {
+          loaderData: {
+            index: "INDEX DATA",
+          },
+        },
+      });
+      router.initialize();
+
+      expect(router.state).toMatchObject({
+        historyAction: "POP",
+        location: {
+          pathname: "/",
+        },
+        initialized: true,
+        navigation: IDLE_NAVIGATION,
+        loaderData: {
+          index: "INDEX DATA",
+        },
+      });
+      expect(middlewareSpy).not.toHaveBeenCalled();
+      expect(loaderSpy).not.toHaveBeenCalled();
+    });
+
     it("kicks off initial data load if no hydration data is provided", async () => {
       let parentDfd = createDeferred();
       let parentSpy = jest.fn(() => parentDfd.promise);
@@ -993,6 +1029,61 @@ describe("a router", () => {
       router.dispose();
     });
 
+    it("run middlewares without loaders on initial load if no hydration data is provided", async () => {
+      let parentDfd = createDeferred();
+      let parentSpy = jest.fn(() => parentDfd.promise);
+      let childDfd = createDeferred();
+      let childSpy = jest.fn(() => childDfd.promise);
+      let router = createRouter({
+        history: createMemoryHistory(),
+        routes: [
+          {
+            path: "/",
+            middleware: [parentSpy],
+            children: [
+              {
+                index: true,
+                middleware: [childSpy],
+              },
+            ],
+          },
+        ],
+      });
+      router.initialize();
+      await tick();
+
+      expect(console.warn).not.toHaveBeenCalled();
+      expect(parentSpy.mock.calls.length).toBe(1);
+      expect(childSpy.mock.calls.length).toBe(0);
+      expect(router.state).toMatchObject({
+        historyAction: "POP",
+        location: expect.objectContaining({ pathname: "/" }),
+        initialized: false,
+        navigation: IDLE_NAVIGATION,
+      });
+      expect(router.state.loaderData).toEqual({});
+
+      await parentDfd.resolve(undefined);
+      expect(router.state).toMatchObject({
+        historyAction: "POP",
+        location: expect.objectContaining({ pathname: "/" }),
+        initialized: false,
+        navigation: IDLE_NAVIGATION,
+      });
+      expect(router.state.loaderData).toEqual({});
+
+      await childDfd.resolve(undefined);
+      expect(router.state).toMatchObject({
+        historyAction: "POP",
+        location: expect.objectContaining({ pathname: "/" }),
+        initialized: true,
+        navigation: IDLE_NAVIGATION,
+        loaderData: {},
+      });
+
+      router.dispose();
+    });
+
     it("allows routes to be initialized with undefined loaderData", async () => {
       let t = setup({
         routes: [

packages/react-router/lib/dom/ssr/routes-test-stub.tsx

@@ -4,6 +4,7 @@ import type {
   ActionFunctionArgs,
   LoaderFunction,
   LoaderFunctionArgs,
+  MiddlewareFunction,
 } from "../../router/utils";
 import type {
   DataRouteObject,
@@ -209,6 +210,16 @@ function processRoutes(
       loader: route.loader
         ? (args: LoaderFunctionArgs) => route.loader!({ ...args, context })
         : undefined,
+      middleware: route.middleware
+        ? route.middleware.map(
+            (mw) =>
+              (...args: Parameters<MiddlewareFunction>) =>
+                mw(
+                  { ...args[0], context: context as RouterContextProvider },
+                  args[1],
+                ),
+          )
+        : undefined,
       handle: route.handle,
       shouldRevalidate: route.shouldRevalidate,
     };

packages/react-router/lib/router/router.ts

@@ -959,8 +959,10 @@ export function createRouter(init: RouterInit): Router {
       // All initialMatches need to be loaded before we're ready.  If we have lazy
       // functions around still then we'll need to run them in initialize()
       initialized = false;
-    } else if (!initialMatches.some((m) => m.route.loader)) {
-      // If we've got no loaders to run, then we're good to go
+    } else if (
+      !initialMatches.some((m) => routeHasLoaderOrMiddleware(m.route))
+    ) {
+      // If we've got no loaders or middleware to run, then we're good to go
       initialized = true;
     } else {
       // With "partial hydration", we're initialized so long as we were
@@ -2092,7 +2094,9 @@ export function createRouter(init: RouterInit): Router {
     if (
       !init.dataStrategy &&
       !dsMatches.some((m) => m.shouldLoad) &&
-      !dsMatches.some((m) => m.route.middleware) &&
+      !dsMatches.some(
+        (m) => m.route.middleware && m.route.middleware.length > 0,
+      ) &&
       revalidatingFetchers.length === 0
     ) {
       let updatedFetchers = markFetchRedirectsDone();
@@ -4763,7 +4767,7 @@ function getMatchesToLoad(
     } else if (route.lazy) {
       // We haven't loaded this route yet so we don't know if it's got a loader!
       forceShouldLoad = true;
-    } else if (route.loader == null) {
+    } else if (!routeHasLoaderOrMiddleware(route)) {
       // Nothing to load!
       forceShouldLoad = false;
     } else if (initialHydration) {
@@ -4950,6 +4954,13 @@ function getMatchesToLoad(
   return { dsMatches, revalidatingFetchers };
 }
 
+function routeHasLoaderOrMiddleware(route: RouteObject) {
+  return (
+    route.loader != null ||
+    (route.middleware != null && route.middleware.length > 0)
+  );
+}
+
 function shouldLoadRouteOnHydration(
   route: AgnosticDataRouteObject,
   loaderData: RouteData | null | undefined,
@@ -4960,8 +4971,8 @@ function shouldLoadRouteOnHydration(
     return true;
   }
 
-  // No loader, nothing to initialize
-  if (!route.loader) {
+  // No loader or middleware, nothing to run
+  if (!routeHasLoaderOrMiddleware(route)) {
     return false;
   }
 
@@ -5519,9 +5530,15 @@ function runClientMiddlewarePipeline(
       let { matches } = args;
       let maxBoundaryIdx = Math.min(
         // Throwing route
-        matches.findIndex((m) => m.route.id === routeId) || 0,
+        Math.max(
+          matches.findIndex((m) => m.route.id === routeId),
+          0,
+        ),
         // or the shallowest route that needs to load data
-        matches.findIndex((m) => m.unstable_shouldCallHandler()) || 0,
+        Math.max(
+          matches.findIndex((m) => m.unstable_shouldCallHandler()),
+          0,
+        ),
       );
       let boundaryRouteId = findNearestBoundary(
         matches,