fix: Improved support for imageSrcSet + imageSizes in route `link…

…s` (#184) (#2944) Improves support for returned links that include an `imagesrcset` attribute to preload images when matched against a responsive set of URLs. This only works when `rel === 'preload'` and `as === 'image'`. https://html.spec.whatwg.org/commit-snapshots/cb4f5ff75de5f4cbd7013c4abad02f21c77d4d1c/#attr-link-imagesrcset Prior to this change, a few problems with our implementation: 1. Our types for `HtmlLinkDescriptor` make `href` required. This is true _unless_ a link includes `imagesrcset` and `imagesizes`, though `imagesrcset` and `href` will also work with or without `imagesizes` according to the spec. This change uses an intersection type to account for these cases. 2. React 17 was released prior to these attributes being accepted into the spec and implemented by browsers, so passing the props as camel-cased will issue a warning about unsupported DOM props. Our types recognized this and supported the all-lowercase version to skip the warning and forward the attribute to the node, but React 18 now recognizes the camel-cased version. This change updates our types to use the camel-cased prop names, but it will support either at runtime and normalize the object based on the detected version of React. 3. Because we use `href` + `rel` as a key, links without an `href` will have duplicate keys. So `imageSrcSet` is added as a fallback to prevent that.
remix-run · May 20, 2022 · d19d888 · d19d888
1 parent 000f37c
commit d19d888
Show file tree

Hide file tree

Showing 4 changed files with 275 additions and 59 deletions.
diff --git a/integration/link-test.ts b/integration/link-test.ts
@@ -37,6 +37,10 @@ test.describe("route module link export", () => {
 
         "app/guitar.jpg": js``,
 
+        "app/guitar-600.jpg": js``,
+
+        "app/guitar-900.jpg": js``,
+
         "app/reset.css": css`
           * {
             box-sizing: border-box;
@@ -267,6 +271,39 @@ test.describe("route module link export", () => {
           }
         `,
 
+        "app/routes/responsive-image-preload.jsx": js`
+          import { Link } from "@remix-run/react";
+          import guitar600 from "~/guitar-600.jpg";
+          import guitar900 from "~/guitar-900.jpg";
+
+          export function links()  {
+            return [
+              {
+                rel: "preload",
+                as: "image",
+                imageSrcSet: guitar600 + " 600w, " + guitar900 + " 900w",
+                imageSizes: "100vw",
+              },
+            ];
+          }
+          export default function LinksPage() {
+            return (
+              <div data-test-id="/responsive-image-preload">
+                <h2>Responsive Guitar</h2>
+                <p>
+                  <img
+                    alt="a guitar"
+                    srcSet={guitar600 + " 600w, " + guitar900 + " 900w"}
+                    sizes="100vw"
+                    data-test-id="blocked"
+                  />{" "}
+                  Prefetched because it's a preload.
+                </p>
+              </div>
+            );
+          }
+        `,
+
         "app/routes/gists.jsx": js`
           import { json } from "@remix-run/node";
           import { Link, Outlet, useLoaderData, useTransition } from "@remix-run/react";
@@ -438,6 +475,16 @@ test.describe("route module link export", () => {
     await appFixture.close();
   });
 
+  test("adds responsive image preload links to the document", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/responsive-image-preload");
+    await page.waitForSelector('[data-test-id="/responsive-image-preload"]');
+    let locator = page.locator("link[rel=preload][as=image]");
+    expect(await locator.getAttribute("imagesizes")).toBe("100vw");
+  });
+
   test("waits for new styles to load before transitioning", async ({
     page,
   }) => {
@@ -472,5 +519,15 @@ test.describe("route module link export", () => {
       await page.waitForSelector('[data-test-id="/links"]');
       expect(responses.length).toEqual(4);
     });
+
+    test("adds responsive image preload links to the document", async ({
+      page,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/responsive-image-preload");
+      await page.waitForSelector('[data-test-id="/responsive-image-preload"]');
+      let locator = page.locator("link[rel=preload][as=image]");
+      expect(await locator.getAttribute("imagesizes")).toBe("100vw");
+    });
   });
 });
diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx
@@ -532,13 +532,46 @@ export function Links() {
 
   return (
     <>
-      {links.map((link) =>
-        isPageLinkDescriptor(link) ? (
-          <PrefetchPageLinks key={link.page} {...link} />
-        ) : (
-          <link key={link.rel + link.href} {...link} />
-        )
-      )}
+      {links.map((link) => {
+        if (isPageLinkDescriptor(link)) {
+          return <PrefetchPageLinks key={link.page} {...link} />;
+        }
+
+        let imageSrcSet: string | null = null;
+
+        // In React 17, <link imageSrcSet> and <link imageSizes> will warn
+        // because the DOM attributes aren't recognized, so users need to pass
+        // them in all lowercase to forward the attributes to the node without a
+        // warning. Normalize so that either property can be used in Remix.
+        if ("useId" in React) {
+          if (link.imagesrcset) {
+            link.imageSrcSet = imageSrcSet = link.imagesrcset;
+            delete link.imagesrcset;
+          }
+
+          if (link.imagesizes) {
+            link.imageSizes = link.imagesizes;
+            delete link.imagesizes;
+          }
+        } else {
+          if (link.imageSrcSet) {
+            link.imagesrcset = imageSrcSet = link.imageSrcSet;
+            delete link.imageSrcSet;
+          }
+
+          if (link.imageSizes) {
+            link.imagesizes = link.imageSizes;
+            delete link.imageSizes;
+          }
+        }
+
+        return (
+          <link
+            key={link.rel + (link.href || "") + (imageSrcSet || "")}
+            {...link}
+          />
+        );
+      })}
     </>
   );
 }

diff --git a/packages/remix-react/links.ts b/packages/remix-react/links.ts
@@ -16,16 +16,11 @@ type LiteralUnion<LiteralType, BaseType extends Primitive> =
   | LiteralType
   | (BaseType & Record<never, never>);
 
-/**
- * Represents a `<link>` element.
- *
- * WHATWG Specification: https://html.spec.whatwg.org/multipage/semantics.html#the-link-element
- */
-export interface HtmlLinkDescriptor {
+interface HtmlLinkProps {
   /**
    * Address of the hyperlink
    */
-  href: string;
+  href?: string;
 
   /**
    * How the element handles crossorigin requests
@@ -91,16 +86,6 @@ export interface HtmlLinkDescriptor {
    */
   sizes?: string;
 
-  /**
-   * Images to use in different situations, e.g., high-resolution displays, small monitors, etc. (for rel="preload")
-   */
-  imagesrcset?: string;
-
-  /**
-   * Image sizes for different page layouts (for rel="preload")
-   */
-  imagesizes?: string;
-
   /**
    * Potential destination for a preload request (for rel="preload" and rel="modulepreload")
    */
@@ -143,15 +128,82 @@ export interface HtmlLinkDescriptor {
    * The title attribute has special semantics on this element: Title of the link; CSS style sheet set name.
    */
   title?: string;
+
+  /**
+   * Images to use in different situations, e.g., high-resolution displays,
+   * small monitors, etc. (for rel="preload")
+   */
+  imageSrcSet?: string;
+
+  /**
+   * Image sizes for different page layouts (for rel="preload")
+   */
+  imageSizes?: string;
 }
 
+interface HtmlLinkPreloadImage extends HtmlLinkProps {
+  /**
+   * Relationship between the document containing the hyperlink and the destination resource
+   */
+  rel: "preload";
+
+  /**
+   * Potential destination for a preload request (for rel="preload" and rel="modulepreload")
+   */
+  as: "image";
+
+  /**
+   * Address of the hyperlink
+   */
+  href?: string;
+
+  /**
+   * Images to use in different situations, e.g., high-resolution displays,
+   * small monitors, etc. (for rel="preload")
+   */
+  imageSrcSet: string;
+
+  /**
+   * Image sizes for different page layouts (for rel="preload")
+   */
+  imageSizes?: string;
+}
+
+/**
+ * Represents a `<link>` element.
+ *
+ * WHATWG Specification: https://html.spec.whatwg.org/multipage/semantics.html#the-link-element
+ */
+export type HtmlLinkDescriptor =
+  // Must have an href *unless* it's a `<link rel="preload" as="image">` with an
+  // `imageSrcSet` and `imageSizes` props
+  (
+    | (HtmlLinkProps & Pick<Required<HtmlLinkProps>, "href">)
+    | (HtmlLinkPreloadImage &
+        Pick<Required<HtmlLinkPreloadImage>, "imageSizes">)
+    | (HtmlLinkPreloadImage &
+        Pick<Required<HtmlLinkPreloadImage>, "href"> & { imageSizes?: never })
+  ) & {
+    /**
+     * @deprecated Use `imageSrcSet` instead.
+     */
+    imagesrcset?: string;
+
+    /**
+     * @deprecated Use `imageSizes` instead.
+     */
+    imagesizes?: string;
+  };
+
 export interface PrefetchPageDescriptor
   extends Omit<
     HtmlLinkDescriptor,
     | "href"
     | "rel"
     | "type"
     | "sizes"
+    | "imageSrcSet"
+    | "imageSizes"
     | "imagesrcset"
     | "imagesizes"
     | "as"
@@ -195,10 +247,14 @@ export async function prefetchStyleLinks(
   let descriptors = routeModule.links();
   if (!descriptors) return;
 
-  let styleLinks = [];
+  let styleLinks: HtmlLinkDescriptor[] = [];
   for (let descriptor of descriptors) {
     if (!isPageLinkDescriptor(descriptor) && descriptor.rel === "stylesheet") {
-      styleLinks.push({ ...descriptor, rel: "preload", as: "style" });
+      styleLinks.push({
+        ...descriptor,
+        rel: "preload",
+        as: "style",
+      } as HtmlLinkDescriptor);
     }
   }
 
@@ -250,17 +306,27 @@ export function isPageLinkDescriptor(
 export function isHtmlLinkDescriptor(
   object: any
 ): object is HtmlLinkDescriptor {
-  return (
-    object != null &&
-    typeof object.rel === "string" &&
-    typeof object.href === "string"
-  );
+  if (object == null) return false;
+
+  // <link> may not have an href if <link rel="preload"> is used with imagesrcset + imagesizes
+  // https://github.com/remix-run/remix/issues/184
+  // https://html.spec.whatwg.org/commit-snapshots/cb4f5ff75de5f4cbd7013c4abad02f21c77d4d1c/#attr-link-imagesrcset
+  if (object.href == null) {
+    return (
+      object.rel === "preload" &&
+      (typeof object.imageSrcSet === "string" ||
+        typeof object.imagesrcset === "string") &&
+      (typeof object.imageSizes === "string" ||
+        typeof object.imagesizes === "string")
+    );
+  }
+  return typeof object.rel === "string" && typeof object.href === "string";
 }
 
 export async function getStylesheetPrefetchLinks(
   matches: RouteMatch<ClientRoute>[],
   routeModules: RouteModules
-) {
+): Promise<HtmlLinkDescriptor[]> {
   let links = await Promise.all(
     matches.map(async (match) => {
       let mod = await loadRouteModule(match.route, routeModules);
@@ -272,10 +338,10 @@ export async function getStylesheetPrefetchLinks(
     .flat(1)
     .filter(isHtmlLinkDescriptor)
     .filter((link) => link.rel === "stylesheet" || link.rel === "preload")
-    .map(({ rel, ...attrs }) =>
-      rel === "preload"
-        ? { rel: "prefetch", ...attrs }
-        : { rel: "prefetch", as: "style", ...attrs }
+    .map((link) =>
+      link.rel === "preload"
+        ? ({ ...link, rel: "prefetch" } as HtmlLinkDescriptor)
+        : ({ ...link, rel: "prefetch", as: "style" } as HtmlLinkDescriptor)
     );
 }