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

Image component lazy loading #17916

Merged
merged 24 commits into from Oct 17, 2020
Merged

Conversation

atcastle
Copy link
Collaborator

This PR adds intersection observer-based lazy loading to the image component. Any image with the 'lazy' attribute will use this functionality.

Before merging: Some clarification is needed on how this will interact with AMP mode, since it renders the lazy images without a src, then updates them using JavaScript. I'd like to get some more insight on that so I can update this PR if needed ASAP.

@ijjk
Copy link
Member

ijjk commented Oct 15, 2020

Failing test suites

Commit: 24ec9ee

test/integration/app-document/test/index.test.js

  • Document and App > Rendering via HTTP > _document > It adds nonces to all scripts and preload links
  • Document and App > Rendering via HTTP > _document > It adds crossOrigin to all scripts and preload links
Expand output

● Document and App › Rendering via HTTP › _document › It adds nonces to all scripts and preload links

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

Expected: true
Received: false

  40 |           if ($(element).attr('nonce') !== nonce) noncesAdded = false
  41 |         })
> 42 |         expect(noncesAdded).toBe(true)
     |                             ^
  43 |       })
  44 | 
  45 |       test('It adds crossOrigin to all scripts and preload links', async () => {

  at Object.<anonymous> (integration/app-document/test/rendering.js:42:29)

● Document and App › Rendering via HTTP › _document › It adds crossOrigin to all scripts and preload links

expect(received).toBeTruthy()

Received: false

  47 |         const crossOrigin = 'anonymous'
  48 |         $('script, link[rel=preload]').each((index, element) => {
> 49 |           expect($(element).attr('crossorigin') === crossOrigin).toBeTruthy()
     |                                                                  ^
  50 |         })
  51 |       })
  52 | 

  at Object.<anonymous> (integration/app-document/test/rendering.js:49:66)
  at initialize.each (../node_modules/cheerio/lib/api/traversing.js:300:24)
  at Object.<anonymous> (integration/app-document/test/rendering.js:48:40)

test/integration/build-output/test/index.test.js

  • Build Output > Basic Application Output > should not deviate from snapshot
Expand output

● Build Output › Basic Application Output › should not deviate from snapshot

expect(received).toBeLessThanOrEqual(expected)

Expected: <= 0
Received:    0.20000000000000284

   96 | 
   97 |       // should be no bigger than 60.8 kb
>  98 |       expect(parseFloat(indexFirstLoad) - 61).toBeLessThanOrEqual(0)
      |                                               ^
   99 |       expect(indexFirstLoad.endsWith('kB')).toBe(true)
  100 | 
  101 |       expect(parseFloat(err404Size) - 3.5).toBeLessThanOrEqual(0)

  at Object.<anonymous> (integration/build-output/test/index.test.js:98:47)

test/integration/size-limit/test/index.test.js

  • Production response size > should not increase the overall response size of default build
  • Production response size > should not increase the overall response size of modern build
Expand output

● Production response size › should not increase the overall response size of default build

expect(received).toBeLessThanOrEqual(expected)

Expected: <= 1024
Received:    2001

  82 |     // These numbers are without gzip compression!
  83 |     const delta = responseSizesBytes - 280 * 1024
> 84 |     expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb
     |                   ^
  85 |     expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target
  86 |   })
  87 | 

  at Object.<anonymous> (integration/size-limit/test/index.test.js:84:19)

● Production response size › should not increase the overall response size of modern build

expect(received).toBeLessThanOrEqual(expected)

Expected: <= 1024
Received:    1506

  102 |     // These numbers are without gzip compression!
  103 |     const delta = responseSizesBytes - 171 * 1024
> 104 |     expect(delta).toBeLessThanOrEqual(1024) // don't increase size more than 1kb
      |                   ^
  105 |     expect(delta).toBeGreaterThanOrEqual(-1024) // don't decrease size more than 1kb without updating target
  106 |   })
  107 | })

  at Object.<anonymous> (integration/size-limit/test/index.test.js:104:19)

Copy link
Contributor

@prateekbh prateekbh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the following block

if("IntersectionObserver"in window)

should there be an else where we just go and modify all .data-src to src so if the browser does not support IntersectionObserver the user would still see the images.

@ijjk
Copy link
Member

