Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

81 changes: 58 additions & 23 deletions packages/next/src/client/image-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
</>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
50 changes: 31 additions & 19 deletions test/integration/next-image-new/app-dir/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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(
Expand All @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
8 changes: 4 additions & 4 deletions test/integration/next-image-new/default/pages/priority.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
50 changes: 31 additions & 19 deletions test/integration/next-image-new/default/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
20 changes: 12 additions & 8 deletions test/unit/next-image-new.test.ts
Original file line number Diff line number Diff line change
@@ -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, {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down