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

Add warning when LCP image is missing priority prop #30221

Merged
merged 7 commits into from Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api-reference/next/image.md
Expand Up @@ -134,7 +134,7 @@ The quality of the optimized image, an integer between `1` and `100` where `100`
When true, the image will be considered high priority and
[preload](https://web.dev/preload-responsive-images/). Lazy loading is automatically disabled for images using `priority`.

You should use the `priority` attribute on any image which you suspect will be the [Largest Contentful Paint (LCP) element](https://nextjs.org/learn/seo/web-performance/lcp). It may be appropriate to have multiple priority images, as different images may be the LCP element for different viewport sizes.
You should use the `priority` property on any image detected as the [Largest Contentful Paint (LCP)](https://nextjs.org/learn/seo/web-performance/lcp) element. It may be appropriate to have multiple priority images, as different images may be the LCP element for different viewport sizes.

Should only be used when the image is visible above the fold. Defaults to `false`.

Expand Down
41 changes: 41 additions & 0 deletions packages/next/client/image.tsx
Expand Up @@ -10,6 +10,11 @@ import {
import { useIntersection } from './use-intersection'

const loadedImageURLs = new Set<string>()
const allImgs = new Map<
string,
{ src: string; priority: boolean; placeholder: string }
>()
let perfObserver: PerformanceObserver | undefined
const emptyDataURL =
'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'

Expand Down Expand Up @@ -450,6 +455,30 @@ export default function Image({
`\nRead more: https://nextjs.org/docs/messages/next-image-missing-loader-width`
)
}

if (typeof window !== 'undefined' && !perfObserver) {
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
perfObserver = new PerformanceObserver((entryList) => {
for (const entry of entryList.getEntries()) {
// @ts-ignore - missing "LargestContentfulPaint" class with "element" prop
const imgSrc = entry?.element?.src || ''
const lcpImage = allImgs.get(imgSrc)
if (
lcpImage &&
!lcpImage.priority &&
lcpImage.placeholder !== 'blur' &&
!lcpImage.src.startsWith('data:') &&
!lcpImage.src.startsWith('blob:')
) {
// https://web.dev/lcp/#measure-lcp-in-javascript
console.warn(
`Image with src "${lcpImage.src}" was detected as the Largest Contentful Paint (LCP). Please add the "priority" property if this image is above the fold.` +
`\nRead more: https://nextjs.org/docs/api-reference/next/image#priority`
)
}
}
})
perfObserver.observe({ type: 'largest-contentful-paint', buffered: true })
}
}

const [setRef, isIntersected] = useIntersection<HTMLImageElement>({
Expand Down Expand Up @@ -580,6 +609,18 @@ export default function Image({

let srcString: string = src

if (process.env.NODE_ENV !== 'production') {
if (typeof window !== 'undefined') {
let fullUrl: URL
try {
fullUrl = new URL(imgAttributes.src)
} catch (e) {
fullUrl = new URL(imgAttributes.src, window.location.href)
}
allImgs.set(fullUrl.href, { src, priority, placeholder })
}
}

return (
<span style={wrapperStyle}>
{hasSizer ? (
Expand Down
@@ -0,0 +1,27 @@
import React from 'react'
import Image from 'next/image'

const Page = () => {
return (
<div>
<h1>Priority Missing Warning Page</h1>
<Image
id="responsive"
layout="responsive"
src="/wide.png"
width="1200"
height="700"
/>
<Image
id="fixed"
layout="fixed"
src="/test.jpg"
width="400"
height="400"
/>
<footer>Priority Missing Warning Footer</footer>
</div>
)
}

export default Page
36 changes: 36 additions & 0 deletions test/integration/image-component/default/test/index.test.js
Expand Up @@ -135,6 +135,13 @@ function runTests(mode) {
'/_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',
},
])

const warnings = (await browser.log('browser'))
.map((log) => log.message)
.join('\n')
expect(warnings).not.toMatch(
/was detected as the Largest Contentful Paint/gm
)
} finally {
if (browser) {
await browser.close()
Expand Down Expand Up @@ -658,6 +665,35 @@ function runTests(mode) {
)
expect(warnings).not.toMatch(/cannot appear as a descendant/gm)
})

it('should warn when priority prop is missing on LCP image', async () => {
let browser
try {
browser = await webdriver(appPort, '/priority-missing-warning')
// Wait for image to load:
await check(async () => {
const result = await browser.eval(
`document.getElementById('responsive').naturalWidth`
)
if (result < 1) {
throw new Error('Image not ready')
}
return 'done'
}, 'done')
await waitFor(1000)
const warnings = (await browser.log('browser'))
.map((log) => log.message)
.join('\n')
expect(await hasRedbox(browser)).toBe(false)
expect(warnings).toMatch(
/Image with src (.*)wide.png(.*) was detected as the Largest Contentful Paint/gm
)
} finally {
if (browser) {
await browser.close()
}
}
})
} else {
//server-only tests
it('should not create an image folder in server/chunks', async () => {
Expand Down