Skip to content

Commit 87ad3c8

Browse files
joseph0926joseph0926
committed
fix: pass skipRevalidation option through middleware pipeline in singleFetchAction
1 parent 23b62b6 commit 87ad3c8

File tree

4 files changed

+216
-4
lines changed

4 files changed

+216
-4
lines changed

‎contributors.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@
187187
- jmjpro
188188
- johnpangalos
189189
- jonkoops
190+
- joseph0926
190191
- jrakotoharisoa
191192
- jrestall
192193
- juanpprieto

‎integration/middleware-revaildate-test.ts‎

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
import { test, expect } from "@playwright/test";
2+
3+
import {
4+
createAppFixture,
5+
createFixture,
6+
js,
7+
} from "./helpers/create-fixture.js";
8+
import { PlaywrightFixture } from "./helpers/playwright-fixture.js";
9+
import { reactRouterConfig } from "./helpers/vite.js";
10+
11+
const files = {
12+
"react-router.config.ts": reactRouterConfig({
13+
ssr: true,
14+
v8_middleware: false,
15+
}),
16+
"app/root.tsx": js`
17+
import { Links, Meta, Outlet, Scripts, redirect } from "react-router";
18+
19+
export function loader({ params }) {
20+
if (!params.child) {
21+
throw redirect("/parent/1");
22+
}
23+
return null;
24+
}
25+
26+
export default function Root() {
27+
return (
28+
<html lang="en">
29+
<head>
30+
<Meta />
31+
<Links />
32+
</head>
33+
<body>
34+
<Outlet />
35+
<Scripts />
36+
</body>
37+
</html>
38+
);
39+
}
40+
`,
41+
42+
"app/routes/parent.tsx": js`
43+
import { Outlet, useLoaderData } from "react-router";
44+
45+
export const shouldRevalidate = () => {
46+
console.log("[PARENT] shouldRevalidate called - returning false");
47+
return false;
48+
};
49+
50+
let loaderCallCount = 0;
51+
52+
export function loader() {
53+
loaderCallCount++;
54+
console.log("[PARENT_LOADER] Called: " + loaderCallCount + " times");
55+
return {
56+
data: "parent data",
57+
callCount: loaderCallCount,
58+
timestamp: Date.now(),
59+
};
60+
}
61+
62+
export default function Parent() {
63+
const parentData = useLoaderData();
64+
if (!parentData) throw new Error("no parent data");
65+
66+
return (
67+
<div style={{ padding: "20px" }}>
68+
<h1>Parent</h1>
69+
<div id="parent-loader-count" data-count={parentData.callCount}>
70+
Parent Loader Count: {parentData.callCount}
71+
</div>
72+
<div id="parent-timestamp" data-timestamp={parentData.timestamp}>
73+
Parent Timestamp: {parentData.timestamp}
74+
</div>
75+
<Outlet />
76+
</div>
77+
);
78+
}
79+
`,
80+
81+
"app/routes/parent.$child.tsx": js`
82+
import { href, Link, useParams, useLoaderData } from "react-router";
83+
84+
export const shouldRevalidate = () => {
85+
console.log("[CHILD] shouldRevalidate called - returning true");
86+
return true;
87+
};
88+
89+
let loaderCallCount = 0;
90+
91+
export function loader({ params }) {
92+
loaderCallCount++;
93+
console.log("[CHILD_LOADER] Child " + params.child + " - Called: " + loaderCallCount + " times");
94+
return {
95+
data: "child data",
96+
callCount: loaderCallCount,
97+
childNumber: params.child,
98+
};
99+
}
100+
101+
export default function Child() {
102+
const childData = useLoaderData();
103+
if (!childData) throw new Error("no child data");
104+
105+
const params = useParams();
106+
const nextChild = Number(params.child) + 1;
107+
108+
return (
109+
<div style={{ padding: "20px" }}>
110+
<h1>Child: {params.child}</h1>
111+
<div id="child-loader-count" data-count={childData.callCount}>
112+
Child Loader Count: {childData.callCount}
113+
</div>
114+
<Link
115+
id="next-child-link"
116+
style={{
117+
border: "1px solid #CCC",
118+
marginTop: "20px",
119+
display: "inline-block",
120+
padding: "4px 8px",
121+
}}
122+
to={href("/parent/:child", {
123+
child: nextChild.toString(),
124+
})}
125+
>
126+
Go to next child
127+
</Link>
128+
</div>
129+
);
130+
}
131+
`,
132+
};
133+
134+
test.describe("shouldRevalidate with middleware", () => {
135+
test("v8_middleware false - parent loader called once", async ({ page }) => {
136+
let fixture = await createFixture({
137+
files,
138+
});
139+
let appFixture = await createAppFixture(fixture);
140+
let app = new PlaywrightFixture(appFixture, page);
141+
142+
await app.goto("/parent/1");
143+
await app.clickLink("/parent/2");
144+
await app.clickLink("/parent/3");
145+
await app.clickLink("/parent/4");
146+
147+
const initialParentCount = await page
148+
.locator("#parent-loader-count")
149+
.getAttribute("data-count");
150+
expect(initialParentCount).toBe("1");
151+
152+
const secondParentCount = await page
153+
.locator("#parent-loader-count")
154+
.getAttribute("data-count");
155+
156+
expect(secondParentCount).toBe("1");
157+
158+
const thirdParentCount = await page
159+
.locator("#parent-loader-count")
160+
.getAttribute("data-count");
161+
162+
expect(thirdParentCount).toBe("1");
163+
});
164+
165+
test("v8_middleware true - server execution tracking", async ({ page }) => {
166+
const filesWithMiddleware = {
167+
...files,
168+
"react-router.config.ts": reactRouterConfig({
169+
ssr: true,
170+
v8_middleware: true,
171+
}),
172+
};
173+
174+
let fixture = await createFixture({
175+
files: filesWithMiddleware,
176+
});
177+
let appFixture = await createAppFixture(fixture);
178+
let app = new PlaywrightFixture(appFixture, page);
179+
180+
const parentLoaderRegex = /\[PARENT_LOADER\] Called: (\d+) times/;
181+
let maxParentLoaderCount = 0;
182+
183+
const originalLog = console.log;
184+
console.log = (...args) => {
185+
const message = args.join(" ");
186+
const match = message.match(parentLoaderRegex);
187+
if (match) {
188+
maxParentLoaderCount = Math.max(
189+
maxParentLoaderCount,
190+
parseInt(match[1]),
191+
);
192+
}
193+
originalLog(...args);
194+
};
195+
196+
await app.goto("/parent/1");
197+
await app.clickLink("/parent/2");
198+
await app.clickLink("/parent/3");
199+
await app.clickLink("/parent/4");
200+
201+
expect(maxParentLoaderCount).toBe(1);
202+
});
203+
});

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ export interface StaticHandler {
437437
r: Request,
438438
args?: {
439439
filterMatchesToLoad?: (match: AgnosticDataRouteMatch) => boolean;
440+
skipRevalidation?: boolean;
440441
},
441442
) => Promise<StaticHandlerContext | Response>,
442443
) => MaybePromise<Response>;
@@ -3557,7 +3558,7 @@ export function createStaticHandler(
35573558
skipLoaderErrorBubbling,
35583559
skipRevalidation,
35593560
dataStrategy,
3560-
generateMiddlewareResponse: generateMiddlewareResponse,
3561+
generateMiddlewareResponse,
35613562
}: Parameters<StaticHandler["query"]>[1] = {},
35623563
): Promise<StaticHandlerContext | Response> {
35633564
let url = new URL(request.url);
@@ -3625,6 +3626,7 @@ export function createStaticHandler(
36253626
// redirects from loaders after a server action, etc.
36263627
// - If we decide this isn't warranted than the stream implementation
36273628
// can simplify and potentially even collapse back into respond()
3629+
36283630
if (generateMiddlewareResponse) {
36293631
invariant(
36303632
requestContext instanceof RouterContextProvider,
@@ -3655,6 +3657,7 @@ export function createStaticHandler(
36553657
filterMatchesToLoad?:
36563658
| ((match: AgnosticDataRouteMatch) => boolean)
36573659
| undefined;
3660+
skipRevalidation?: boolean;
36583661
} = {},
36593662
) => {
36603663
let result = await queryImpl(
@@ -3668,7 +3671,7 @@ export function createStaticHandler(
36683671
"filterMatchesToLoad" in opts
36693672
? (opts.filterMatchesToLoad ?? null)
36703673
: null,
3671-
skipRevalidation === true,
3674+
opts?.skipRevalidation ?? skipRevalidation === true,
36723675
);
36733676

36743677
if (isResponse(result)) {
@@ -3831,7 +3834,7 @@ export function createStaticHandler(
38313834
routeId,
38323835
requestContext,
38333836
dataStrategy,
3834-
generateMiddlewareResponse: generateMiddlewareResponse,
3837+
generateMiddlewareResponse,
38353838
}: Parameters<StaticHandler["queryRoute"]>[1] = {},
38363839
): Promise<any> {
38373840
let url = new URL(request.url);

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,15 @@ export async function singleFetchLoaders(
144144
requestContext: loadContext,
145145
filterMatchesToLoad: (m) => !loadRouteIds || loadRouteIds.has(m.route.id),
146146
skipLoaderErrorBubbling: true,
147+
skipRevalidation: true,
147148
generateMiddlewareResponse: build.future.v8_middleware
148149
? async (query) => {
149150
try {
150-
let innerResult = await query(handlerRequest);
151+
let innerResult = await query(handlerRequest, {
152+
filterMatchesToLoad: (m) =>
153+
!loadRouteIds || loadRouteIds.has(m.route.id),
154+
skipRevalidation: true,
155+
});
151156
return handleQueryResult(innerResult);
152157
} catch (error) {
153158
return handleQueryError(error);

0 commit comments

Comments
 (0)