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

Remove preload for priority images #19118

Merged
merged 4 commits into from
Nov 12, 2020
Merged

Remove preload for priority images #19118

merged 4 commits into from
Nov 12, 2020

Conversation

Timer
Copy link
Member

@Timer Timer commented Nov 12, 2020 •
edited
Loading

Fixes #18720

This removes image preloading. It doesn't work correctly on any browser other than Chrome (with Chrome's real engine). On all other browsers, it triggers 2x the bytes to be downloaded. The tradeoff isn't worth it here IMO.

Chrome itself should be smart enough to bump an <img /> tag's priority over other preloads that are script type during the preparse phase.

We can reintroduce this when we don't hurt non-Chrome users.

@vercel vercel bot temporarily deployed to Preview November 12, 2020 19:03 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 12, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
buildDuration 12.4s 12.9s ⚠️ +457ms
nodeModulesSize 84.9 MB 84.9 MB -3.42 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.406 2.371 -0.04
/ avg req/sec 1039.19 1054.39 +15.2
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.262 1.305 ⚠️ +0.04
/error-in-render avg req/sec 1980.24 1916.25 ⚠️ -63.99
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..d9fb.js gzip 12.7 kB 12.7 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-43dd9e5..8439.js gzip 6.48 kB 6.48 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.9 kB 58.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..dule.js gzip 8.23 kB 8.23 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-472f7d5..dule.js gzip 5.58 kB 5.58 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.5 kB 53.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB ✓
_error-6f635..c393.js gzip 3.44 kB 3.44 kB ✓
hooks-d4ffc3..9e0f.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d9a15a0..130a.js gzip 1.57 kB 1.57 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.98 kB 7.98 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-f2fcc..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-973cce3..dule.js gzip 1.51 kB 1.51 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.6 kB 5.6 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 330 B 330 B ✓
Overall change 651 B 651 B ✓
Rendered Page Sizes
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
index.html gzip 972 B 972 B ✓
link.html gzip 977 B 977 B ✓
withRouter.html gzip 964 B 964 B ✓
Overall change 2.91 kB 2.91 kB ✓
Serverless Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
buildDuration 14.1s 14.5s ⚠️ +434ms
nodeModulesSize 84.9 MB 84.9 MB -3.42 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..d9fb.js gzip 12.7 kB 12.7 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-43dd9e5..8439.js gzip 6.48 kB 6.48 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.9 kB 58.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..dule.js gzip 8.23 kB 8.23 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-472f7d5..dule.js gzip 5.58 kB 5.58 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.5 kB 53.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB ✓
_error-6f635..c393.js gzip 3.44 kB 3.44 kB ✓
hooks-d4ffc3..9e0f.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d9a15a0..130a.js gzip 1.57 kB 1.57 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.98 kB 7.98 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-f2fcc..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-973cce3..dule.js gzip 1.51 kB 1.51 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.6 kB 5.6 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 330 B 330 B ✓
Overall change 651 B 651 B ✓
Serverless bundles
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_error.js 916 kB 916 kB ✓
404.html 4.61 kB 4.61 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 916 kB 916 kB ✓
link.js 974 kB 974 kB ✓
routerDirect.js 967 kB 967 kB ✓
withRouter.js 967 kB 967 kB ✓
Overall change 4.75 MB 4.75 MB ✓
Commit: ee88768

@ijjk

This comment has been minimized.

Sign in to view
ijjk
ijjk previously approved these changes Nov 12, 2020
View reviewed changes
@vercel vercel bot temporarily deployed to Preview November 12, 2020 19:18 Inactive
@Timer Timer requested a review from ijjk November 12, 2020 19:20
@ijjk
Copy link
Member

ijjk commented Nov 12, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
buildDuration 13.8s 13.1s -656ms
nodeModulesSize 84.9 MB 84.9 MB -3.42 kB
Page Load Tests Overall increase ✓
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.485 2.394 -0.09
/ avg req/sec 1006.04 1044.24 +38.2
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.323 1.314 -0.01
/error-in-render avg req/sec 1890.35 1902.47 +12.12
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..d9fb.js gzip 12.7 kB 12.7 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-43dd9e5..8439.js gzip 6.48 kB 6.48 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.9 kB 58.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..dule.js gzip 8.23 kB 8.23 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-472f7d5..dule.js gzip 5.58 kB 5.58 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.5 kB 53.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB ✓
_error-6f635..c393.js gzip 3.44 kB 3.44 kB ✓
hooks-d4ffc3..9e0f.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d9a15a0..130a.js gzip 1.57 kB 1.57 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.98 kB 7.98 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-f2fcc..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-973cce3..dule.js gzip 1.51 kB 1.51 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.6 kB 5.6 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 330 B 330 B ✓
Overall change 651 B 651 B ✓
Rendered Page Sizes
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
index.html gzip 972 B 972 B ✓
link.html gzip 977 B 977 B ✓
withRouter.html gzip 964 B 964 B ✓
Overall change 2.91 kB 2.91 kB ✓
Serverless Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
buildDuration 15.3s 15.1s -191ms
nodeModulesSize 84.9 MB 84.9 MB -3.42 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..d9fb.js gzip 12.7 kB 12.7 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-43dd9e5..8439.js gzip 6.48 kB 6.48 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 58.9 kB 58.9 kB ✓
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
677f882d2ed8..dule.js gzip 8.23 kB 8.23 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-472f7d5..dule.js gzip 5.58 kB 5.58 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 53.5 kB 53.5 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-3b0cf13..85f8.js gzip 1.28 kB 1.28 kB ✓
_error-6f635..c393.js gzip 3.44 kB 3.44 kB ✓
hooks-d4ffc3..9e0f.js gzip 887 B 887 B ✓
index-17468f..5d83.js gzip 227 B 227 B ✓
link-d9a15a0..130a.js gzip 1.57 kB 1.57 kB ✓
routerDirect..924c.js gzip 284 B 284 B ✓
withRouter-7..c13d.js gzip 284 B 284 B ✓
Overall change 7.98 kB 7.98 kB ✓
Client Pages Modern
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_app-75d3a82..dule.js gzip 625 B 625 B ✓
_error-f2fcc..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-973cce3..dule.js gzip 1.51 kB 1.51 kB ✓
routerDirect..dule.js gzip 284 B 284 B ✓
withRouter-f..dule.js gzip 282 B 282 B ✓
Overall change 5.6 kB 5.6 kB ✓
Client Build Manifests
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_buildManifest.js gzip 321 B 321 B ✓
_buildManife..dule.js gzip 330 B 330 B ✓
Overall change 651 B 651 B ✓
Serverless bundles
vercel/next.js canary Timer/next.js hotfix/drop-preload Change
_error.js 916 kB 916 kB ✓
404.html 4.61 kB 4.61 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 916 kB 916 kB ✓
link.js 974 kB 974 kB ✓
routerDirect.js 967 kB 967 kB ✓
withRouter.js 967 kB 967 kB ✓
Overall change 4.75 MB 4.75 MB ✓
Commit: b5ccc72

@Timer Timer mentioned this pull request Nov 12, 2020
Closed
@kodiakhq kodiakhq bot merged commit 04ab352 into vercel:canary Nov 12, 2020
@Timer Timer deleted the hotfix/drop-preload branch November 12, 2020 19:46
This was referenced Dec 25, 2020
Merged
Closed
kodiakhq bot pushed a commit that referenced this pull request Dec 29, 2020
@danielyefet
Remove "priority" prop from Image docs (#20471)
69ff649 
I don't think the docs should still mention the `priority` prop because the feature no longer exists since this was merged -  #19118

It certainly caught me out! 😬 #20438
@Timer Timer mentioned this pull request Dec 30, 2020
Merged
kodiakhq bot pushed a commit that referenced this pull request Dec 30, 2020
@Timer
feat(next/image): preload priority images (#20615)
45b87aa 
The HTML Living Standard explicitly says `href` should be omitted to prevent the loading of an incorrectly sized image:
https://html.spec.whatwg.org/multipage/semantics.html#attr-link-imagesrcset

![image](https://user-images.githubusercontent.com/616428/103378205-8a013800-4aaf-11eb-9085-10f547263fed.png)

Since it's in the spec, I assume this is valid-enough HTML.

This also dedupes preloads which the old implementation did not.

---

Fixes #18756
x-ref #19118
Fixes #18720
@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

@ijjk ijjk ijjk approved these changes

@styfle styfle styfle approved these changes

@lfades lfades Awaiting requested review from lfades

@timneutkens timneutkens Awaiting requested review from timneutkens

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

Successfully merging this pull request may close these issues.

next/image priority preloads the largest size
3 participants
@Timer @ijjk @styfle