fix: add missing <preload> for next/image in App Router (#52425)

- Depends on facebook/react#27096 - Fixes #43134 - Closes NEXT-811 - Closes NEXT-846
vercel · Jul 14, 2023 · c3ca17c · c3ca17c
1 parent d10f105
commit c3ca17c
Show file tree

Hide file tree

Showing 6 changed files with 140 additions and 77 deletions.
diff --git a/packages/next/src/client/image-component.tsx b/packages/next/src/client/image-component.tsx
@@ -10,6 +10,7 @@ import React, {
   forwardRef,
   version,
 } from 'react'
+import { preload } from 'react-dom'
 import Head from '../shared/lib/head'
 import { getImgProps } from '../shared/lib/get-img-props'
 import type {
@@ -26,6 +27,7 @@ import type {
 import { imageConfigDefault } from '../shared/lib/image-config'
 import { ImageConfigContext } from '../shared/lib/image-config-context'
 import { warnOnce } from '../shared/lib/utils/warn-once'
+import { RouterContext } from '../shared/lib/router-context'
 
 // @ts-ignore - This is replaced by webpack alias
 import defaultLoader from 'next/dist/shared/lib/image-loader'
@@ -302,8 +304,60 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
   }
 )
 
+function ImagePreload({
+  isAppRouter,
+  imgAttributes,
+}: {
+  isAppRouter: boolean
+  imgAttributes: ImgProps
+}) {
+  const opts = {
+    as: 'image',
+    imageSrcSet: imgAttributes.srcSet,
+    imageSizes: imgAttributes.sizes,
+    crossOrigin: imgAttributes.crossOrigin,
+    referrerPolicy: imgAttributes.referrerPolicy,
+    ...getDynamicProps(imgAttributes.fetchPriority),
+  }
+
+  if (isAppRouter) {
+    // See https://github.com/facebook/react/pull/26940
+    preload(
+      imgAttributes.src,
+      // @ts-expect-error TODO: upgrade to `@types/react-dom@18.3.x`
+      opts
+    )
+    return null
+  }
+
+  return (
+    <Head>
+      <link
+        key={
+          '__nimg-' +
+          imgAttributes.src +
+          imgAttributes.srcSet +
+          imgAttributes.sizes
+        }
+        rel="preload"
+        // Note how we omit the `href` attribute, as it would only be relevant
+        // for browsers that do not support `imagesrcset`, and in those cases
+        // it would cause the incorrect image to be preloaded.
+        //
+        // https://html.spec.whatwg.org/multipage/semantics.html#attr-link-imagesrcset
+        href={imgAttributes.srcSet ? undefined : imgAttributes.src}
+        {...opts}
+      />
+    </Head>
+  )
+}
+
 export const Image = forwardRef<HTMLImageElement | null, ImageProps>(
   (props, forwardedRef) => {
+    const pagesRouter = useContext(RouterContext)
+    // We're in the app directory if there is no pages router.
+    const isAppRouter = !pagesRouter
+
     const configContext = useContext(ImageConfigContext)
     const config = useMemo(() => {
       const c = configEnv || configContext || imageConfigDefault
@@ -351,29 +405,10 @@ export const Image = forwardRef<HTMLImageElement | null, ImageProps>(
           />
         }
         {imgMeta.priority ? (
-          // Note how we omit the `href` attribute, as it would only be relevant
-          // for browsers that do not support `imagesrcset`, and in those cases
-          // it would likely cause the incorrect image to be preloaded.
-          //
-          // https://html.spec.whatwg.org/multipage/semantics.html#attr-link-imagesrcset
-          <Head>
-            <link
-              key={
-                '__nimg-' +
-                imgAttributes.src +
-                imgAttributes.srcSet +
-                imgAttributes.sizes
-              }
-              rel="preload"
-              as="image"
-              href={imgAttributes.srcSet ? undefined : imgAttributes.src}
-              imageSrcSet={imgAttributes.srcSet}
-              imageSizes={imgAttributes.sizes}
-              crossOrigin={imgAttributes.crossOrigin}
-              referrerPolicy={imgAttributes.referrerPolicy}
-              {...getDynamicProps(imgAttributes.fetchPriority)}
-            />
-          </Head>
+          <ImagePreload
+            isAppRouter={isAppRouter}
+            imgAttributes={imgAttributes}
+          />
         ) : null}
       </>
     )

diff --git a/test/integration/next-image-new/app-dir/app/priority/page.js b/test/integration/next-image-new/app-dir/app/priority/page.js
@@ -17,10 +17,10 @@ const Page = () => {
         priority
         id="basic-image-crossorigin"
         alt="basic-image-crossorigin"
-        src="/test.jpg"
+        src="/test.webp"
         width="400"
         height="400"
-        crossOrigin="anonymous"
+        crossOrigin="use-credentials"
       ></Image>
       <Image
         priority
@@ -36,8 +36,8 @@ const Page = () => {
         id="load-eager"
         alt="load-eager"
         src="/test.png"
-        width="400"
-        height="400"
+        width="200"
+        height="200"
       ></Image>
       <Image
         priority

diff --git a/test/integration/next-image-new/app-dir/test/index.test.ts b/test/integration/next-image-new/app-dir/test/index.test.ts
@@ -101,8 +101,7 @@ function runTests(mode) {
     }
   })
 
-  // TODO: need to add <link preload> to app dir
-  it.skip('should preload priority images', async () => {
+  it('should preload priority images', async () => {
     let browser
     try {
       browser = await webdriver(appPort, '/priority')
@@ -125,20 +124,30 @@ function runTests(mode) {
         const fetchpriority = await link.getAttribute('fetchpriority')
         const imagesrcset = await link.getAttribute('imagesrcset')
         const imagesizes = await link.getAttribute('imagesizes')
-        entries.push({ fetchpriority, imagesrcset, imagesizes })
+        const crossorigin = await link.getAttribute('crossorigin')
+        const referrerpolicy = await link.getAttribute('referrerPolicy')
+        entries.push({
+          fetchpriority,
+          imagesrcset,
+          imagesizes,
+          crossorigin,
+          referrerpolicy,
+        })
       }
 
       expect(
         entries.find(
           (item) =>
             item.imagesrcset ===
-            '/_next/image?url=%2Ftest.jpg&w=640&q=75 1x, /_next/image?url=%2Ftest.jpg&w=828&q=75 2x'
+            '/_next/image?url=%2Ftest.webp&w=640&q=75 1x, /_next/image?url=%2Ftest.webp&w=828&q=75 2x'
         )
       ).toEqual({
         fetchpriority: 'high',
         imagesizes: '',
         imagesrcset:
-          '/_next/image?url=%2Ftest.jpg&w=640&q=75 1x, /_next/image?url=%2Ftest.jpg&w=828&q=75 2x',
+          '/_next/image?url=%2Ftest.webp&w=640&q=75 1x, /_next/image?url=%2Ftest.webp&w=828&q=75 2x',
+        crossorigin: 'use-credentials',
+        referrerpolicy: '',
       })
 
       expect(
@@ -152,6 +161,23 @@ function runTests(mode) {
         imagesizes: '100vw',
         imagesrcset:
           '/_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w',
+        crossorigin: '',
+        referrerpolicy: '',
+      })
+
+      expect(
+        entries.find(
+          (item) =>
+            item.imagesrcset ===
+            '/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x'
+        )
+      ).toEqual({
+        fetchpriority: 'high',
+        imagesizes: '',
+        imagesrcset:
+          '/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x',
+        crossorigin: '',
+        referrerpolicy: 'no-referrer',
       })
 
       // When priority={true}, we should _not_ set loading="lazy"
@@ -197,20 +223,6 @@ function runTests(mode) {
         /was detected as the Largest Contentful Paint/gm
       )
       expect(warnings).not.toMatch(/React does not recognize the (.+) prop/gm)
-
-      // should preload with crossorigin
-      expect(
-        await browser.elementsByCss(
-          'link[rel=preload][as=image][crossorigin=anonymous][imagesrcset*="test.jpg"]'
-        )
-      ).toHaveLength(1)
-
-      // should preload with referrerpolicy
-      expect(
-        await browser.elementsByCss(
-          'link[rel=preload][as=image][referrerpolicy="no-referrer"][imagesrcset*="test.png"]'
-        )
-      ).toHaveLength(1)
     } finally {
       if (browser) {
         await browser.close()

diff --git a/test/integration/next-image-new/default/pages/priority.js b/test/integration/next-image-new/default/pages/priority.js
@@ -17,10 +17,10 @@ const Page = () => {
         priority
         id="basic-image-crossorigin"
         alt="basic-image-crossorigin"
-        src="/test.jpg"
+        src="/test.webp"
         width="400"
         height="400"
-        crossOrigin="anonymous"
+        crossOrigin="use-credentials"
       ></Image>
       <Image
         priority
@@ -36,8 +36,8 @@ const Page = () => {
         id="load-eager"
         alt="load-eager"
         src="/test.png"
-        width="400"
-        height="400"
+        width="200"
+        height="200"
       ></Image>
       <Image
         priority

diff --git a/test/integration/next-image-new/default/test/index.test.ts b/test/integration/next-image-new/default/test/index.test.ts
@@ -125,19 +125,30 @@ function runTests(mode) {
         const fetchpriority = await link.getAttribute('fetchpriority')
         const imagesrcset = await link.getAttribute('imagesrcset')
         const imagesizes = await link.getAttribute('imagesizes')
-        entries.push({ fetchpriority, imagesrcset, imagesizes })
+        const crossorigin = await link.getAttribute('crossorigin')
+        const referrerpolicy = await link.getAttribute('referrerPolicy')
+        entries.push({
+          fetchpriority,
+          imagesrcset,
+          imagesizes,
+          crossorigin,
+          referrerpolicy,
+        })
       }
+
       expect(
         entries.find(
           (item) =>
             item.imagesrcset ===
-            '/_next/image?url=%2Ftest.jpg&w=640&q=75 1x, /_next/image?url=%2Ftest.jpg&w=828&q=75 2x'
+            '/_next/image?url=%2Ftest.webp&w=640&q=75 1x, /_next/image?url=%2Ftest.webp&w=828&q=75 2x'
         )
       ).toEqual({
         fetchpriority: 'high',
         imagesizes: '',
         imagesrcset:
-          '/_next/image?url=%2Ftest.jpg&w=640&q=75 1x, /_next/image?url=%2Ftest.jpg&w=828&q=75 2x',
+          '/_next/image?url=%2Ftest.webp&w=640&q=75 1x, /_next/image?url=%2Ftest.webp&w=828&q=75 2x',
+        crossorigin: 'use-credentials',
+        referrerpolicy: '',
       })
 
       expect(
@@ -151,6 +162,23 @@ function runTests(mode) {
         imagesizes: '100vw',
         imagesrcset:
           '/_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w',
+        crossorigin: '',
+        referrerpolicy: '',
+      })
+
+      expect(
+        entries.find(
+          (item) =>
+            item.imagesrcset ===
+            '/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x'
+        )
+      ).toEqual({
+        fetchpriority: 'high',
+        imagesizes: '',
+        imagesrcset:
+          '/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x',
+        crossorigin: '',
+        referrerpolicy: 'no-referrer',
       })
 
       // When priority={true}, we should _not_ set loading="lazy"
@@ -196,22 +224,6 @@ function runTests(mode) {
         /was detected as the Largest Contentful Paint/gm
       )
       expect(warnings).not.toMatch(/React does not recognize the (.+) prop/gm)
-
-      // should preload with crossorigin
-      expect(
-        (
-          await browser.elementsByCss(
-            'link[rel=preload][as=image][crossorigin=anonymous][imagesrcset*="test.jpg"]'
-          )
-        ).length
-      ).toBeGreaterThanOrEqual(1)
-
-      // should preload with referrerpolicy
-      expect(
-        await browser.elementsByCss(
-          'link[rel=preload][as=image][referrerpolicy="no-referrer"][imagesrcset*="test.png"]'
-        )
-      ).toHaveLength(1)
     } finally {
       if (browser) {
         await browser.close()

diff --git a/test/unit/next-image-new.test.ts b/test/unit/next-image-new.test.ts
@@ -1,9 +1,13 @@
 /* eslint-env jest */
 import React from 'react'
-import ReactDOM from 'react-dom/server'
+import ReactDOMServer from 'react-dom/server'
 import Image from 'next/image'
 import cheerio from 'cheerio'
 
+// Since this unit test doesn't check the result of
+// ReactDOM.preload(), we can turn it into a noop.
+jest.mock('react-dom', () => ({ preload: () => null }))
+
 describe('Image rendering', () => {
   it('should render Image on its own', async () => {
     const element = React.createElement(Image, {
@@ -14,7 +18,7 @@ describe('Image rendering', () => {
       height: 100,
       loading: 'eager',
     })
-    const html = ReactDOM.renderToString(element)
+    const html = ReactDOMServer.renderToString(element)
     const $ = cheerio.load(html)
     const img = $('#unit-image')
     // order matters here
@@ -54,9 +58,9 @@ describe('Image rendering', () => {
       width: 100,
       height: 100,
     })
-    const $ = cheerio.load(ReactDOM.renderToString(element))
-    const $2 = cheerio.load(ReactDOM.renderToString(element2))
-    const $lazy = cheerio.load(ReactDOM.renderToString(elementLazy))
+    const $ = cheerio.load(ReactDOMServer.renderToString(element))
+    const $2 = cheerio.load(ReactDOMServer.renderToString(element2))
+    const $lazy = cheerio.load(ReactDOMServer.renderToString(elementLazy))
     expect($('noscript').length).toBe(0)
     expect($2('noscript').length).toBe(0)
     expect($lazy('noscript').length).toBe(0)
@@ -90,9 +94,9 @@ describe('Image rendering', () => {
       blurDataURL: 'data:image/png;base64',
       priority: true,
     })
-    const $1 = cheerio.load(ReactDOM.renderToString(element1))
-    const $2 = cheerio.load(ReactDOM.renderToString(element2))
-    const $3 = cheerio.load(ReactDOM.renderToString(element3))
+    const $1 = cheerio.load(ReactDOMServer.renderToString(element1))
+    const $2 = cheerio.load(ReactDOMServer.renderToString(element2))
+    const $3 = cheerio.load(ReactDOMServer.renderToString(element3))
     expect($1('noscript').length).toBe(0)
     expect($2('noscript').length).toBe(0)
     expect($3('noscript').length).toBe(0)