fix(react): dedupe prefetch links (#7060)

remix-run · Aug 9, 2023 · a179aa7 · a179aa7
1 parent 9543712
commit a179aa7
Show file tree

Hide file tree

Showing 4 changed files with 211 additions and 39 deletions.
diff --git a/.changeset/deduplicate-prefetch-link-tags.md b/.changeset/deduplicate-prefetch-link-tags.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/react": patch
+---
+
+Deduplicate prefetch link tags
diff --git a/integration/prefetch-test.ts b/integration/prefetch-test.ts
@@ -1,6 +1,11 @@
 import { test, expect } from "@playwright/test";
 
-import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+import {
+  createAppFixture,
+  createFixture,
+  js,
+  css,
+} from "./helpers/create-fixture";
 import type {
   Fixture,
   FixtureInit,
@@ -424,4 +429,140 @@ test.describe("other scenarios", () => {
     );
     expect(stylesheets.length).toBe(1);
   });
+
+  test("dedupes prefetch tags", async ({ page }) => {
+    fixture = await createFixture({
+      files: {
+        "app/root.tsx": js`
+          import {
+            Link,
+            Links,
+            Meta,
+            Outlet,
+            Scripts,
+            useLoaderData,
+          } from "@remix-run/react";
+
+          export default function Root() {
+            const styles =
+            'a:hover { color: red; } a:hover:after { content: " (hovered)"; }' +
+            'a:focus { color: green; } a:focus:after { content: " (focused)"; }';
+
+            return (
+              <html lang="en">
+                <head>
+                  <Meta />
+                  <Links />
+                </head>
+                <body>
+                  <style>{styles}</style>
+                  <h1>Root</h1>
+                  <nav id="nav">
+                    <Link to="/with-nested-links/nested" prefetch="intent">
+                      Nested Links Page
+                    </Link>
+                  </nav>
+                  <Outlet />
+                  <Scripts />
+                </body>
+              </html>
+            );
+          }
+        `,
+
+        "app/global.css": css`
+          .global-class {
+            background-color: gray;
+            color: black;
+          }
+        `,
+
+        "app/local.css": css`
+          .local-class {
+            background-color: black;
+            color: white;
+          }
+        `,
+
+        "app/routes/_index.tsx": js`
+          export default function() {
+            return <h2 className="index">Index</h2>;
+          }
+        `,
+
+        "app/routes/with-nested-links.tsx": js`
+          import { Outlet } from "@remix-run/react";
+          import globalCss from "../global.css";
+          
+          export function links() {
+            return [
+              // Same links as child route but with different key order
+              {
+                rel: "stylesheet",
+                href: globalCss,
+              },
+              {
+                rel: "preload",
+                as: "image",
+                imageSrcSet: "image-600.jpg 600w, image-1200.jpg 1200w",
+                imageSizes: "9999px",
+              },
+            ];
+          }
+          export default function() {
+            return <Outlet />;
+          }
+        `,
+
+        "app/routes/with-nested-links.nested.tsx": js`
+          import globalCss from '../global.css';
+          import localCss from '../local.css';
+          
+          export function links() {
+            return [
+              // Same links as parent route but with different key order
+              {
+                href: globalCss,
+                rel: "stylesheet",
+              },
+              {
+                imageSrcSet: "image-600.jpg 600w, image-1200.jpg 1200w",
+                imageSizes: "9999px",
+                rel: "preload",
+                as: "image",
+              },
+              // Unique links for child route
+              {
+                rel: "stylesheet",
+                href: localCss,
+              },
+              {
+                rel: "preload",
+                as: "image",
+                imageSrcSet: "image-700.jpg 700w, image-1400.jpg 1400w",
+                imageSizes: "9999px",
+              },
+            ];
+          }
+          export default function() {
+            return <h2 className="with-nested-links">With Nested Links</h2>;
+          }
+        `,
+      },
+    });
+    appFixture = await createAppFixture(fixture);
+
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/");
+    await page.hover("a[href='/with-nested-links/nested']");
+    await page.waitForSelector("#nav link[rel='prefetch'][as='style']", {
+      state: "attached",
+    });
+    expect(
+      await page.locator("#nav link[rel='prefetch'][as='style']").count()
+    ).toBe(2);
+    expect(
+      await page.locator("#nav link[rel='prefetch'][as='image']").count()
+    ).toBe(2);
+  });
 });
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
@@ -36,13 +36,13 @@ import { RemixRootDefaultErrorBoundary } from "./errorBoundaries";
 import invariant from "./invariant";
 import {
   getDataLinkHrefs,
-  getLinksForMatches,
+  getKeyedLinksForMatches,
+  getKeyedPrefetchLinks,
   getModuleLinkHrefs,
   getNewMatchesForLinks,
-  getStylesheetPrefetchLinks,
   isPageLinkDescriptor,
 } from "./links";
