From 4b9ba88ff4cb44b4021fe8613d95701b1565a2a3 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Fri, 6 Sep 2024 13:56:34 -0400 Subject: [PATCH] Align single fetch prefetching with new revalidation logic --- .changeset/giant-olives-sort.md | 5 + integration/single-fetch-test.ts | 212 +++++++++++++++++++++++++++- packages/remix-react/components.tsx | 96 +++++++++---- 3 files changed, 279 insertions(+), 34 deletions(-) create mode 100644 .changeset/giant-olives-sort.md diff --git a/.changeset/giant-olives-sort.md b/.changeset/giant-olives-sort.md new file mode 100644 index 00000000000..91d868f245a --- /dev/null +++ b/.changeset/giant-olives-sort.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +[REMOVE] Align single fetch prefetchign with new revalidation logic diff --git a/integration/single-fetch-test.ts b/integration/single-fetch-test.ts index 40a24fc0eb4..79d1a860d5e 100644 --- a/integration/single-fetch-test.ts +++ b/integration/single-fetch-test.ts @@ -3253,9 +3253,9 @@ test.describe("single-fetch", () => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/", true); - // A/B can be prefetched, C doesn't get prefetched due to its `clientLoader` + // root/A/B can be prefetched, C doesn't get prefetched due to its `clientLoader` await page.waitForSelector( - "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=routes%2Fa%2Croutes%2Fa.b']", + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa%2Croutes%2Fa.b']", { state: "attached" } ); expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); @@ -3351,9 +3351,9 @@ test.describe("single-fetch", () => { let app = new PlaywrightFixture(appFixture, page); await app.goto("/", true); - // Only A can get prefetched, B/C can't due to `clientLoader` + // root/A can get prefetched, B/C can't due to `clientLoader` await page.waitForSelector( - "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=routes%2Fa']", + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa']", { state: "attached" } ); expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); @@ -3368,6 +3368,35 @@ test.describe("single-fetch", () => { }, files: { ...files, + "app/root.tsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export function loader() { + return { + message: "ROOT", + }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (root client loader)" }; + } + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, "app/routes/_index.tsx": js` import { Link } from "@remix-run/react"; @@ -3457,6 +3486,181 @@ test.describe("single-fetch", () => { // No prefetching due to clientLoaders expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(0); }); + + test("when a reused route opts out of revalidation", async ({ page }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/a.tsx": js` + import { Link, Outlet, useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "A server loader" }; + } + + export function shouldRevalidate() { + return false; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "C server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/a", true); + + // A opted out of revalidation + await page.waitForSelector( + "link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa.b%2Croutes%2Fa.b.c']", + { state: "attached" } + ); + expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); + }); + + test("when a reused route opts out of revalidation and another route has a clientLoader", async ({ + page, + }) => { + let fixture = await createFixture({ + config: { + future: { + unstable_singleFetch: true, + }, + }, + files: { + ...files, + "app/routes/a.tsx": js` + import { Link, Outlet, useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "A server loader" }; + } + + export function shouldRevalidate() { + return false; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

A

+

{data.message}

+ + + + ); + } + `, + "app/routes/a.b.tsx": js` + import { Outlet, useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "B server loader" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

B

+

{data.message}

+ + + ); + } + `, + "app/routes/a.b.c.tsx": js` + import { useLoaderData } from '@remix-run/react'; + + export function loader() { + return { message: "C server loader" }; + } + + export async function clientLoader({ serverLoader }) { + let data = await serverLoader(); + return { message: data.message + " (C client loader)" }; + } + + export default function Comp() { + let data = useLoaderData(); + return ( + <> +

C

+

{data.message}

+ + ); + } + `, + }, + }); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/a", true); + + // A opted out of revalidation + await page.waitForSelector( + "nav link[rel='prefetch'][as='fetch'][href='/a/b/c.data?_routes=root%2Croutes%2Fa.b']", + { state: "attached" } + ); + expect(await app.page.locator("nav link[as='fetch']").count()).toEqual(1); + }); }); test("supports nonce on streaming script tags", async ({ page }) => { diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 0f8725b824e..046a9f8ce8a 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -449,7 +449,7 @@ function PrefetchPageLinksImpl({ }) { let location = useLocation(); let { future, manifest, routeModules } = useRemixContext(); - let { matches } = useDataRouterStateContext(); + let { loaderData, matches } = useDataRouterStateContext(); let newMatchesForData = React.useMemo( () => @@ -491,37 +491,73 @@ function PrefetchPageLinksImpl({ // just the manifest like the other links in here. let keyedPrefetchLinks = useKeyedPrefetchLinks(newMatchesForAssets); - let linksToRender: React.ReactNode | React.ReactNode[] | null = null; - if (!future.unstable_singleFetch) { - // Non-single-fetch prefetching - linksToRender = dataHrefs.map((href) => ( - - )); - } else if (newMatchesForData.length > 0) { - // Single-fetch with routes that require data - let url = addRevalidationParam( - manifest, - routeModules, - nextMatches.map((m) => m.route), - newMatchesForData.map((m) => m.route), - singleFetchUrl(page) - ); - if (url.searchParams.get("_routes") !== "") { - linksToRender = ( - - ); - } else { + let linksToRender = React.useMemo(() => { + if (!future.unstable_singleFetch) { + // Non-single-fetch prefetching is easy... + return dataHrefs.map((href) => ( + + )); + } + + // Single-fetch is harder :) + // This parallels the logic in the single fetch data strategy + let routesParams = new Set(); + let foundOptOutRoute = false; + nextMatches.forEach((m) => { + if (manifest.routes[m.route.id].hasLoader) { + if ( + !newMatchesForData.some((m2) => m2.route.id === m.route.id) && + m.route.id in loaderData && + routeModules[m.route.id]?.shouldRevalidate + ) { + foundOptOutRoute = true; + } else if (manifest.routes[m.route.id].hasClientLoader) { + foundOptOutRoute = true; + } else { + routesParams.add(m.route.id); + } + } + }); + + if (routesParams.size === 0) { // No single-fetch prefetching if _routes param is empty due to `clientLoader`'s + return null; } - } else { - // No single-fetch prefetching if there are no new matches for data - } + + let url = singleFetchUrl(page); + // When one or more routes have opted out, we add a _routes param to + // limit the loaders to those that have a server loader and did not + // opt out + if (foundOptOutRoute && routesParams.size > 0) { + url.searchParams.set( + "_routes", + nextMatches + .filter((m) => routesParams.has(m.route.id)) + .map((m) => m.route.id) + .join(",") + ); + } + + return ( + + ); + }, [ + dataHrefs, + future.unstable_singleFetch, + linkProps, + loaderData, + manifest.routes, + newMatchesForData, + nextMatches, + page, + routeModules, + ]); return ( <>