Don't lazy-load already-loaded image in client-side transition #26968

atcastle · 2021-07-06T19:33:23Z

fixes #19074
This change disables image lazy-loading when both of the following are true:

A image is being rendered following a client-side page transition
The image has been previously loaded during this session.

Before this change, all images with lazy-loading enabled have a visible flicker during client-side page transitions, even though they're already loaded.

With this change, there's are two performance risks:

There's a chance that some offscreen images will have lazy-loading disabled unnecessarily because they were previously loaded. I think the performance hit here is pretty negligible and the situation is unlikely to come up very often.
There's a chance a different-sized version of the image will be selected by the browser, but lazy-loading will be disabled anyway. This seems even more unlikely to me, and anyway the performance hit from a stray un-lazy-loaded image (on a client-side transition) is very minor.

In both cases, I think the performance risk is outweighed by the UX improvement of getting rid of the image flicker on page transition.

fixes 19074

styfle

Thanks!

styfle · 2021-07-07T13:59:23Z

Will this also fix #20178?

ijjk · 2021-07-07T17:03:45Z

Failing test suites

Commit: eb681c5

test/integration/image-component/basic/test/index.test.js

Image Component Tests > Client-side Lazy Loading Tests > should have loaded the first image immediately
Image Component Tests > Client-side Lazy Loading Tests > should not have loaded the second image immediately
Image Component Tests > Client-side Lazy Loading Tests > should load the second image after scrolling down
Image Component Tests > Client-side Lazy Loading Tests > should load the third image, which is unoptimized, after scrolling further down
Image Component Tests > Client-side Lazy Loading Tests > should load the fourth image lazily after scrolling down
Image Component Tests > Client-side Lazy Loading Tests > should load the fifth image eagerly, without scrolling
Image Component Tests > SSR Lazy Loading Tests > should have loaded the first image immediately
Image Component Tests > SSR Lazy Loading Tests > should load the second image after scrolling down
Image Component Tests > SSR Lazy Loading Tests > should load the third image, which is unoptimized, after scrolling further down
Image Component Tests > SSR Lazy Loading Tests > should load the fourth image lazily after scrolling down
Image Component Tests > SSR Lazy Loading Tests > should load the fifth image eagerly, without scrolling

Expand output

