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

Remove old modern mode experiment #19275

Merged
merged 8 commits into from
Nov 18, 2020
Merged

Conversation

Timer
Copy link
Member

@Timer Timer commented Nov 18, 2020

This PR removes the modern mode experiment because:

  • It does not yield meaningful bundle size wins when compared to other initiatives we've taken
  • It's not compatible with webpack 5 (which we're upgrading to)
  • It's currently broken and causes most apps to malfunction
  • There's no champion currently owning the experiment

We can re-introduce this in the future when we'd like to make it a default for all Next.js apps.

Note: Next.js still supports Differential Loading (nomodule) and does it by default. This PR strictly removes the experimental modern syntax, and does not disable our existing modern/legacy polyfilling.


Fixes #19200
Fixes #18960
Fixes #14707
Fixes #14465

@vercel vercel bot temporarily deployed to Preview November 18, 2020 16:36 Inactive
@vercel vercel bot temporarily deployed to Preview November 18, 2020 16:53 Inactive
@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@vercel vercel bot temporarily deployed to Preview November 18, 2020 17:08 Inactive
@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@vercel vercel bot temporarily deployed to Preview November 18, 2020 17:30 Inactive
@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@vercel vercel bot temporarily deployed to Preview November 18, 2020 17:53 Inactive
@ijjk
Copy link
Member

ijjk commented Nov 18, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary Timer/next.js remove/modern-mode Change
buildDuration 10.2s 10.1s -92ms
nodeModulesSize 85 MB 84.9 MB -52.8 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary Timer/next.js remove/modern-mode Change
/ failed reqs 0 0
/ total time (seconds) 2.216 2.231 ⚠️ +0.01
/ avg req/sec 1128.39 1120.6 ⚠️ -7.79
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.229 1.304 ⚠️ +0.07
/error-in-render avg req/sec 2034.46 1916.68 ⚠️ -117.78
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js remove/modern-mode Change
677f882d2ed8..4972.js gzip 12.7 kB 12.7 kB
framework.HASH.js gzip 39 kB 39 kB
main-f1a49fb..e45e.js gzip 6.52 kB 6.52 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.9 kB 58.9 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js remove/modern-mode 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 remove/modern-mode 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-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary Timer/next.js remove/modern-mode Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Rendered Page Sizes
vercel/next.js canary Timer/next.js remove/modern-mode Change
index.html gzip 613 B 613 B
link.html gzip 621 B 621 B
withRouter.html gzip 608 B 608 B
Overall change 1.84 kB 1.84 kB

Serverless Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary Timer/next.js remove/modern-mode Change
buildDuration 12.1s 11.7s -372ms
nodeModulesSize 85 MB 84.9 MB -52.8 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary Timer/next.js remove/modern-mode Change
677f882d2ed8..4972.js gzip 12.7 kB 12.7 kB
framework.HASH.js gzip 39 kB 39 kB
main-f1a49fb..e45e.js gzip 6.52 kB 6.52 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 58.9 kB 58.9 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary Timer/next.js remove/modern-mode 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 remove/modern-mode 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-b618194..5477.js gzip 1.61 kB 1.61 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 8.01 kB 8.01 kB
Client Build Manifests
vercel/next.js canary Timer/next.js remove/modern-mode Change
_buildManifest.js gzip 321 B 321 B
Overall change 321 B 321 B
Serverless bundles Overall decrease ✓
vercel/next.js canary Timer/next.js remove/modern-mode Change
_error.js 915 kB 914 kB -687 B
404.html 2.67 kB 2.67 kB
hooks.html 1.92 kB 1.92 kB
index.js 915 kB 915 kB -687 B
link.js 974 kB 973 kB -687 B
routerDirect.js 966 kB 966 kB -687 B
withRouter.js 966 kB 966 kB -687 B
Overall change 4.74 MB 4.74 MB -3.44 kB
Commit: 91289dd

@Timer Timer marked this pull request as ready for review November 18, 2020 18:19
@kodiakhq kodiakhq bot merged commit 30c2dfd into vercel:canary Nov 18, 2020
@Timer Timer deleted the remove/modern-mode branch November 18, 2020 18:31
@franklinjavier
Copy link

Awesome

@PepijnSenders
Copy link
Contributor

It does not yield meaningful bundle size wins when compared to other initiatives we've taken

When we implemented this we saw a 3%-6% improvement in bundle size, which other initiatives can we use to achieve the same @Timer ?

kamermans pushed a commit to kamermans/next.js that referenced this pull request Dec 14, 2020
This PR removes the modern mode experiment because:

- It does not yield meaningful bundle size wins when compared to other initiatives we've taken
- It's not compatible with webpack 5 (which we're upgrading to)
- It's currently broken and causes most apps to malfunction
- There's no champion currently owning the experiment

We can re-introduce this in the future when we'd like to make it a default for all Next.js apps.

Note: **Next.js still supports Differential Loading (`nomodule`) and does it by default.** This PR strictly removes the experimental modern _syntax_, and does not disable our existing modern/legacy polyfilling.

---

Fixes vercel#19200
Fixes vercel#18960
Fixes vercel#14707
Fixes vercel#14465
@developit
Copy link
Contributor

Would be nice to see a replacement for this given the effort involved in removing it (rather than updating it). Modern mode shaves 21% off the initial page weight for Next.js apps using Preact, independently of the savings attributable to the nomodule polyfill chunk.

DoctorDerek added a commit to DoctorDerek/tailwind-nextjs-starter-blog that referenced this pull request Mar 31, 2021
This feature got dropped by Next.js so should be removed, see: vercel/next.js#19275
@DoctorDerek
Copy link

Would be nice to see a replacement for this given the effort involved in removing it (rather than updating it). Modern mode shaves 21% off the initial page weight for Next.js apps using Preact, independently of the savings attributable to the nomodule polyfill chunk.

Yeah I love this concept and think it should be a core feature, but it seems like the incompatibility with the upgrade to Webpack 5 is a dealbreaker. Any thoughts on how we would reimplement this feature in a compatible way @developit?

ka2n added a commit to ka2n/chakra-ui that referenced this pull request Oct 19, 2021
`experimental.modern: true` is deprecated and removed.

vercel/next.js#19275
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants