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.

Sign up for GitHub

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

Jump to bottom

Unlazify images if no intersection observer found #18345

Merged
merged 2 commits into from
Oct 27, 2020
Merged

Unlazify images if no intersection observer found #18345

merged 2 commits into from
Oct 27, 2020

Conversation

atcastle
Copy link
Collaborator

This PR does two things to handle the case of a browser without intersection observer:

  1. If the image component is rendering on the client, it simply disables the lazy flag, so it will fall back to loading eagerly.
  2. If the image was rendered on the server, then as the component hydrates it will detect no observer and automatically unlazify itself (change src to data-src etc). The "unlazification" logic is refactored to support this.

A test is also added to simulate a browser without intersection observer. This test does fail if you remove the "unlazification" logic added in this PR.

@ijjk
Copy link
Member

ijjk commented Oct 27, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
buildDuration 11.5s 12.1s ⚠️ +541ms
nodeModulesSize 91.3 MB 91.3 MB ⚠️ +811 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.229 2.273 ⚠️ +0.04
/ avg req/sec 1121.82 1099.7 ⚠️ -22.12
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.192 1.228 ⚠️ +0.04
/error-in-render avg req/sec 2097.81 2036.5 ⚠️ -61.31
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..3aa9.js gzip 11.3 kB 11.3 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-fc2ed98..cd2b.js gzip 7.33 kB 7.33 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.3 kB 58.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..dule.js gzip 7.03 kB 7.03 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-20f9619..dule.js gzip 6.31 kB 6.31 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.1 kB 53.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check 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-41ceea0..6b62.js gzip 1.35 kB 1.35 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.75 kB 7.75 kB ✓
Client Pages Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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-2d41a89..dule.js gzip 1.31 kB 1.31 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.4 kB 5.4 kB ✓
Client Build Manifests
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_buildManifest.js gzip 323 B 323 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 652 B 652 B ✓
Rendered Page Sizes
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check Change
buildDuration 13.5s 13.7s ⚠️ +140ms
nodeModulesSize 91.3 MB 91.3 MB ⚠️ +811 B
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..3aa9.js gzip 11.3 kB 11.3 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-fc2ed98..cd2b.js gzip 7.33 kB 7.33 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.3 kB 58.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..dule.js gzip 7.03 kB 7.03 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-20f9619..dule.js gzip 6.31 kB 6.31 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.1 kB 53.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check 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-41ceea0..6b62.js gzip 1.35 kB 1.35 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.75 kB 7.75 kB ✓
Client Pages Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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-2d41a89..dule.js gzip 1.31 kB 1.31 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.4 kB 5.4 kB ✓
Client Build Manifests
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_buildManifest.js gzip 323 B 323 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 652 B 652 B ✓
Serverless bundles
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_error.js 1.06 MB 1.06 MB ✓
404.html 4.73 kB 4.73 kB ✓
hooks.html 3.92 kB 3.92 kB ✓
index.js 1.06 MB 1.06 MB ✓
link.js 1.11 MB 1.11 MB ✓
routerDirect.js 1.1 MB 1.1 MB ✓
withRouter.js 1.1 MB 1.1 MB ✓
Overall change 5.43 MB 5.43 MB ✓
Commit: bec4c8b

styfle
styfle reviewed Oct 27, 2020
View reviewed changes
packages/next/client/image.tsx
@@ -234,6 +237,11 @@ export default function Image({
lazy = true
}

if (typeof window !== 'undefined' && !window.IntersectionObserver) {
Copy link
Member

@styfle styfle Oct 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You're doing this check twice, can you do it once and pass in the variable to the getObserver() function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's actually a little trickier than you would think because of the weird test for missing intersection observer. Re-accessing the window.IntersectionObserver at the time of getObserver allows the test to delete that property in time. If I cache it at the root level of the script, it doesn't work.

@ijjk
Copy link
Member

ijjk commented Oct 27, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
buildDuration 12.3s 12.3s -20ms
nodeModulesSize 91.3 MB 91.3 MB ⚠️ +817 B
Page Load Tests Overall increase ✓
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.314 2.324 ⚠️ +0.01
/ avg req/sec 1080.25 1075.76 ⚠️ -4.49
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.226 1.222 0
/error-in-render avg req/sec 2039.76 2045.79 +6.03
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..3aa9.js gzip 11.3 kB 11.3 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-fc2ed98..cd2b.js gzip 7.33 kB 7.33 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.3 kB 58.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..dule.js gzip 7.03 kB 7.03 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-20f9619..dule.js gzip 6.31 kB 6.31 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.1 kB 53.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check 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-41ceea0..6b62.js gzip 1.35 kB 1.35 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.75 kB 7.75 kB ✓
Client Pages Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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-2d41a89..dule.js gzip 1.31 kB 1.31 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.4 kB 5.4 kB ✓
Client Build Manifests
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_buildManifest.js gzip 323 B 323 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 652 B 652 B ✓
Rendered Page Sizes
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check Change
buildDuration 14.1s 14.2s ⚠️ +61ms
nodeModulesSize 91.3 MB 91.3 MB ⚠️ +817 B
Client Bundles (main, webpack, commons)
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..3aa9.js gzip 11.3 kB 11.3 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-fc2ed98..cd2b.js gzip 7.33 kB 7.33 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.3 kB 58.3 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
677f882d2ed8..dule.js gzip 7.03 kB 7.03 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-20f9619..dule.js gzip 6.31 kB 6.31 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.1 kB 53.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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 add-intersection-observer-check 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-41ceea0..6b62.js gzip 1.35 kB 1.35 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.75 kB 7.75 kB ✓
Client Pages Modern
vercel/next.js canary azukaru/next.js add-intersection-observer-check 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-2d41a89..dule.js gzip 1.31 kB 1.31 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.4 kB 5.4 kB ✓
Client Build Manifests
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_buildManifest.js gzip 323 B 323 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 652 B 652 B ✓
Serverless bundles
vercel/next.js canary azukaru/next.js add-intersection-observer-check Change
_error.js 1.06 MB 1.06 MB ✓
404.html 4.73 kB 4.73 kB ✓
hooks.html 3.92 kB 3.92 kB ✓
index.js 1.06 MB 1.06 MB ✓
link.js 1.11 MB 1.11 MB ✓
routerDirect.js 1.1 MB 1.1 MB ✓
withRouter.js 1.1 MB 1.1 MB ✓
Overall change 5.43 MB 5.43 MB ✓
Commit: 8b58a56

@timneutkens timneutkens merged commit 5d7f7a4 into vercel:canary Oct 27, 2020
@timneutkens timneutkens deleted the add-intersection-observer-check branch October 27, 2020 23:41
@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.
Reviewers

@timneutkens timneutkens timneutkens approved these changes

@styfle styfle styfle approved these changes

@ijjk ijjk Awaiting requested review from ijjk

@lfades lfades Awaiting requested review from lfades

@Timer Timer Awaiting requested review from Timer

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@atcastle @ijjk @styfle @timneutkens