-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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 commons chunk config #34445
remove commons chunk config #34445
Conversation
This comment has been minimized.
This comment has been minimized.
Looks fine to me. The commons chunk was mainly a semantic affordance anyway, to make it easy to quickly identify the "all pages" dependencies. I had thought that the next/dynamically-imported module counted as it's own entry point, and therefor wouldn't get grouped with the page that imports it in the dependency graph. But if that's wrong and it's accidentally picking up modules from next/dynamic, then I see no issue removing it. The only impact will be that the chunk that used to be called "commons" will have a dynamically generated name instead. |
i observed that in my case, @material-ui is placed in commons, while we only use it on very few pages (through import or dynamic import), so it should not be bundled there. I now stumbled over this PR here and i wonder if this removes the common chunk alltogether and i should just wait for it to land? |
Stats from current PRDefault Build (Decrease detected ✓)General Overall decrease ✓
Page Load Tests Overall decrease
|
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.597 | 3.746 | |
/ avg req/sec | 695 | 667.43 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.848 | 1.915 | |
/error-in-render avg req/sec | 1352.63 | 1305.54 |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 27.9 kB | 27.9 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.6 kB | 71.6 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.57 kB | 2.57 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 5.05 kB | 5.05 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.26 kB | 2.26 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.7 kB | 14.7 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
_buildManifest.js gzip | 460 B | 460 B | ✓ |
Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
index.html gzip | 532 B | 532 B | ✓ |
link.html gzip | 545 B | 545 B | ✓ |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Increase detected ⚠️ )
General Overall decrease ✓
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
buildDuration | 21.8s | 21.8s | |
buildDurationCached | 6.8s | 6.9s | |
nodeModulesSize | 367 MB | 367 MB | -241 B |
Page Load Tests Overall increase ✓
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.717 | 3.641 | -0.08 |
/ avg req/sec | 672.61 | 686.55 | +13.94 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 2.025 | 1.869 | -0.16 |
/error-in-render avg req/sec | 1234.69 | 1337.93 | +103.24 |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.1 kB | 42.1 kB | ✓ |
main-HASH.js gzip | 27.9 kB | 27.9 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.7 kB | 71.7 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.56 kB | 2.56 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 911 B | 911 B | ✓ |
image-HASH.js gzip | 5.08 kB | 5.08 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.28 kB | 2.28 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.7 kB | 14.7 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js bugfix/commons-chunk | Change | |
---|---|---|---|
index.html gzip | 534 B | 534 B | ✓ |
link.html gzip | 547 B | 547 B | ✓ |
withRouter.html gzip | 528 B | 528 B | ✓ |
Overall change | 1.61 kB | 1.61 kB | ✓ |
This removes the config for the
commons
chunk.I think the idea was that modules are that in all pages are put into a
commons
chunk, but that breaks when next/dynamic comes into play, which also creates chunks. So thetotalPages
condition is broken and could lead to too many modules placed into the commons chunk.Example: 2 pages, each has one next/dynamic. Both on demand chunks include module A. page 1 includes module B and next/dynamic on page 2 includes module B. A and B are placed into commons. commonjs chunk is loaded in page 1 and both next/dynamic. Page 1 would load module A even while it doesn't need it.
Bug
fixes #number
contributing.md
Feature
fixes #number
contributing.md
Documentation / Examples
yarn lint