Skip to content

Commit 433872f

Browse files
markdalgleishmarkdalgleish
authored
Force revalidation of RSC routes with errors (#14026)
1 parent 3fdce65 commit 433872f

File tree

3 files changed

+142
-2
lines changed

3 files changed

+142
-2
lines changed

‎.changeset/kind-ways-notice.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
In RSC Data Mode, fix bug where routes with errors weren't forced to revalidate when `shouldRevalidate` returned false

‎integration/rsc/rsc-test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,6 +2207,134 @@ implementations.forEach((implementation) => {
22072207
// Ensure this is using RSC
22082208
validateRSCHtml(await page.content());
22092209
});
2210+
2211+
test("Forces revalidation of routes with errors", async ({ page }) => {
2212+
let port = await getPort();
2213+
stop = await setupRscTest({
2214+
implementation,
2215+
port,
2216+
dev: true,
2217+
files: {
2218+
"src/routes.ts": js`
2219+
import type { unstable_RSCRouteConfig as RSCRouteConfig } from "react-router";
2220+
2221+
export const routes = [
2222+
{
2223+
id: "root",
2224+
path: "",
2225+
lazy: () => import("./routes/root"),
2226+
children: [
2227+
{
2228+
id: "index",
2229+
index: true,
2230+
lazy: () => import("./routes/index"),
2231+
},
2232+
{
2233+
id: "other",
2234+
path: "other",
2235+
lazy: () => import("./routes/other"),
2236+
},
2237+
],
2238+
},
2239+
] satisfies RSCRouteConfig;
2240+
`,
2241+
"src/routes/root.tsx": js`
2242+
import { Outlet, Link } from "react-router";
2243+
2244+
export { shouldRevalidate } from "./root.client";
2245+
2246+
let loaderCallCount = 0;
2247+
2248+
export function loader() {
2249+
loaderCallCount++;
2250+
throw new Error("Root loader error (call #" + loaderCallCount + ")");
2251+
}
2252+
2253+
export function Layout({ children }: { children: React.ReactNode }) {
2254+
return (
2255+
<html>
2256+
<body>
2257+
<ul>
2258+
<li><Link to="/" data-link-index>Index route</Link></li>
2259+
<li><Link to="/other" data-link-other>Other route</Link></li>
2260+
</ul>
2261+
{children}
2262+
</body>
2263+
</html>
2264+
);
2265+
}
2266+
2267+
export function ErrorBoundary({ error }) {
2268+
return (
2269+
<div>
2270+
<h1 data-error-boundary-loader-call-count={loaderCallCount}>
2271+
Root ErrorBoundary (loaderCallCount: {loaderCallCount})
2272+
</h1>
2273+
</div>
2274+
);
2275+
}
2276+
2277+
export default function RootRoute() {
2278+
return (
2279+
<div>
2280+
<h1>Root Route</h1>
2281+
<p>This should never be rendered since the root loader always throws</p>
2282+
<Outlet />
2283+
</div>
2284+
);
2285+
}
2286+
`,
2287+
"src/routes/root.client.tsx": js`
2288+
"use client";
2289+
2290+
export function shouldRevalidate() {
2291+
// This should be ignored since this route always throws an error
2292+
return false;
2293+
}
2294+
`,
2295+
"src/routes/index.tsx": js`
2296+
export default function IndexRoute() {
2297+
return (
2298+
<div>
2299+
<h2>Index Route</h2>
2300+
<p>This should never be rendered since the root loader always throws</p>
2301+
</div>
2302+
);
2303+
}
2304+
`,
2305+
"src/routes/other.tsx": js`
2306+
export default function OtherRoute() {
2307+
return (
2308+
<div>
2309+
<h2>Other Route</h2>
2310+
<p>This should never be rendered since the root loader always throws</p>
2311+
</div>
2312+
);
2313+
}
2314+
`,
2315+
},
2316+
});
2317+
2318+
await page.goto(`http://localhost:${port}/`, {
2319+
waitUntil: "networkidle",
2320+
});
2321+
2322+
// Verify that the root error boundary is re-rendered as we navigate around
2323+
await page.waitForSelector(
2324+
"[data-error-boundary-loader-call-count='1']",
2325+
);
2326+
await page.click("[data-link-other]");
2327+
await page.waitForSelector(
2328+
"[data-error-boundary-loader-call-count='2']",
2329+
);
2330+
await page.click("[data-link-index]");
2331+
await page.waitForSelector(
2332+
"[data-error-boundary-loader-call-count='3']",
2333+
);
2334+
2335+
// Ensure this is using RSC
2336+
validateRSCHtml(await page.content());
2337+
});
22102338
});
22112339

22122340
test.describe("Route Client Component Props", () => {

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -869,9 +869,16 @@ async function generateStaticContextResponse(
869869
// actually have a loader so the single fetch logic can find a result for
870870
// the route. This is a bit of a hack but allows us to re-use all the
871871
// existing logic. This can go away if we ever fork off and re-implement a
872-
// standalone RSC `dataStrategy`
872+
// standalone RSC `dataStrategy`. We also need to ensure that we don't set
873+
// `loaderData` to `null` for routes that have an error, otherwise they won't
874+
// be forced to revalidate on navigation.
873875
staticContext.matches.forEach((m) => {
874-
if (staticContext.loaderData[m.route.id] === undefined) {
876+
const routeHasNoLoaderData =
877+
staticContext.loaderData[m.route.id] === undefined;
878+
const routeHasError = Boolean(
879+
staticContext.errors && m.route.id in staticContext.errors,
880+
);
881+
if (routeHasNoLoaderData && !routeHasError) {
875882
staticContext.loaderData[m.route.id] = null;
876883
}
877884
});

0 commit comments

Comments
 (0)