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 image has unused sizes prop #31064

Merged
merged 7 commits into from Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
6 changes: 4 additions & 2 deletions docs/api-reference/next/image.md
Expand Up @@ -119,9 +119,11 @@ const MyImage = (props) => {

A string that provides information about how wide the image will be at different breakpoints. Defaults to `100vw` (the full width of the screen) when using `layout="responsive"` or `layout="fill"`.

`sizes` is important for performance when using `layout="responsive"` or `layout="fill"` with images that take up less than the full viewport width.
If you are using `layout="fill"` or `layout="responsive"`, it's important to assign `sizes` for any image that takes up less than the full viewport width.

If you are using `layout="fill"` or `layout="responsive"` and the image will always be less than half the viewport width, include `sizes="50vw"`. Without `sizes`, the image will be sent at twice the necessary resolution, decreasing performance.
For example, when the parent element will constrain the image to always be less than half the viewport width, use `sizes="50vw"`. Without `sizes`, the image will be sent at twice the necessary resolution, decreasing performance.

If you are using `layout="intrinsic"` or `layout="fixed"`, then `sizes` is not needed because the upperbound width is constrained already.

[Learn more](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-sizes).

Expand Down
5 changes: 5 additions & 0 deletions packages/next/client/image.tsx
Expand Up @@ -418,6 +418,11 @@ export default function Image({
`Image with src "${src}" has both "priority" and "loading='lazy'" properties. Only one should be used.`
)
}
if (sizes && layout !== 'fill' && layout !== 'responsive') {
console.warn(
`Image with src "${src}" has "sizes" property but it will be ignored. Only use "sizes" with "layout='fill'" or "layout='responsive'".`
)
}
if (placeholder === 'blur') {
if (layout !== 'fill' && (widthInt || 0) * (heightInt || 0) < 1600) {
console.warn(
Expand Down
37 changes: 37 additions & 0 deletions test/integration/image-component/default/pages/invalid-sizes.js
@@ -0,0 +1,37 @@
import React from 'react'
import Image from 'next/image'

const Page = () => {
return (
<div>
<h1>Warn when "sizes" is not used</h1>
<Image
layout="fixed"
src="/test.png"
width={400}
height={400}
sizes="50vw"
/>
<Image
layout="intrinsic"
src="/test.jpg"
width={400}
height={400}
sizes="50vw"
/>
<Image
layout="responsive"
src="/test.webp"
width={400}
height={400}
sizes="50vw"
/>
<div style={{ position: 'relative', width: '200px', height: '200px' }}>
<Image src="/test.gif" layout="fill" objectFit="cover" sizes="50vw" />
</div>
<footer>footer</footer>
</div>
)
}

export default Page
21 changes: 21 additions & 0 deletions test/integration/image-component/default/test/index.test.js
Expand Up @@ -726,6 +726,27 @@ function runTests(mode) {
/Image with src (.*)tiff(.*) has a "loader" property that does not implement width/gm
)
})

it('should warn when using sizes with incorrect layout', async () => {
const browser = await webdriver(appPort, '/invalid-sizes')
await browser.eval(`document.querySelector("footer").scrollIntoView()`)
const warnings = (await browser.log('browser'))
.map((log) => log.message)
.join('\n')
expect(await hasRedbox(browser)).toBe(false)
expect(warnings).toMatch(
/Image with src (.*)png(.*) has "sizes" property but it will be ignored/gm
)
expect(warnings).toMatch(
/Image with src (.*)jpg(.*) has "sizes" property but it will be ignored/gm
)
expect(warnings).not.toMatch(
/Image with src (.*)webp(.*) has "sizes" property but it will be ignored/gm
)
expect(warnings).not.toMatch(
/Image with src (.*)gif(.*) has "sizes" property but it will be ignored/gm
)
})
} else {
//server-only tests
it('should not create an image folder in server/chunks', async () => {
Expand Down