fix: pass skipRevalidation option through middleware pipeline in singleFetchAction

[joseph0926]

Note on Middleware Selection

Per the contributing guidelines, all code changes must target the dev branch. The dev branch uses v8_middleware (whereas main uses unstable_middleware), so I implemented and tested the fix against v8_middleware to comply with the guidelines. The underlying bug reproduces identically on both unstable_middleware and v8_middleware; this PR fixes the shared path by correctly forwarding skipRevalidation through generateMiddlewareResponse.

Description

Fixes a regression where shouldRevalidate: false is ignored when v8_middleware is enabled in SSR mode, causing unnecessary parent loader re-executions during child route navigation.

Problem

When using React Router v7.8.2 with

• ssr: true
• v8_middleware: true
• Parent route with shouldRevalidate: () => false

The parent loader is incorrectly re-executed on every child route navigation, despite shouldRevalidate returning false. This does not occur when v8_middleware: false.

Root Cause

After comparing v7.8.1 (working) and v7.8.2 (broken), I identified the issue in the middleware implementation

1. packages/react-router/lib/server-runtime/single-fetch.ts

   • The skipRevalidation: true option is set in singleFetchAction to prevent unnecessary loader re-executions
   • However, when generateMiddlewareResponse is provided (middleware enabled), this option is not forwarded to the inner query call

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

   • The generateMiddlewareResponse callback signature doesn't include skipRevalidation in its args type
   • The implementation checks opts?.skipRevalidation but it's always undefined because it's not passed through

Solution

Changes Made

1. packages/react-router/lib/server-runtime/single-fetch.ts

   generateMiddlewareResponse: build.future.v8_middleware
     ? async (query) => {
         try {
   -       let innerResult = await query(handlerRequest);
   +       let innerResult = await query(handlerRequest, {
   +         filterMatchesToLoad: (m) => !loadRouteIds || loadRouteIds.has(m.route.id),
   +         skipRevalidation: true
   +       });
           return handleQueryResult(innerResult);
         } catch (error) {
           return handleQueryError(error);
         }
       }
     : undefined,

2. Type definitions updated: to include skipRevalidation in the generateMiddlewareResponse callback signature

  generateMiddlewareResponse?: (
    query: (
      r: Request,
      args?: {
        filterMatchesToLoad?: (match: AgnosticDataRouteMatch) => boolean;
+        skipRevalidation?: boolean;
      },
    ) => Promise<StaticHandlerContext | Response>,
) => MaybePromise<Response>;

Test Plan

Added new integration test: integration/middleware-revalidate-test.ts

The test verifies

• Parent route with shouldRevalidate: () => false
• Child route navigation
• Parent loader execution count remains at 1 (initial load only)
• Test runs with both v8_middleware: false (passes) and v8_middleware: true (now passes with fix)

Test Results

• Without fix: v8_middleware: false passes, v8_middleware: true fails (parent loader called 4 times)
• With fix: Both configurations pass (parent loader called only once)

Breaking Changes

None. This is a bug fix that restores the intended behavior of shouldRevalidate: false when middleware is enabled.

Related Issues

Fixes #14257

[changeset-bot]

⚠️ No Changeset found

Latest commit: 66fd275

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[rossipedia]

This should address #14285 as well 👍

[timdorr]

Shouldn't the flag be unstable_middleware? The v8_middleware is a future flag, but hasn't yet been implemented.

[joseph0926]

Shouldn't the flag be unstable_middleware? The v8_middleware is a future flag, but hasn't yet been implemented.

@timdorr

You are correct
the current bug is related to unstable_middleware.
However, according to the contributing.md, it states

“All new features, bug-fixes, or anything that touches react-router code should be branched off of and merged into the dev branch.”

Therefore, I based my work on the dev branch. In the dev branch, the reactRouterConfig uses v8_middleware reference,
while in the main branch it uses unstable_middleware reference.

So in order to follow the contribution guidelines and run tests, I had no choice but to proceed with v8_middleware.
For reference, the same issue that appears with unstable_middleware can also be reproduced with v8_middleware.

[timdorr]

Oh, sorry, the search only looks at the default branch, so I missed that change on dev. Nevermind!

[joseph0926]

Should I add a changeset for this bug fix?
I noticed the CI is requesting one, but wanted to confirm since the Contributing documentation doesn't mention changesets and I'm unsure of the project's policy on this.

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[joseph0926]

1541ce0

improve test isolation and cleanup console.log handling

add beforeEach/afterEach hooks to properly save and restore console.log and remove unnecessary console output to keep CI logs clean

[rossipedia]

v8_middleware is the stabilized version of unstable_middleware. It's the same feature 👍

[brophdawg11]

