Skip to content

Commit 67518cb

Browse files
dadamssgdadamssg
authored
Remove unnecessary hasShouldRevalidate condition for opting out (#15012)
1 parent 6f18edd commit 67518cb

4 files changed

Lines changed: 93 additions & 11 deletions

File tree

‎integration/single-fetch-test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,96 @@ test.describe("single-fetch", () => {
10631063
expect(urls).toEqual([expect.stringMatching(/\/action\.data$/)]);
10641064
});
10651065

1066+
test("call-site revalidation opt-out handles parent routes w/o shouldRevalidate", async ({
1067+
page,
1068+
}) => {
1069+
let fixture = await createFixture({
1070+
files: {
1071+
"app/root.tsx": js`
1072+
import { Link, Links, Meta, Outlet, Scripts, useMatches } from "react-router";
1073+
1074+
let count = 0
1075+
1076+
export function loader() {
1077+
return { count: ++count };
1078+
}
1079+
1080+
export default function Root() {
1081+
return (
1082+
<html lang="en">
1083+
<head>
1084+
<Meta />
1085+
<Links />
1086+
</head>
1087+
<body>
1088+
<nav>
1089+
<Link to="/">Home</Link>
1090+
<Link to="/page">Page (default)</Link>
1091+
<Link to="/page?optout" unstable_defaultShouldRevalidate={false}>Page (opt-out)</Link>
1092+
</nav>
1093+
<pre id="data">
1094+
{JSON.stringify(useMatches().map(m => [m.id, m.data]))}
1095+
</pre>
1096+
<Outlet />
1097+
<Scripts />
1098+
</body>
1099+
</html>
1100+
);
1101+
}
1102+
`,
1103+
"app/routes/page.tsx": js`
1104+
let count = 0
1105+
1106+
export function loader() {
1107+
return { count: ++count }
1108+
}
1109+
1110+
export default function Component() {
1111+
return <h2>Page</h2>
1112+
}
1113+
`,
1114+
},
1115+
});
1116+
1117+
let urls: string[] = [];
1118+
page.on("request", (req) => {
1119+
if (req.url().includes(".data")) {
1120+
let url = new URL(req.url());
1121+
urls.push(url.pathname + url.search);
1122+
}
1123+
});
1124+
1125+
console.error = () => {};
1126+
1127+
let appFixture = await createAppFixture(fixture);
1128+
let app = new PlaywrightFixture(appFixture, page);
1129+
await app.goto("/"); // root increments to 1
1130+
expect(await page.locator("#data").innerText()).toBe(
1131+
'[["root",{"count":1}],["routes/_index",null]]',
1132+
);
1133+
1134+
await app.clickLink("/page"); // root increments to 2
1135+
expect(await page.locator("#data").innerText()).toBe(
1136+
'[["root",{"count":2}],["routes/page",{"count":1}]]',
1137+
);
1138+
expect(urls).toEqual(["/page.data"]);
1139+
urls.splice(0, urls.length);
1140+
1141+
await app.clickLink("/"); // root increments to 3
1142+
expect(await page.locator("#data").innerText()).toBe(
1143+
'[["root",{"count":3}],["routes/_index",null]]',
1144+
);
1145+
expect(urls).toEqual(["/_root.data"]);
1146+
urls.splice(0, urls.length);
1147+
1148+
await app.clickLink("/page?optout");
1149+
// root stays at 3, page is a fresh load so increments to 2
1150+
expect(await page.locator("#data").innerText()).toBe(
1151+
'[["root",{"count":3}],["routes/page",{"count":2}]]',
1152+
);
1153+
expect(urls).toEqual(["/page.data?optout=&_routes=routes%2Fpage"]);
1154+
});
1155+
10661156
test("returns headers correctly for singular loader and action calls", async () => {
10671157
let fixture = await createFixture({
10681158
files: {

‎packages/react-router/.changes/patch.fixes-bug-when-unstabledefaultshouldrevalidatefalse-used-matched.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a bug with `unstable_defaultShouldRevalidate={false}` where parent routes that did not export a `shouldRevalidate` function could be incorrectly included in the single fetch call for new child route data

‎packages/react-router/lib/dom/ssr/single-fetch.tsx‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ export function StreamTransfer({
166166
type GetRouteInfoFunction = (match: DataRouteMatch) => {
167167
hasLoader: boolean;
168168
hasClientLoader: boolean;
169-
hasShouldRevalidate: boolean;
170169
};
171170

172171
type ShouldAllowOptOutFunction = (match: DataRouteMatch) => boolean;
@@ -192,11 +191,9 @@ export function getTurboStreamSingleFetchDataStrategy(
192191
(match: DataRouteMatch) => {
193192
let manifestRoute = manifest.routes[match.route.id];
194193
invariant(manifestRoute, "Route not found in manifest");
195-
let routeModule = routeModules[match.route.id];
196194
return {
197195
hasLoader: manifestRoute.hasLoader,
198196
hasClientLoader: manifestRoute.hasClientLoader,
199-
hasShouldRevalidate: Boolean(routeModule?.shouldRevalidate),
200197
};
201198
},
202199
fetchAndDecodeViaTurboStream,
@@ -415,8 +412,7 @@ async function singleFetchLoaderNavigationStrategy(
415412
m.resolve(async (handler) => {
416413
routeDfds[i].resolve();
417414
let routeId = m.route.id;
418-
let { hasLoader, hasClientLoader, hasShouldRevalidate } =
419-
getRouteInfo(m);
415+
let { hasLoader, hasClientLoader } = getRouteInfo(m);
420416

421417
let defaultShouldRevalidate =
422418
!m.shouldRevalidateArgs ||
@@ -428,8 +424,7 @@ async function singleFetchLoaderNavigationStrategy(
428424
// If this route opted out, don't include in the .data request
429425
foundOptOutRoute ||=
430426
m.shouldRevalidateArgs != null && // This is a revalidation,
431-
hasLoader && // for a route with a server loader,
432-
hasShouldRevalidate === true; // and a shouldRevalidate function
427+
hasLoader; // for a route with a server loader
433428
return;
434429
}
435430

‎packages/react-router/lib/rsc/browser.tsx‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,6 @@ export function getRSCSingleFetchDataStrategy(
460460
hasComponent: boolean;
461461
hasAction: boolean;
462462
hasClientAction: boolean;
463-
hasShouldRevalidate: boolean;
464463
};
465464
};
466465

@@ -475,7 +474,6 @@ export function getRSCSingleFetchDataStrategy(
475474
hasComponent: M.route.hasComponent,
476475
hasAction: M.route.hasAction,
477476
hasClientAction: M.route.hasClientAction,
478-
hasShouldRevalidate: M.route.hasShouldRevalidate,
479477
};
480478
},
481479
// pass map into fetchAndDecode so it can add payloads
@@ -889,7 +887,6 @@ type DataRouteObjectWithManifestInfo = DataRouteObject & {
889887
hasClientLoader: boolean;
890888
hasAction: boolean;
891889
hasClientAction: boolean;
892-
hasShouldRevalidate: boolean;
893890
};
894891

895892
function createRouteFromServerManifest(
@@ -976,7 +973,6 @@ function createRouteFromServerManifest(
976973
hasClientLoader: match.clientLoader != null,
977974
hasAction: match.hasAction,
978975
hasClientAction: match.clientAction != null,
979-
hasShouldRevalidate: match.shouldRevalidate != null,
980976
};
981977

982978
if (typeof dataRoute.loader === "function") {

0 commit comments

Comments
 (0)