ijjk commented Oct 16, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js image-component Change
buildDuration 12s 12.7s ⚠️ +640ms
nodeModulesSize 64.3 MB 64.4 MB ⚠️ +6.48 kB
Page Load Tests Overall increase ✓
vercel/next.js canary azukaru/next.js image-component Change
/ failed reqs 0 0
/ total time (seconds) 2.419 2.327 -0.09
/ avg req/sec 1033.54 1074.14 +40.6
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.457 1.46 0
/error-in-render avg req/sec 1715.8 1712.17 ⚠️ -3.63
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..7765.js gzip 11.1 kB 11.1 kB
framework.HASH.js gzip 39 kB 39 kB
main-43e96dd..7c44.js gzip 7.3 kB 7.3 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.1 kB 58.1 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..dule.js gzip 6.94 kB 6.94 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-ee66c17..dule.js gzip 6.29 kB 6.29 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js image-component Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary azukaru/next.js image-component Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-89ad9e7..25bb.js gzip 1.34 kB 1.34 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.74 kB 7.74 kB
Client Pages Modern
vercel/next.js canary azukaru/next.js image-component Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-aeb707b..dule.js gzip 1.29 kB 1.29 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.39 kB 5.39 kB
Client Build Manifests
vercel/next.js canary azukaru/next.js image-component Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Rendered Page Sizes
vercel/next.js canary azukaru/next.js image-component Change
index.html gzip 1 kB 1 kB
link.html gzip 1.01 kB 1.01 kB
withRouter.html gzip 995 B 995 B
Overall change 3.01 kB 3.01 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js image-component Change
buildDuration 14.1s 13.7s -324ms
nodeModulesSize 64.3 MB 64.4 MB ⚠️ +6.48 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..7765.js gzip 11.1 kB 11.1 kB
framework.HASH.js gzip 39 kB 39 kB
main-43e96dd..7c44.js gzip 7.3 kB 7.3 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.1 kB 58.1 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..dule.js gzip 6.94 kB 6.94 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-ee66c17..dule.js gzip 6.29 kB 6.29 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js image-component Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary azukaru/next.js image-component Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-89ad9e7..25bb.js gzip 1.34 kB 1.34 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.74 kB 7.74 kB
Client Pages Modern
vercel/next.js canary azukaru/next.js image-component Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-aeb707b..dule.js gzip 1.29 kB 1.29 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.39 kB 5.39 kB
Client Build Manifests
vercel/next.js canary azukaru/next.js image-component Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Serverless bundles
vercel/next.js canary azukaru/next.js image-component Change
_error.js 1.06 MB 1.06 MB
404.html 4.34 kB 4.34 kB
hooks.html 3.92 kB 3.92 kB
index.js 1.06 MB 1.06 MB
link.js 1.1 MB 1.1 MB
routerDirect.js 1.1 MB 1.1 MB
withRouter.js 1.1 MB 1.1 MB
Overall change 5.42 MB 5.42 MB
Commit: 9c2e235

timneutkens
timneutkens previously approved these changes Oct 17, 2020
@timneutkens
Copy link
Member

should there be an else where we just go and modify all .data-src to src so if the browser does not support IntersectionObserver the user would still see the images.

Yes, that can be added in a follow-up PR though 👍

…-component

# Conflicts:
#	packages/next/client/image.tsx
@ijjk
Copy link
Member

ijjk commented Oct 17, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js image-component Change
buildDuration 13.4s 13.3s -109ms
nodeModulesSize 64.4 MB 64.4 MB ⚠️ +6.44 kB
Page Load Tests Overall increase ✓
vercel/next.js canary azukaru/next.js image-component Change
/ failed reqs 0 0
/ total time (seconds) 2.51 2.509 0
/ avg req/sec 996.12 996.24 +0.12
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.559 1.551 -0.01
/error-in-render avg req/sec 1603.74 1611.39 +7.65
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..7765.js gzip 11.1 kB 11.1 kB
framework.HASH.js gzip 39 kB 39 kB
main-5aa1543..4e79.js gzip 7.34 kB 7.34 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.1 kB 58.1 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..dule.js gzip 6.94 kB 6.94 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-7d849dd..dule.js gzip 6.32 kB 6.32 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js image-component Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary azukaru/next.js image-component Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-89ad9e7..25bb.js gzip 1.34 kB 1.34 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.74 kB 7.74 kB
Client Pages Modern
vercel/next.js canary azukaru/next.js image-component Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-aeb707b..dule.js gzip 1.29 kB 1.29 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.39 kB 5.39 kB
Client Build Manifests
vercel/next.js canary azukaru/next.js image-component Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Rendered Page Sizes
vercel/next.js canary azukaru/next.js image-component Change
index.html gzip 1 kB 1 kB
link.html gzip 1.01 kB 1.01 kB
withRouter.html gzip 996 B 996 B
Overall change 3.01 kB 3.01 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js image-component Change
buildDuration 15.2s 15.2s ⚠️ +40ms
nodeModulesSize 64.4 MB 64.4 MB ⚠️ +6.44 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..7765.js gzip 11.1 kB 11.1 kB
framework.HASH.js gzip 39 kB 39 kB
main-5aa1543..4e79.js gzip 7.34 kB 7.34 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.1 kB 58.1 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js image-component Change
677f882d2ed8..dule.js gzip 6.94 kB 6.94 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-7d849dd..dule.js gzip 6.32 kB 6.32 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 53 kB 53 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js image-component Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary azukaru/next.js image-component Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-89ad9e7..25bb.js gzip 1.34 kB 1.34 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.74 kB 7.74 kB
Client Pages Modern
vercel/next.js canary azukaru/next.js image-component Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-aeb707b..dule.js gzip 1.29 kB 1.29 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.39 kB 5.39 kB
Client Build Manifests
vercel/next.js canary azukaru/next.js image-component Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Serverless bundles
vercel/next.js canary azukaru/next.js image-component Change
_error.js 1.06 MB 1.06 MB
404.html 4.34 kB 4.34 kB
hooks.html 3.92 kB 3.92 kB
index.js 1.06 MB 1.06 MB
link.js 1.1 MB 1.1 MB
routerDirect.js 1.1 MB 1.1 MB
withRouter.js 1.1 MB 1.1 MB
Overall change 5.42 MB 5.42 MB
Commit: 946ef4b

@timneutkens timneutkens merged commit c9eb3dc into vercel:canary Oct 17, 2020
@timneutkens timneutkens deleted the image-component branch October 17, 2020 18:55
@timneutkens
Copy link
Member

Before merging: Some clarification is needed on how this will interact with AMP mode, since it renders the lazy images without a src, then updates them using JavaScript. I'd like to get some more insight on that so I can update this PR if needed ASAP.

AMP doesn't support <img> so we'd have to render an amp-img always in case of AMP mode.

kodiakhq bot pushed a commit that referenced this pull request Oct 18, 2020
Follow-up #17916

Correcting type errors and organizing redundant descriptions.
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants