Skip to content

Commit

Permalink
Fix next/image being downloaded multiple times on Safari (#22902)
Browse files Browse the repository at this point in the history
Currently if you have `sizes` set in `next/image`, the image will likely be downloaded multiple times (usually twice) on Safari (macOS and iOS): the correct size for the viewport, and the original size specified in `src`. 

Also make sure you have "Ignore Resource Cache" disabled in the Safari Devtools when trying to reproduce:

![CleanShot 2021-03-09 at 21 05 54@2x](https://user-images.githubusercontent.com/3676859/110476820-6399f180-811d-11eb-93ec-5b2482c87884.png)

The root cause is the way Safari handles `<img>`'s attribute updates. Although React updates all the attributes one by one synchronously and programmatically, Safari will still try to fetch the resource immediately and won't wait for other DOM changes to be finished. 

That means if we set the following 3 attributes in this order: `src`, `srcSet`, `sizes`. Safari will fetch the image when `src` is set. And then once `srcSet` is there it will fetch the resource again based on it. And finally, when `sizes` is updated it might correct the resource URL again.

So the fix here is simple: by just reordering those to `sizes`, `srcSet`, `src`, it will only load the image with the correct size only once:

<img width="1498" alt="CleanShot 2021-03-09 at 21 05 30@2x" src="https://user-images.githubusercontent.com/3676859/110477852-a27c7700-811e-11eb-88dc-d6e7895f67bd.png">

Fixes #19478.
  • Loading branch information
shuding authored Mar 9, 2021
1 parent e84571d commit 7dd99fa
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
9 changes: 8 additions & 1 deletion packages/next/client/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ function generateImgAttrs({
const last = widths.length - 1

return {
src: loader({ src, quality, width: widths[last] }),
sizes: !sizes && kind === 'w' ? '100vw' : sizes,
srcSet: widths
.map(
Expand All @@ -177,6 +176,14 @@ function generateImgAttrs({
}${kind}`
)
.join(', '),

// It's intended to keep `src` the last attribute because React updates
// attributes in order. If we keep `src` the first one, Safari will
// immediately start to fetch `src`, before `sizes` and `srcSet` are even
// updated by React. That causes multiple unnecessary requests if `srcSet`
// and `sizes` are defined.
// This bug cannot be reproduced in Chrome or Firefox.
src: loader({ src, quality, width: widths[last] }),
}
}

Expand Down
7 changes: 7 additions & 0 deletions test/integration/image-component/basic/pages/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ const Page = () => {
return (
<div>
<p>Hello World</p>
<Image
id="image-with-sizes"
src="/test-sizes.jpg"
width={2000}
height={100}
sizes="100vw"
/>
<Image
id="basic-image"
src="foo.jpg"
Expand Down
12 changes: 12 additions & 0 deletions test/integration/image-component/basic/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,18 @@ describe('Image Component Tests', () => {
)
).toBe(false)
})
it('should only be loaded once if `sizes` is set', async () => {
// Get all network requests
const resourceEntries = await browser.eval(
'window.performance.getEntries()'
)

// "test-sizes.jpg" should only occur once
const requests = resourceEntries.filter((entry) =>
entry.name.includes('test-sizes.jpg')
)
expect(requests.length).toBe(1)
})
describe('Client-side Errors', () => {
beforeAll(async () => {
await browser.eval(`(function() {
Expand Down

0 comments on commit 7dd99fa

Please sign in to comment.