-import type { HtmlLinkDescriptor, PrefetchPageDescriptor } from "./links";
+import type { KeyedHtmlLinkDescriptor, PrefetchPageDescriptor } from "./links";
 import { createHtml, escapeHtml } from "./markup";
 import type {
   MetaFunction,
@@ -327,16 +327,16 @@ export function Links() {
       )
     : routerMatches;
 
-  let links = React.useMemo(
-    () => getLinksForMatches(matches, routeModules, manifest),
+  let keyedLinks = React.useMemo(
+    () => getKeyedLinksForMatches(matches, routeModules, manifest),
     [matches, routeModules, manifest]
   );
 
   return (
     <>
-      {links.map((link) => {
+      {keyedLinks.map(({ key, link }) => {
         if (isPageLinkDescriptor(link)) {
-          return <PrefetchPageLinks key={link.page} {...link} />;
+          return <PrefetchPageLinks key={key} {...link} />;
         }
 
         let imageSrcSet: string | null = null;
@@ -360,7 +360,7 @@ export function Links() {
 
         return (
           <link
-            key={link.rel + (link.href || "") + (imageSrcSet || "")}
+            key={key}
             {...{
               ...link,
               [imageSizesKey]: imageSizes,
@@ -402,26 +402,28 @@ export function PrefetchPageLinks({
   );
 }
 
-function usePrefetchedStylesheets(matches: AgnosticDataRouteMatch[]) {
+function useKeyedPrefetchLinks(matches: AgnosticDataRouteMatch[]) {
   let { manifest, routeModules } = useRemixContext();
 
-  let [styleLinks, setStyleLinks] = React.useState<HtmlLinkDescriptor[]>([]);
+  let [keyedPrefetchLinks, setKeyedPrefetchLinks] = React.useState<
+    KeyedHtmlLinkDescriptor[]
+  >([]);
 
   React.useEffect(() => {
     let interrupted: boolean = false;
 
-    getStylesheetPrefetchLinks(matches, manifest, routeModules).then(
-      (links) => {
-        if (!interrupted) setStyleLinks(links);
+    getKeyedPrefetchLinks(matches, manifest, routeModules).then((links) => {
+      if (!interrupted) {
+        setKeyedPrefetchLinks(links);
       }
-    );
+    });
 
     return () => {
       interrupted = true;
     };
   }, [matches, manifest, routeModules]);
 
-  return styleLinks;
+  return keyedPrefetchLinks;
 }
 
 function PrefetchPageLinksImpl({
@@ -473,7 +475,7 @@ function PrefetchPageLinksImpl({
 
   // needs to be a hook with async behavior because we need the modules, not
   // just the manifest like the other links in here.
-  let styleLinks = usePrefetchedStylesheets(newMatchesForAssets);
+  let keyedPrefetchLinks = useKeyedPrefetchLinks(newMatchesForAssets);
 
   return (
     <>
@@ -483,10 +485,10 @@ function PrefetchPageLinksImpl({
       {moduleHrefs.map((href) => (
         <link key={href} rel="modulepreload" href={href} {...linkProps} />
       ))}
-      {styleLinks.map((link) => (
+      {keyedPrefetchLinks.map(({ key, link }) => (
         // these don't spread `linkProps` because they are full link descriptors
         // already with their own props
-        <link key={link.href} {...link} />
+        <link key={key} {...link} />
       ))}
     </>
   );

diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts
@@ -205,11 +205,11 @@ export type LinkDescriptor = HtmlLinkDescriptor | PrefetchPageDescriptor;
  * Gets all the links for a set of matches. The modules are assumed to have been
  * loaded already.
  */
-export function getLinksForMatches(
+export function getKeyedLinksForMatches(
   matches: AgnosticDataRouteMatch[],
   routeModules: RouteModules,
   manifest: AssetsManifest
-): LinkDescriptor[] {
+): KeyedLinkDescriptor[] {
   let descriptors = matches
     .map((match): LinkDescriptor[] => {
       let module = routeModules[match.route.id];
@@ -218,7 +218,7 @@ export function getLinksForMatches(
     .flat(1);
 
   let preloads = getCurrentPageModulePreloadHrefs(matches, manifest);
-  return dedupe(descriptors, preloads);
+  return dedupeLinkDescriptors(descriptors, preloads);
 }
 
 let stylesheetPreloadTimeouts = 0;
@@ -339,11 +339,13 @@ function isHtmlLinkDescriptor(object: any): object is HtmlLinkDescriptor {
   return typeof object.rel === "string" && typeof object.href === "string";
 }
 
-export async function getStylesheetPrefetchLinks(
+export type KeyedHtmlLinkDescriptor = { key: string; link: HtmlLinkDescriptor };
+
+export async function getKeyedPrefetchLinks(
   matches: AgnosticDataRouteMatch[],
   manifest: AssetsManifest,
   routeModules: RouteModules
-): Promise<HtmlLinkDescriptor[]> {
+): Promise<KeyedHtmlLinkDescriptor[]> {
   let links = await Promise.all(
     matches.map(async (match) => {
       let mod = await loadRouteModule(
@@ -354,15 +356,17 @@ export async function getStylesheetPrefetchLinks(
     })
   );
 
-  return links
-    .flat(1)
-    .filter(isHtmlLinkDescriptor)
-    .filter((link) => link.rel === "stylesheet" || link.rel === "preload")
-    .map((link) =>
-      link.rel === "preload"
-        ? ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor)
-        : ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor)
-    );
+  return dedupeLinkDescriptors(
+    links
+      .flat(1)
+      .filter(isHtmlLinkDescriptor)
+      .filter((link) => link.rel === "stylesheet" || link.rel === "preload")
+      .map((link) =>
+        link.rel === "stylesheet"
+          ? ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor)
+          : ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor)
+      )
+  );
 }
 
 // This is ridiculously identical to transition.ts `filterMatchesToLoad`
@@ -499,12 +503,32 @@ function dedupeHrefs(hrefs: string[]): string[] {
   return [...new Set(hrefs)];
 }
 
-export function dedupe(descriptors: LinkDescriptor[], preloads: string[]) {
+function sortKeys<Obj extends { [Key in keyof Obj]: Obj[Key] }>(obj: Obj): Obj {
+  let sorted = {} as Obj;
+  let keys = Object.keys(obj).sort();
+
+  for (let key of keys) {
+    sorted[key as keyof Obj] = obj[key as keyof Obj];
+  }
+
+  return sorted;
+}
+
+type KeyedLinkDescriptor<Descriptor extends LinkDescriptor = LinkDescriptor> = {
+  key: string;
+  link: Descriptor;
+};
+
+function dedupeLinkDescriptors<Descriptor extends LinkDescriptor>(
+  descriptors: Descriptor[],
+  preloads?: string[]
+): KeyedLinkDescriptor<Descriptor>[] {
   let set = new Set();
   let preloadsSet = new Set(preloads);
 
   return descriptors.reduce((deduped, descriptor) => {
     let alreadyModulePreload =
+      preloads &&
       !isPageLinkDescriptor(descriptor) &&
       descriptor.as === "script" &&
       descriptor.href &&
@@ -514,14 +538,14 @@ export function dedupe(descriptors: LinkDescriptor[], preloads: string[]) {
       return deduped;
     }
 
-    let str = JSON.stringify(descriptor);
-    if (!set.has(str)) {
-      set.add(str);
-      deduped.push(descriptor);
+    let key = JSON.stringify(sortKeys(descriptor));
+    if (!set.has(key)) {
+      set.add(key);
+      deduped.push({ key, link: descriptor });
     }
 
     return deduped;
-  }, [] as LinkDescriptor[]);
+  }, [] as KeyedLinkDescriptor<Descriptor>[]);
 }
 
 // https://github.com/remix-run/history/issues/897