Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/blue-masks-boil.md
Original file line number Diff line number Diff line change
@@ -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
59 changes: 57 additions & 2 deletions packages/react-router/__tests__/dom/stub-test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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([
Expand Down Expand Up @@ -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([
Expand Down
91 changes: 91 additions & 0 deletions packages/react-router/__tests__/router/router-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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: [
Expand Down
11 changes: 11 additions & 0 deletions packages/react-router/lib/dom/ssr/routes-test-stub.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
ActionFunctionArgs,
LoaderFunction,
LoaderFunctionArgs,
MiddlewareFunction,
} from "../../router/utils";
import type {
DataRouteObject,
Expand Down Expand Up @@ -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,
};
Expand Down
33 changes: 25 additions & 8 deletions packages/react-router/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

Expand Down Expand Up @@ -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,
Expand Down