diff --git a/.changeset/blue-masks-boil.md b/.changeset/blue-masks-boil.md new file mode 100644 index 0000000000..bdb9c7892a --- /dev/null +++ b/.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 `` flag to enable the proper `context` type diff --git a/packages/react-router/__tests__/dom/stub-test.tsx b/packages/react-router/__tests__/dom/stub-test.tsx index b0de8d060c..825349e6de 100644 --- a/packages/react-router/__tests__/dom/stub-test.tsx +++ b/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
Home
; + }, + }, + { + path: "/target", + Component() { + return
Target
; + }, + }, + ]); + + act(() => { + render(); + }); + + await waitFor(() => screen.findByText("Target")); +}); + +test("middleware works with loader", async () => { + let stringContext = createContext(); + let RoutesStub = createRoutesStub([ + { + path: "/", + HydrateFallback: () => null, + Component() { + let data = useLoaderData(); + return
Message: {data.message}
; + }, + middleware: [({ context }) => context.set(stringContext, "hello")], + loader({ context }) { + return { message: context.get(stringContext) }; + }, + }, + ]); + + render(); + + await waitFor(() => screen.findByText("Message: hello")); +}); + // eslint-disable-next-line jest/expect-expect test("loaders work", async () => { let RoutesStub = createRoutesStub([ diff --git a/packages/react-router/__tests__/router/router-test.ts b/packages/react-router/__tests__/router/router-test.ts index d1dc920151..704d7eff3f 100644 --- a/packages/react-router/__tests__/router/router-test.ts +++ b/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: [ diff --git a/packages/react-router/lib/dom/ssr/routes-test-stub.tsx b/packages/react-router/lib/dom/ssr/routes-test-stub.tsx index a41dd8612c..5dc5b2d25e 100644 --- a/packages/react-router/lib/dom/ssr/routes-test-stub.tsx +++ b/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) => + mw( + { ...args[0], context: context as RouterContextProvider }, + args[1], + ), + ) + : undefined, handle: route.handle, shouldRevalidate: route.shouldRevalidate, }; diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 2ab3040a8f..2c3e379345 100644 --- a/packages/react-router/lib/router/router.ts +++ b/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,