Thank you for the PR @joseph0926! We can fix this in a slightly simpler manner by falling back on the internal filterMatchesToLoad (which was what we missed originally in #14154) so I made that change and collapsed your tests into the existing set of middleware tests.

[github-actions]

🤖 Hello there,

We just published version 7.9.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 7.9.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[foxlau]

shouldRevalidate in layout or root doesn't work as expected for nested routes

When visiting /servers/new, I expect the loader for the /servers route not to be triggered. However, using shouldRevalidate in the layout or root does not work as expected. It only works when shouldRevalidate is defined in the /servers route itself. I am using the official template: remix-run/react-router-templates/tree/main/node-custom-server. Does this mean multi-level nested routes are not supported, or am I missing something? 🤔 Requesting assistance.

// packages.json
"react-router": "^7.9.1",

// routes.ts
export default [
	index("routes/home.tsx"),
	route("auth/login", "routes/auth/login.tsx"),

	// Auth Protected
	layout("routes/layout.tsx", [
		route("overview", "routes/overview.tsx"),

		// Server
		route("servers", "routes/servers/list.tsx", [
			route("new", "routes/servers/new.tsx"),
			route(":id", "routes/servers/detail.tsx"),
		]),
	]),

	// 404 Not Found
	route("*", "routes/not-found.tsx"),
] satisfies RouteConfig;

// utils/misc.ts
import type { ShouldRevalidateFunction } from "react-router";

export const shouldRevalidate: ShouldRevalidateFunction = ({
	formMethod,
	currentUrl,
	nextUrl,
}) => {
	if (formMethod && formMethod !== "GET") return true;
	if (currentUrl.toString() === nextUrl.toString()) return true;
	return false;
};

// app/layout.tsx
import { requireAuth, requireUser } from "~/middlewares/auth";
import { shouldRevalidate } from "~/utils/misc";

export const middleware = [requireAuth];
export { shouldRevalidate }; // does not work as expected when visiting /servers/new

export async function loader() {
	return requireUser();
}

export default function AppLayout() {
	return (
		<div className="min-h-screen">
			<Outlet />
		</div>
	);
}

// app/servers/list.tsx
import { shouldRevalidate } from "~/utils/misc";

export { shouldRevalidate }; // shouldRevalidate works as expected

export async function loader(_: Route.LoaderArgs) {
  const servers = await getServers();
	return { servers };
}

export default function AppLayout({ loaderData }: Route.ComponentProps) {
	return (
		<div>
		      {JSON.stringify(servers)}
		      <Link to="/servers/new">New server</Link>
		      <Outlet />
		</div>
	);
}

Is this a limitation of multi-level nested routes, or is there something I'm overlooking in the configuration? Any guidance would be appreciated!

[brophdawg11]

Can you open an issue with a minimal reproduction and we can take a look?

[joseph0926]

@foxlau

As I understand it, your comment implies that in a nested route structure, if shouldRevalidate returns false in the parent layout, the loader should not be called in the child layout even if shouldRevalidate is not explicitly set to false there. Is that correct?
For example, in your code

export default [
	index(“routes/home.tsx”), 
// Here, if shouldRevalidate returns false...
    layout(“routes/layout.tsx”, [
// Here, you expect it to work without explicitly returning false
		route(“servers”, “routes/servers/list.tsx”, [ 
            route(“new”, “routes/servers/new.tsx”),
            route(“:id”, “routes/servers/detail.tsx”),
        ]),
    ])
] satisfies RouteConfig;

However, the official documentation states
react-router v7(https://reactrouter.com/start/data/route-object#shouldrevalidate): A route loader is revalidated when...
remix v2(https://v2.remix.run/docs/route/should-revalidate/#shouldrevalidate): If you define this function on a route module, Remix will defer to your function on every navigation and every revalidation after an action is called.

Based on these statements, I'm guessing this is the intended behavior?

Also, in the code

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

Lines 5005 to 5017 in c1cdded

	function shouldRevalidateLoader(
	loaderMatch: AgnosticDataRouteMatch,
	arg: ShouldRevalidateFunctionArgs,
	) {
	if (loaderMatch.route.shouldRevalidate) {
	let routeChoice = loaderMatch.route.shouldRevalidate(arg);
	if (typeof routeChoice === "boolean") {
	return routeChoice;
	}
	}
	return arg.defaultShouldRevalidate;
	}

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

Line 4795 in c1cdded

let shouldLoad = shouldRevalidateLoader(match, shouldRevalidateArgs);

Looking at this, it doesn't seem like a design that affects propagation or sub-layouts... I'm not sure.

Also, it doesn't seem significantly related to the core issue of this PR—that the parent loader keeps getting called even when shouldRevalidate is false only when middleware is true.
Because I tested your scenario in v7.8.2 (where this fix wasn't applied) with unstable_middleware: false, and got the same result.

[rossipedia]

shouldRevalidate applies only to the route that it is defined in, and does not affect any other route (parent or child).