diff --git a/.changeset/three-oranges-sort.md b/.changeset/three-oranges-sort.md new file mode 100644 index 0000000000..5b90ed6b49 --- /dev/null +++ b/.changeset/three-oranges-sort.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Fix `shouldRevalidate` behavior for `clientLoader`-only routes in `ssr:true` apps diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 9246818e33..51a670b208 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -2200,6 +2200,97 @@ test.describe("single-fetch", () => { ).toBe(true); }); + test("allows reused routes to opt out via shouldRevalidate (w/only clientLoader)", async ({ + page, + }) => { + let fixture = await createFixture({ + files: { + ...files, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Component() { + return Go to /parent/a; + } + `, + "app/routes/parent.tsx": js` + import { Link, Outlet, useLoaderData } from "react-router"; + let count = 0; + export function clientLoader({ request }) { + return { count: ++count }; + } + export function shouldRevalidate() { + return false; + } + export default function Component() { + return ( + <> +

Parent Count: {useLoaderData().count}

+ Go to /parent/a + Go to /parent/b + + + ); + } + `, + "app/routes/parent.a.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

A Count: {useLoaderData().count}

; + } + `, + "app/routes/parent.b.tsx": js` + import { useLoaderData } from "react-router"; + let count = 0; + export function loader({ request }) { + return { count: ++count }; + } + export default function Component() { + return

B Count: {useLoaderData().count}

; + } + `, + }, + }); + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + let urls: string[] = []; + page.on("request", (req) => { + if (req.url().includes(".data")) { + urls.push(req.url()); + } + }); + + await app.goto("/"); + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 1"); + expect(urls.length).toBe(1); + // Client loader triggers 2 requests on the first navigation + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/b"); + await page.waitForSelector("#b"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#b")).toContain("B Count: 1"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/b.data")).toBe(true); + urls = []; + + await app.clickLink("/parent/a"); + await page.waitForSelector("#a"); + expect(await app.getHtml("#parent")).toContain("Parent Count: 1"); + expect(await app.getHtml("#a")).toContain("A Count: 2"); + expect(urls.length).toBe(1); + expect(urls[0].endsWith("/parent/a.data")).toBe(true); + }); + test("provides the proper defaultShouldRevalidate value", async ({ page, }) => { diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx index dee9a4305d..bec8bbf53f 100644 --- a/packages/react-router/lib/dom/ssr/single-fetch.tsx +++ b/packages/react-router/lib/dom/ssr/single-fetch.tsx @@ -304,21 +304,6 @@ async function nonSsrStrategy( return results; } -function isOptedOut( - manifestRoute: EntryRoute | undefined, - routeModule: RouteModule | undefined, - match: DataStrategyMatch, - router: DataRouter -) { - return ( - match.route.id in router.state.loaderData && - manifestRoute && - manifestRoute.hasLoader && - routeModule && - routeModule.shouldRevalidate - ); -} - // Loaders are trickier since we only want to hit the server once, so we // create a singular promise for all server-loader routes to latch onto. async function singleFetchLoaderNavigationStrategy( @@ -373,11 +358,19 @@ async function singleFetchLoaderNavigationStrategy( return; } - // Otherwise, we opt out if we currently have data, a `loader`, and a + // Otherwise, we opt out if we currently have data and a // `shouldRevalidate` function. This implies that the user opted out // via `shouldRevalidate` - if (isOptedOut(manifestRoute, routeModules[m.route.id], m, router)) { - foundOptOutRoute = true; + if ( + m.route.id in router.state.loaderData && + manifestRoute && + m.route.shouldRevalidate + ) { + if (manifestRoute.hasLoader) { + // If we have a server loader, make sure we don't include it in the + // single fetch .data request + foundOptOutRoute = true; + } return; } }