● Image Component Tests › SSR Lazy Loading Tests › should have loaded the first image immediately

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy1.jpg?auto=format&fit=max&w=2000"
Received: "https://example.com/myaccount/foo1.jpg?auto=format&fit=max&w=2000"

  109 | function lazyLoadingTests() {
  110 |   it('should have loaded the first image immediately', async () => {
> 111 |     expect(await browser.elementById('lazy-top').getAttribute('src')).toBe(
      |                                                                       ^
  112 |       'https://example.com/myaccount/lazy1.jpg?auto=format&fit=max&w=2000'
  113 |     )
  114 |     expect(await browser.elementById('lazy-top').getAttribute('srcset')).toBe(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:111:71)

● Image Component Tests › SSR Lazy Loading Tests › should load the second image after scrolling down

thrown: "Exceeded timeout of 30000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  129 |     )
  130 |   })
> 131 |   it('should load the second image after scrolling down', async () => {
      |   ^
  132 |     let viewportHeight = await browser.eval(`window.innerHeight`)
  133 |     let topOfMidImage = await browser.eval(
  134 |       `document.getElementById('lazy-mid').parentElement.offsetTop`

  at lazyLoadingTests (integration/image-component/basic/test/index.test.js:131:3)
  at integration/image-component/basic/test/index.test.js:342:5
  at integration/image-component/basic/test/index.test.js:335:3
  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:239:1)

● Image Component Tests › SSR Lazy Loading Tests › should load the third image, which is unoptimized, after scrolling further down

expect(received).toBe(expected) // Object.is equality

Expected: "https://www.otherhost.com/lazy3.jpg"
Received: "https://www.otherhost.com/foo3.jpg"

  165 |     )
  166 |     await waitFor(200)
> 167 |     expect(await browser.elementById('lazy-bottom').getAttribute('src')).toBe(
      |                                                                          ^
  168 |       'https://www.otherhost.com/lazy3.jpg'
  169 |     )
  170 |     expect(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:167:74)

● Image Component Tests › SSR Lazy Loading Tests › should load the fourth image lazily after scrolling down

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy4.jpg?auto=format&fit=max&w=1600"
Received: "https://example.com/myaccount/foo4.jpg?auto=format&fit=max&w=1600"

  190 |     expect(
  191 |       await browser.elementById('lazy-without-attribute').getAttribute('src')
> 192 |     ).toBe('https://example.com/myaccount/lazy4.jpg?auto=format&fit=max&w=1600')
      |       ^
  193 |     expect(
  194 |       await browser.elementById('lazy-without-attribute').getAttribute('srcset')
  195 |     ).toBe(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:192:7)

● Image Component Tests › SSR Lazy Loading Tests › should load the fifth image eagerly, without scrolling

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy5.jpg?auto=format&fit=max&w=2000"
Received: "https://example.com/myaccount/foo5.jpg?auto=format&fit=max&w=2000"

  199 |
  200 |   it('should load the fifth image eagerly, without scrolling', async () => {
> 201 |     expect(await browser.elementById('eager-loading').getAttribute('src')).toBe(
      |                                                                            ^
  202 |       'https://example.com/myaccount/lazy5.jpg?auto=format&fit=max&w=2000'
  203 |     )
  204 |     expect(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:201:76)

● Image Component Tests › Client-side Lazy Loading Tests › should have loaded the first image immediately

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy1.jpg?auto=format&fit=max&w=2000"
Received: "https://example.com/myaccount/foo1.jpg?auto=format&fit=max&w=2000"

  109 | function lazyLoadingTests() {
  110 |   it('should have loaded the first image immediately', async () => {
> 111 |     expect(await browser.elementById('lazy-top').getAttribute('src')).toBe(
      |                                                                       ^
  112 |       'https://example.com/myaccount/lazy1.jpg?auto=format&fit=max&w=2000'
  113 |     )
  114 |     expect(await browser.elementById('lazy-top').getAttribute('srcset')).toBe(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:111:71)

● Image Component Tests › Client-side Lazy Loading Tests › should not have loaded the second image immediately

expect(received).toBe(expected) // Object.is equality

Expected: "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7"
Received: "https://example.com/myaccount/foo2.jpg?auto=format&fit=max&w=1024"

  117 |   })
  118 |   it('should not have loaded the second image immediately', async () => {
> 119 |     expect(await browser.elementById('lazy-mid').getAttribute('src')).toBe(
      |                                                                       ^
  120 |       emptyImage
  121 |     )
  122 |     expect(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:119:71)

● Image Component Tests › Client-side Lazy Loading Tests › should load the second image after scrolling down

thrown: "Exceeded timeout of 30000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  129 |     )
  130 |   })
> 131 |   it('should load the second image after scrolling down', async () => {
      |   ^
  132 |     let viewportHeight = await browser.eval(`window.innerHeight`)
  133 |     let topOfMidImage = await browser.eval(
  134 |       `document.getElementById('lazy-mid').parentElement.offsetTop`

  at lazyLoadingTests (integration/image-component/basic/test/index.test.js:131:3)
  at integration/image-component/basic/test/index.test.js:366:5
  at integration/image-component/basic/test/index.test.js:357:3
  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:239:1)

● Image Component Tests › Client-side Lazy Loading Tests › should load the third image, which is unoptimized, after scrolling further down

expect(received).toBe(expected) // Object.is equality

Expected: "https://www.otherhost.com/lazy3.jpg"
Received: "https://www.otherhost.com/foo3.jpg"

  165 |     )
  166 |     await waitFor(200)
> 167 |     expect(await browser.elementById('lazy-bottom').getAttribute('src')).toBe(
      |                                                                          ^
  168 |       'https://www.otherhost.com/lazy3.jpg'
  169 |     )
  170 |     expect(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:167:74)

● Image Component Tests › Client-side Lazy Loading Tests › should load the fourth image lazily after scrolling down

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy4.jpg?auto=format&fit=max&w=1600"
Received: "https://example.com/myaccount/foo4.jpg?auto=format&fit=max&w=1600"

  190 |     expect(
  191 |       await browser.elementById('lazy-without-attribute').getAttribute('src')
> 192 |     ).toBe('https://example.com/myaccount/lazy4.jpg?auto=format&fit=max&w=1600')
      |       ^
  193 |     expect(
  194 |       await browser.elementById('lazy-without-attribute').getAttribute('srcset')
  195 |     ).toBe(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:192:7)

● Image Component Tests › Client-side Lazy Loading Tests › should load the fifth image eagerly, without scrolling

expect(received).toBe(expected) // Object.is equality

Expected: "https://example.com/myaccount/lazy5.jpg?auto=format&fit=max&w=2000"
Received: "https://example.com/myaccount/foo5.jpg?auto=format&fit=max&w=2000"

  199 |
  200 |   it('should load the fifth image eagerly, without scrolling', async () => {
> 201 |     expect(await browser.elementById('eager-loading').getAttribute('src')).toBe(
      |                                                                            ^
  202 |       'https://example.com/myaccount/lazy5.jpg?auto=format&fit=max&w=2000'
  203 |     )
  204 |     expect(

  at Object.<anonymous> (integration/image-component/basic/test/index.test.js:201:76)

ijjk · 2021-07-07T17:29:27Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js fix-19074	Change
buildDuration	12.3s	12.4s	⚠️ +145ms
buildDurationCached	3s	2.9s	-96ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +734 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	atcastle/next.js fix-19074	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.041	2.043	0
/ avg req/sec	1224.95	1223.56	⚠️ -1.39
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.134	1.1	-0.03
/error-in-render avg req/sec	2203.64	2272.84	+69.2

Client Bundles (main, webpack, commons)

	vercel/next.js canary	atcastle/next.js fix-19074	Change
359.HASH.js gzip	3.09 kB	3.09 kB	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	20.6 kB	20.6 kB	✓
webpack-HASH.js gzip	1.49 kB	1.49 kB	✓
Overall change	67.2 kB	67.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	atcastle/next.js fix-19074	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js fix-19074	Change
_app-HASH.js gzip	803 B	803 B	✓
_error-HASH.js gzip	3.18 kB	3.18 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	329 B	329 B	✓
hooks-HASH.js gzip	903 B	903 B	✓
image-HASH.js gzip	5.58 kB	5.61 kB	⚠️ +34 B
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	319 B	319 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	14 kB	14 kB	⚠️ +34 B

Client Build Manifests Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js fix-19074	Change
_buildManifest.js gzip	418 B	419 B	⚠️ +1 B
Overall change	418 B	419 B	⚠️ +1 B

Rendered Page Sizes

	vercel/next.js canary	atcastle/next.js fix-19074	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	524 B	524 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for _buildManifest.js

@@ -13,7 +13,7 @@ self.__BUILD_MANIFEST = {
     "static\u002Fchunks\u002Fpages\u002Fhooks-4e38b05900fed34fcf34.js"
   ],
   "/image": [
-    "static\u002Fchunks\u002Fpages\u002Fimage-5cc691491b3c13e4a3e9.js"
+    "static\u002Fchunks\u002Fpages\u002Fimage-5fb28be4f48700653962.js"
   ],
   "/link": ["static\u002Fchunks\u002Fpages\u002Flink-461e3656f357dad048d3.js"],
   "/routerDirect": [

Diff for image-HASH.js

@@ -121,6 +121,8 @@
 
       var _useIntersection = __webpack_require__(7426);
 
+      var loadedImageURLs = new Set();
+
       if (false) {
       }
 
@@ -331,7 +333,7 @@
       } // See https://stackoverflow.com/q/39777833/266535 for why we use this ref
       // handler instead of the img's onLoad attribute.
 
-      function handleLoading(img, placeholder, onLoadingComplete) {
+      function handleLoading(img, src, placeholder, onLoadingComplete) {
         if (!img) {
           return;
         }
@@ -346,6 +348,8 @@
                 img.style.backgroundImage = "none";
               }
 
+              loadedImageURLs.add(src);
+
               if (onLoadingComplete) {
                 onLoadingComplete();
               }
@@ -459,6 +463,10 @@
           isLazy = false;
         }
 
+        if (src && true && loadedImageURLs.has(src)) {
+          isLazy = false;
+        }
+
         var _ref4 = (0, _useIntersection.useIntersection)({
             rootMargin: "200px",
             disabled: !isLazy
@@ -601,6 +609,7 @@
           });
         }
 
+        var srcString = src;
         return /*#__PURE__*/ _react["default"].createElement(
           "div",
           {
@@ -664,7 +673,7 @@
               className: className,
               ref: function ref(img) {
                 setRef(img);
-                handleLoading(img, placeholder, onLoadingComplete);
+                handleLoading(img, srcString, placeholder, onLoadingComplete);
               },
               style: imgStyle
             })

Webpack 4 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js fix-19074	Change
buildDuration	9.9s	9.8s	-92ms
buildDurationCached	3.8s	3.9s	⚠️ +15ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +734 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	atcastle/next.js fix-19074	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2	2.064	⚠️ +0.06
/ avg req/sec	1250.24	1210.96	⚠️ -39.28
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.138	1.133	0
/error-in-render avg req/sec	2196.62	2206.25	+9.63

Client Bundles (main, webpack, commons)

	vercel/next.js canary	atcastle/next.js fix-19074	Change
14.HASH.js gzip	3.11 kB	3.11 kB	✓
677f882d2ed8..HASH.js gzip	13.9 kB	13.9 kB	✓
framework.HASH.js gzip	41.8 kB	41.8 kB	✓
main-HASH.js gzip	7.81 kB	7.81 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.8 kB	67.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	atcastle/next.js fix-19074	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	atcastle/next.js fix-19074	Change
_app-HASH.js gzip	791 B	791 B	✓
_error-HASH.js gzip	3.83 kB	3.83 kB	✓
amp-HASH.js gzip	531 B	531 B	✓
css-HASH.js gzip	333 B	333 B	✓
hooks-HASH.js gzip	910 B	910 B	✓
index-HASH.js gzip	230 B	230 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	297 B	297 B	✓
withRouter-HASH.js gzip	293 B	293 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	8.98 kB	8.98 kB	✓

Client Build Manifests

	vercel/next.js canary	atcastle/next.js fix-19074	Change
_buildManifest.js gzip	418 B	418 B	✓
Overall change	418 B	418 B	✓

Rendered Page Sizes

	vercel/next.js canary	atcastle/next.js fix-19074	Change
index.html gzip	575 B	575 B	✓
link.html gzip	586 B	586 B	✓
withRouter.html gzip	568 B	568 B	✓
Overall change	1.73 kB	1.73 kB	✓

Commit: c53600a

…l#26968) fixes vercel#19074 This change disables image lazy-loading when both of the following are true: 1) A image is being rendered following a client-side page transition 2) The image has been previously loaded during this session. Before this change, all images with lazy-loading enabled have a visible flicker during client-side page transitions, even though they're already loaded. With this change, there's are two performance risks: 1) There's a chance that some offscreen images will have lazy-loading disabled unnecessarily because they were previously loaded. I think the performance hit here is pretty negligible and the situation is unlikely to come up very often. 2) There's a chance a different-sized version of the image will be selected by the browser, but lazy-loading will be disabled anyway. This seems even more unlikely to me, and anyway the performance hit from a stray un-lazy-loaded image (on a client-side transition) is very minor. In both cases, I think the performance risk is outweighed by the UX improvement of getting rid of the image flicker on page transition.

atcastle added 2 commits July 6, 2021 12:06

Don't lazy-load already-loaded image in CSR

c5beca0

fixes 19074

scope change to client-side only

0b6da95

atcastle requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners July 6, 2021 19:33

ijjk added created-by: Chrome Aurora PRs by the Google Chrome team: https://web.dev/aurora type: next labels Jul 6, 2021

This comment has been minimized.

Sign in to view

ijjk mentioned this pull request Jul 6, 2021

Add page with next/image to stats-app #26973

Merged

10 tasks

styfle previously approved these changes Jul 7, 2021

View reviewed changes

Merge branch 'canary' into fix-19074

92dc966

This comment has been minimized.

Sign in to view

atcastle added 2 commits July 7, 2021 09:46

Fix test in basic suite

065446d

Merge branch 'fix-19074' of github.com:atcastle/next.js into fix-19074

eb681c5

atcastle dismissed styfle’s stale review via eb681c5 July 7, 2021 16:49

This comment has been minimized.

Sign in to view

Also commit the test page component

9d5d787

This comment has been minimized.

Sign in to view

styfle approved these changes Jul 7, 2021

View reviewed changes

Merge branch 'canary' into fix-19074

c53600a

kodiakhq bot merged commit 8066e42 into vercel:canary Jul 7, 2021

atcastle mentioned this pull request Aug 23, 2021

next/image flashes without objectFit property and when full width #20178

Closed

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't lazy-load already-loaded image in client-side transition #26968

Don't lazy-load already-loaded image in client-side transition #26968

atcastle commented Jul 6, 2021

This comment has been minimized.

styfle left a comment

styfle commented Jul 7, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jul 7, 2021

This comment has been minimized.

ijjk commented Jul 7, 2021

Diffs

Don't lazy-load already-loaded image in client-side transition #26968

Don't lazy-load already-loaded image in client-side transition #26968

Conversation

atcastle commented Jul 6, 2021

This comment has been minimized.

styfle left a comment

Choose a reason for hiding this comment

styfle commented Jul 7, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jul 7, 2021

Failing test suites

This comment has been minimized.

ijjk commented Jul 7, 2021

Stats from current PR

Diffs