-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
Prefer module over main on main fields for app router server compiler #56532
Conversation
Tests Passed |
Stats from current PRDefault BuildGeneral Overall increase
|
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
buildDuration | 10.5s | 10.3s | N/A |
buildDurationCached | 6.3s | 6.4s | N/A |
nodeModulesSize | 172 MB | 172 MB | |
nextStartRea..uration (ms) | 895ms | 896ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
199-HASH.js gzip | 27.5 kB | 27.5 kB | N/A |
3f784ff6-HASH.js gzip | 50.9 kB | 50.9 kB | ✓ |
99.HASH.js gzip | 182 B | 182 B | ✓ |
framework-HASH.js gzip | 45.3 kB | 45.3 kB | ✓ |
main-app-HASH.js gzip | 254 B | 251 B | N/A |
main-HASH.js gzip | 32.9 kB | 32.9 kB | N/A |
webpack-HASH.js gzip | 1.75 kB | 1.75 kB | N/A |
Overall change | 96.4 kB | 96.4 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
_app-HASH.js gzip | 206 B | 205 B | N/A |
_error-HASH.js gzip | 182 B | 180 B | N/A |
amp-HASH.js gzip | 506 B | 505 B | N/A |
css-HASH.js gzip | 322 B | 323 B | N/A |
dynamic-HASH.js gzip | 2.57 kB | 2.57 kB | N/A |
edge-ssr-HASH.js gzip | 260 B | 259 B | N/A |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 369 B | 369 B | ✓ |
image-HASH.js gzip | 4.35 kB | 4.35 kB | N/A |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.64 kB | 2.63 kB | N/A |
routerDirect..HASH.js gzip | 312 B | 311 B | N/A |
script-HASH.js gzip | 385 B | 384 B | N/A |
withRouter-HASH.js gzip | 307 B | 308 B | N/A |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 1.08 kB | 1.08 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
_buildManifest.js gzip | 485 B | 482 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
index.html gzip | 528 B | 530 B | N/A |
link.html gzip | 541 B | 543 B | N/A |
withRouter.html gzip | 523 B | 525 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
edge-ssr.js gzip | 93.7 kB | 93.7 kB | N/A |
page.js gzip | 152 kB | 152 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js app-router/main-fields | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 629 B | 623 B | N/A |
middleware-r..fest.js gzip | 150 B | 151 B | N/A |
middleware.js gzip | 22.9 kB | 22.9 kB | N/A |
edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
Overall change | 1.92 kB | 1.92 kB | ✓ |
17df1b3
to
85d0547
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we do that for the server code as well?
@feedthejim Could you elaborate which part of server code? This change is for user land server target code for both RSC and SSR |
Clarified offline, what I meant was Pages SSR, but we're not bundling those so this would be a breaking change |
…#56960) Reverts #56766 Reland #56532 x-ref: https://vercel.slack.com/archives/C04DUD7EB1B/p1697576563771229?thread_ts=1697146531.305779&cid=C04DUD7EB1B This change was breaking package which was actually a dual package but marked as esm module, and when the package is ESM but import a CJS dependency without fully spepcified file path it will fail to resolve. The way is to either remove the esmodule type in package.json or change the import to full path import with extension for the package
This change is to pick the esm assets for RSC server layer and server rendering side, this is for production and development perf purpose, also to let them apply with the ESM related optimization such as tree-shaking, barrel files optimizations, etc.
We found a few packages that can't be optimized easily in bundling because we're using "main" field so the packages are not able to be tree-shaked, ending up with large bundle in the dist. This will change a lot for the bundling improvements as some packages are only having "main" and "module" field. So switching from CJS to ESM means better bundling, more optimization, and faster code.
#56501 was a precondition for this, as previously the bundling strategy was applied to some library but triggered the invalid hooks erroring.
Other Monior Change
Previously we'll prefer to resolve CJS as there're 2 versions of react, using CJS assets will help let them pass by require-hook to use canary react for app router bundling assets. But now we changed the approach to bundling nextjs runtime and react packages. Now we dropped the condition that prefered to resolve CJS exports for externals, since if you're putting them in
serverComponentsExternalPackages
, they're not using the built-in react, so could potentially having trouble if any dependency is using react but excluded in bundles. So far we didn't see any report to this.Closes NEXT-1286