diff --git a/.changeset/orange-sloths-tease.md b/.changeset/orange-sloths-tease.md new file mode 100644 index 0000000000..6fe10ad2d1 --- /dev/null +++ b/.changeset/orange-sloths-tease.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +Properly revalidate prerendered paths when param values change diff --git a/integration/vite-prerender-test.ts b/integration/vite-prerender-test.ts index c6b0c6d18c..ec31dd0e72 100644 --- a/integration/vite-prerender-test.ts +++ b/integration/vite-prerender-test.ts @@ -169,38 +169,38 @@ test.describe("Prerendering", () => { files: { ...files, "app/routes/parent.tsx": js` - import { Outlet } from 'react-router' - export default function Component() { - return - } - `, + import { Outlet } from 'react-router' + export default function Component() { + return + } + `, "app/routes/parent.child.tsx": js` - import { Outlet } from 'react-router' - export function loader() { - return null; - } - export default function Component() { - return - } - `, + import { Outlet } from 'react-router' + export function loader() { + return null; + } + export default function Component() { + return + } + `, "app/routes/$slug.tsx": js` - import { Outlet } from 'react-router' - export function loader() { - return null; - } - export default function Component() { - return - } - `, + import { Outlet } from 'react-router' + export function loader() { + return null; + } + export default function Component() { + return + } + `, "app/routes/$.tsx": js` - import { Outlet } from 'react-router' - export function loader() { - return null; - } - export default function Component() { - return - } - `, + import { Outlet } from 'react-router' + export function loader() { + return null; + } + export default function Component() { + return + } + `, }, }); @@ -239,24 +239,24 @@ test.describe("Prerendering", () => { files: { ...files, "react-router.config.ts": js` - export default { - async prerender() { - await new Promise(r => setTimeout(r, 1)); - return ['/', '/about']; - }, - } - `, + export default { + async prerender() { + await new Promise(r => setTimeout(r, 1)); + return ['/', '/about']; + }, + } + `, "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; - - export default defineConfig({ - build: { manifest: true }, - plugins: [ - reactRouter() - ], - }); - `, + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; + + export default defineConfig({ + build: { manifest: true }, + plugins: [ + reactRouter() + ], + }); + `, }, }); appFixture = await createAppFixture(fixture); @@ -291,26 +291,26 @@ test.describe("Prerendering", () => { files: { ...files, "react-router.config.ts": js` - let counter = 1; - export default { - serverBundles: () => "server" + counter++, - async prerender() { - await new Promise(r => setTimeout(r, 1)); - return ['/', '/about']; - }, - } - `, + let counter = 1; + export default { + serverBundles: () => "server" + counter++, + async prerender() { + await new Promise(r => setTimeout(r, 1)); + return ['/', '/about']; + }, + } + `, "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; - - export default defineConfig({ - build: { manifest: true }, - plugins: [ - reactRouter() - ], - }); - `, + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; + + export default defineConfig({ + build: { manifest: true }, + plugins: [ + reactRouter() + ], + }); + `, }, }); appFixture = await createAppFixture(fixture); @@ -344,29 +344,29 @@ test.describe("Prerendering", () => { files: { ...files, "react-router.config.ts": js` - export default { - async prerender({ getStaticPaths }) { - return [...getStaticPaths(), "/a", "/b"]; - }, - } - `, + export default { + async prerender({ getStaticPaths }) { + return [...getStaticPaths(), "/a", "/b"]; + }, + } + `, "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; - export default defineConfig({ - build: { manifest: true }, - plugins: [reactRouter()], - }); - `, + export default defineConfig({ + build: { manifest: true }, + plugins: [reactRouter()], + }); + `, "app/routes/$slug.tsx": js` - export function loader() { - return null - } - export default function component() { - return null; - } - `, + export function loader() { + return null + } + export default function component() { + return null; + } + `, }, }); appFixture = await createAppFixture(fixture); @@ -443,19 +443,19 @@ test.describe("Prerendering", () => { files: { ...files, "app/routes/text[.txt].tsx": js` - export function loader() { - return new Response("Hello, world"); - } - `, + export function loader() { + return new Response("Hello, world"); + } + `, "app/routes/json[.json].tsx": js` - export function loader() { - return new Response(JSON.stringify({ hello: 'world' }), { - headers: { - 'Content-Type': 'application/json', - } - }); - } - `, + export function loader() { + return new Response(JSON.stringify({ hello: 'world' }), { + headers: { + 'Content-Type': 'application/json', + } + }); + } + `, "app/routes/image[.png].tsx": js` export function loader() { return new Response( @@ -531,24 +531,24 @@ test.describe("Prerendering", () => { files: { ...files, "react-router.config.ts": js` - export default { - async prerender() { - await new Promise(r => setTimeout(r, 1)); - return ['/', 'about']; - }, - } - `, + export default { + async prerender() { + await new Promise(r => setTimeout(r, 1)); + return ['/', 'about']; + }, + } + `, "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; - - export default defineConfig({ - build: { manifest: true }, - plugins: [ - reactRouter() - ], - }); - `, + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; + + export default defineConfig({ + build: { manifest: true }, + plugins: [ + reactRouter() + ], + }); + `, }, }); appFixture = await createAppFixture(fixture); @@ -590,36 +590,36 @@ test.describe("Prerendering", () => { prerender: ["/", "/about"], }), "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; - export default defineConfig({ - build: { manifest: true }, - plugins: [reactRouter()], - }); - `, + export default defineConfig({ + build: { manifest: true }, + plugins: [reactRouter()], + }); + `, "app/routes/about.tsx": js` - import { useLoaderData } from 'react-router'; - export function loader({ request }) { - return "ABOUT-" + request.headers.has('X-React-Router-Prerender'); - } - - export default function Comp() { - let data = useLoaderData(); - return

About: {data}

- } - `, + import { useLoaderData } from 'react-router'; + export function loader({ request }) { + return "ABOUT-" + request.headers.has('X-React-Router-Prerender'); + } + + export default function Comp() { + let data = useLoaderData(); + return

About: {data}

+ } + `, "app/routes/not-prerendered.tsx": js` - import { useLoaderData } from 'react-router'; - export function loader({ request }) { - return "NOT-PRERENDERED-" + request.headers.has('X-React-Router-Prerender'); - } - - export default function Comp() { - let data = useLoaderData(); - return

Not-Prerendered: {data}

- } - `, + import { useLoaderData } from 'react-router'; + export function loader({ request }) { + return "NOT-PRERENDERED-" + request.headers.has('X-React-Router-Prerender'); + } + + export default function Comp() { + let data = useLoaderData(); + return

Not-Prerendered: {data}

+ } + `, }, }); appFixture = await createAppFixture(fixture); @@ -646,35 +646,35 @@ test.describe("Prerendering", () => { prerender: ["/", "/about"], }), "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; - export default defineConfig({ - build: { manifest: true }, - plugins: [reactRouter()], - }); - `, + export default defineConfig({ + build: { manifest: true }, + plugins: [reactRouter()], + }); + `, "app/routes/about.tsx": js` - import { useLoaderData } from 'react-router'; - export function loader({ request }) { - return { - prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', - // 24999 characters - data: new Array(5000).fill('test').join('-'), - }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

Large loader

-

{data.prerendered}

-

{data.data.length}

- - ); - } - `, + import { useLoaderData } from 'react-router'; + export function loader({ request }) { + return { + prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', + // 24999 characters + data: new Array(5000).fill('test').join('-'), + }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

Large loader

+

{data.prerendered}

+

{data.data.length}

+ + ); + } + `, }, }); appFixture = await createAppFixture(fixture); @@ -699,54 +699,54 @@ test.describe("Prerendering", () => { prerender: ["/", "/utf8-prerendered"], }), "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; - export default defineConfig({ - build: { manifest: true }, - plugins: [reactRouter()], - }); - `, + export default defineConfig({ + build: { manifest: true }, + plugins: [reactRouter()], + }); + `, "app/routes/utf8-prerendered.tsx": js` - import { useLoaderData } from 'react-router'; - export function loader({ request }) { - return { - prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', - data: "한글 데이터 - UTF-8 문자", - }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

UTF-8 Prerendered

-

{data.prerendered}

-

{data.data}

- - ); - } - `, + import { useLoaderData } from 'react-router'; + export function loader({ request }) { + return { + prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', + data: "한글 데이터 - UTF-8 문자", + }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

UTF-8 Prerendered

+

{data.prerendered}

+

{data.data}

+ + ); + } + `, "app/routes/utf8-not-prerendered.tsx": js` - import { useLoaderData } from 'react-router'; - export function loader({ request }) { - return { - prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', - data: "非プリレンダリングデータ - UTF-8文字", - }; - } - - export default function Comp() { - let data = useLoaderData(); - return ( - <> -

UTF-8 Not Prerendered

-

{data.prerendered}

-

{data.data}

- - ); - } - `, + import { useLoaderData } from 'react-router'; + export function loader({ request }) { + return { + prerendered: request.headers.has('X-React-Router-Prerender') ? 'yes' : 'no', + data: "非プリレンダリングデータ - UTF-8文字", + }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

UTF-8 Not Prerendered

+

{data.prerendered}

+

{data.data}

+ + ); + } + `, }, }); appFixture = await createAppFixture(fixture); @@ -782,47 +782,47 @@ test.describe("Prerendering", () => { prerender: ["/", "/parent", "/parent/child"], }), "vite.config.ts": js` - import { defineConfig } from "vite"; - import { reactRouter } from "@react-router/dev/vite"; + import { defineConfig } from "vite"; + import { reactRouter } from "@react-router/dev/vite"; - export default defineConfig({ - build: { manifest: true }, - plugins: [reactRouter()], - }); - `, + export default defineConfig({ + build: { manifest: true }, + plugins: [reactRouter()], + }); + `, "app/routes/parent.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - export function loader() { - return "PARENT"; - } - export default function Comp() { - let data = useLoaderData(); - return <>

Parent: {data}

- } - `, + import { Outlet, useLoaderData } from 'react-router'; + export function loader() { + return "PARENT"; + } + export default function Comp() { + let data = useLoaderData(); + return <>

Parent: {data}

+ } + `, "app/routes/parent.child.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - export function loader() { - return "CHILD"; - } - export function HydrateFallback() { - return

Child loading...

- } - export default function Comp() { - let data = useLoaderData(); - return <>

Child: {data}

- } - `, + import { Outlet, useLoaderData } from 'react-router'; + export function loader() { + return "CHILD"; + } + export function HydrateFallback() { + return

Child loading...

+ } + export default function Comp() { + let data = useLoaderData(); + return <>

Child: {data}

+ } + `, "app/routes/parent.child._index.tsx": js` - import { Outlet, useLoaderData } from 'react-router'; - export function clientLoader() { - return "INDEX"; - } - export default function Comp() { - let data = useLoaderData(); - return <>

Index: {data}

- } - `, + import { Outlet, useLoaderData } from 'react-router'; + export function clientLoader() { + return "INDEX"; + } + export default function Comp() { + let data = useLoaderData(); + return <>

Index: {data}

+ } + `, }, }); appFixture = await createAppFixture(fixture); @@ -853,6 +853,12 @@ test.describe("Prerendering", () => { return requests; } + function clearRequests(requests: string[]) { + while (requests.length) { + requests.pop(); + } + } + test("Errors on headers/action functions in any route", async () => { let cwd = await createProject({ "react-router.config.ts": reactRouterConfig({ @@ -1206,43 +1212,45 @@ test.describe("Prerendering", () => { expect(await (await page.$("[data-page]"))?.innerText()).toBe( "PAGE DATA" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 1" ); + // No revalidation after submission to self + expect(requests).toEqual([]); await app.clickLink("/page2"); await page.waitForSelector("[data-page2]"); expect(await (await page.$("[data-page2]"))?.innerText()).toBe( "PAGE2 DATA" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 1" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 2" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 2" ); - - // We should only make this call when navigating to the prerendered route - // 2 calls (no revalidation after submission to self): - // - ✅ Initial navigation - // - ❌ No revalidation after submission to self - // - ✅ After submission back from /page - expect(requests).toEqual(["/page.data", "/page.data"]); + expect(requests).toEqual([]); }); test("Navigates across SPA/prerender pages when starting from a prerendered page", async ({ @@ -1345,43 +1353,45 @@ test.describe("Prerendering", () => { expect(await (await page.$("[data-page]"))?.innerText()).toBe( "PAGE DATA" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 1" ); + // No revalidation after submission to self + expect(requests).toEqual([]); await app.clickLink("/page2"); await page.waitForSelector("[data-page2]"); expect(await (await page.$("[data-page2]"))?.innerText()).toBe( "PAGE2 DATA" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 1" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 2" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 2" ); - - // We should only make this call when navigating to the prerendered route - // 2 calls (no revalidation after submission to self): - // - ✅ Initial navigation - // - ❌ No revalidation after submission to self - // - ✅ After submission back from /page - expect(requests).toEqual(["/page.data", "/page.data"]); + expect(requests).toEqual([]); }); test("Navigates across SPA/prerender pages when starting from a SPA page and a root loader exists", async ({ @@ -1496,43 +1506,45 @@ test.describe("Prerendering", () => { expect(await (await page.$("[data-page]"))?.innerText()).toBe( "PAGE DATA" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 1" ); + // No revalidation after submission to self + expect(requests).toEqual([]); await app.clickLink("/page2"); await page.waitForSelector("[data-page2]"); expect(await (await page.$("[data-page2]"))?.innerText()).toBe( "PAGE2 DATA" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 1" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 2" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 2" ); - - // We should only make this call when navigating to the prerendered route - // 2 calls (no revalidation after submission to self): - // - ✅ Initial navigation - // - ❌ No revalidation after submission to self - // - ✅ After submission back from /page - expect(requests).toEqual(["/page.data", "/page.data"]); + expect(requests).toEqual([]); }); test("Navigates across SPA/prerender pages when starting from a prerendered page and a root loader exists", async ({ @@ -1647,43 +1659,45 @@ test.describe("Prerendering", () => { expect(await (await page.$("[data-page]"))?.innerText()).toBe( "PAGE DATA" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 1" ); + // No revalidation after submission to self + expect(requests).toEqual([]); await app.clickLink("/page2"); await page.waitForSelector("[data-page2]"); expect(await (await page.$("[data-page2]"))?.innerText()).toBe( "PAGE2 DATA" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 1" ); + expect(requests).toEqual([]); await app.clickSubmitButton("/page"); await page.waitForSelector("[data-page-action]"); expect(await (await page.$("[data-page-action]"))?.innerText()).toBe( "PAGE ACTION 2" ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); await app.clickSubmitButton("/page2"); await page.waitForSelector("[data-page2-action]"); expect(await (await page.$("[data-page2-action]"))?.innerText()).toBe( "PAGE2 ACTION 2" ); - - // We should only make this call when navigating to the prerendered route - // 2 calls (no revalidation after submission to self): - // - ✅ Initial navigation - // - ❌ No revalidation after submission to self - // - ✅ After submission back from /page - expect(requests).toEqual(["/page.data", "/page.data"]); + expect(requests).toEqual([]); }); test("Navigates between prerendered parent and child SPA route", async ({ @@ -2241,6 +2255,185 @@ test.describe("Prerendering", () => { expect(requests).toEqual(["/parent/child.data"]); }); + test("Navigates prerender pages when params exist", async ({ page }) => { + fixture = await createFixture({ + prerender: true, + files: { + "react-router.config.ts": reactRouterConfig({ + ssr: false, // turn off fog of war since we're serving with a static server + prerender: ["/", "/page", "/param/1", "/param/2"], + }), + "vite.config.ts": files["vite.config.ts"], + "app/root.tsx": js` + import * as React from "react"; + import { Link, Outlet, Scripts, useNavigation } from "react-router"; + + export function Layout({ children }) { + let navigation = useNavigation(); + return ( + + + + +

{navigation.state}

+ {children} + + + + ); + } + + export default function Root({ loaderData }) { + return + } + `, + "app/routes/_index.tsx": js` + export default function Index() { + return

Index

+ } + `, + "app/routes/page.tsx": js` + export function loader() { + return "PAGE DATA" + } + export default function Page({ loaderData }) { + return

{loaderData}

; + } + `, + "app/routes/param.$id.tsx": js` + export function loader({ params }) { + return params.id; + } + export default function Page({ loaderData }) { + return

Param {loaderData}

; + } + `, + }, + }); + appFixture = await createAppFixture(fixture); + + let requests = captureRequests(page); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/", true); + await page.waitForSelector("[data-index]"); + + await app.clickLink("/page"); + await page.waitForSelector("[data-page]"); + expect(await (await page.$("[data-page]"))?.innerText()).toBe( + "PAGE DATA" + ); + expect(requests).toEqual(["/page.data"]); + clearRequests(requests); + + await app.clickLink("/page"); + await page.waitForSelector("#navigation-idle"); + expect(await (await page.$("[data-page]"))?.innerText()).toBe( + "PAGE DATA" + ); + // No revalidation since page.data is static + expect(requests).toEqual([]); + + await app.clickLink("/param/1"); + await page.waitForSelector('[data-param="1"]'); + expect(await (await page.$("[data-param]"))?.innerText()).toBe("Param 1"); + console.log("asserting", requests); + expect(requests).toEqual(["/param/1.data"]); + clearRequests(requests); + + await app.clickLink("/param/2"); + await page.waitForSelector('[data-param="2"]'); + expect(await (await page.$("[data-param]"))?.innerText()).toBe("Param 2"); + expect(requests).toEqual(["/param/2.data"]); + clearRequests(requests); + + await app.clickLink("/page"); + await page.waitForSelector("[data-page]"); + expect(await (await page.$("[data-page]"))?.innerText()).toBe( + "PAGE DATA" + ); + expect(requests).toEqual(["/page.data"]); + }); + + test("Returns a 404 if navigating to a non-prerendered param value", async ({ + page, + }) => { + fixture = await createFixture({ + prerender: true, + files: { + "react-router.config.ts": reactRouterConfig({ + ssr: false, // turn off fog of war since we're serving with a static server + prerender: ["/param/1"], + }), + "vite.config.ts": files["vite.config.ts"], + "app/root.tsx": js` + import * as React from "react"; + import { Link, Outlet, Scripts, useNavigation } from "react-router"; + + export function Layout({ children }) { + let navigation = useNavigation(); + return ( + + + + +

{navigation.state}

+ {children} + + + + ); + } + + export default function Root({ loaderData }) { + return + } + `, + "app/routes/_index.tsx": js` + export default function Index() { + return

Index

+ } + `, + "app/routes/param.$id.tsx": js` + export function loader({ params }) { + return params.id; + } + export default function Page({ loaderData }) { + return

Param {loaderData}

; + } + + export function ErrorBoundary({ error }) { + return

{error.status}

; + } + `, + }, + }); + appFixture = await createAppFixture(fixture); + + let requests = captureRequests(page); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/", true); + await page.waitForSelector("[data-index]"); + + await app.clickLink("/param/1"); + await page.waitForSelector('[data-param="1"]'); + expect(await (await page.$("[data-param]"))?.innerText()).toBe("Param 1"); + console.log("asserting", requests); + expect(requests).toEqual(["/param/1.data"]); + clearRequests(requests); + + await app.clickLink("/param/404"); + await page.waitForSelector('[data-error="404"]'); + expect(requests).toEqual(["/param/404.data"]); + }); + test("Navigates to prerendered parent with clientLoader calling loader", async ({ page, }) => { diff --git a/packages/react-router/lib/dom/ssr/routes.tsx b/packages/react-router/lib/dom/ssr/routes.tsx index 27cb9feb2f..b9ed35559e 100644 --- a/packages/react-router/lib/dom/ssr/routes.tsx +++ b/packages/react-router/lib/dom/ssr/routes.tsx @@ -9,7 +9,7 @@ import type { ShouldRevalidateFunction, ShouldRevalidateFunctionArgs, } from "../../router/utils"; -import { ErrorResponseImpl } from "../../router/utils"; +import { ErrorResponseImpl, compilePath } from "../../router/utils"; import type { RouteModule, RouteModules } from "./routeModules"; import { loadRouteModule } from "./routeModules"; import type { FutureConfig } from "./entry"; @@ -312,6 +312,7 @@ export function createClientRoutes( unstable_middleware: routeModule.unstable_clientMiddleware, handle: routeModule.handle, shouldRevalidate: getShouldRevalidateFunction( + dataRoute.path, routeModule, route, ssr, @@ -524,6 +525,7 @@ export function createClientRoutes( shouldRevalidate: async () => { let lazyRoute = await getLazyRoute(); return getShouldRevalidateFunction( + dataRoute.path, lazyRoute, route, ssr, @@ -556,6 +558,7 @@ export function createClientRoutes( } function getShouldRevalidateFunction( + path: string | undefined, route: Partial, manifestRoute: Omit, ssr: boolean, @@ -572,17 +575,29 @@ function getShouldRevalidateFunction( // When prerendering is enabled with `ssr:false`, any `loader` data is // statically generated at build time so if we have a `loader` but not a - // `clientLoader`, we disable revalidation by default since we can't be sure - // if a `.data` file was pre-rendered. If users are somehow re-generating - // updated versions of these on the backend they can still opt-into - // revalidation which will make the `.data` request + // `clientLoader`, we only revalidate if the route's params changed since we + // can't be sure if a `.data` file was pre-rendered otherwise. + // + // I.e., If I have a parent and a child route and I only prerender `/parent`, + // we can't have parent revalidate when going from `/parent -> /parent/child` + // because `/parent/child.data` doesn't exist. + // + // If users are somehow re-generating updated versions of these on the backend + // they can still opt-into revalidation which will make the `.data` request if (!ssr && manifestRoute.hasLoader && !manifestRoute.hasClientLoader) { + let myParams = path ? compilePath(path)[1].map((p) => p.paramName) : []; + const didParamsChange = (opts: ShouldRevalidateFunctionArgs) => + myParams.some((p) => opts.currentParams[p] !== opts.nextParams[p]); + if (route.shouldRevalidate) { let fn = route.shouldRevalidate; return (opts: ShouldRevalidateFunctionArgs) => - fn({ ...opts, defaultShouldRevalidate: false }); + fn({ + ...opts, + defaultShouldRevalidate: didParamsChange(opts), + }); } else { - return () => false; + return (opts: ShouldRevalidateFunctionArgs) => didParamsChange(opts); } } diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts index c375d3c42c..b20d2323f5 100644 --- a/packages/react-router/lib/router/utils.ts +++ b/packages/react-router/lib/router/utils.ts @@ -1206,7 +1206,7 @@ export function matchPath< type CompiledPathParam = { paramName: string; isOptional?: boolean }; -function compilePath( +export function compilePath( path: string, caseSensitive = false, end = true