-
Notifications
You must be signed in to change notification settings - Fork 27k
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
feat(turbopack): support basic next/dynamic #56389
Conversation
f07b7e6
to
0ee90a1
Compare
Tests Passed |
Stats from current PRDefault BuildGeneral Overall increase
|
vercel/next.js canary | vercel/next.js feat-dynamic-id | Change | |
---|---|---|---|
buildDuration | 10.2s | 10.3s | N/A |
buildDurationCached | 6s | 6.6s | |
nodeModulesSize | 172 MB | 172 MB | |
nextStartRea..uration (ms) | 531ms | 525ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js feat-dynamic-id | 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 | 252 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 feat-dynamic-id | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js feat-dynamic-id | 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 feat-dynamic-id | 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 feat-dynamic-id | Change | |
---|---|---|---|
index.html gzip | 529 B | 530 B | N/A |
link.html gzip | 542 B | 542 B | ✓ |
withRouter.html gzip | 524 B | 525 B | N/A |
Overall change | 542 B | 542 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js feat-dynamic-id | 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 feat-dynamic-id | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 624 B | 623 B | N/A |
middleware-r..fest.js gzip | 150 B | 151 B | N/A |
middleware.js gzip | 22.5 kB | 22.5 kB | N/A |
edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
Overall change | 1.92 kB | 1.92 kB | ✓ |
183c42a
to
8c24187
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.
Unfortunately this will have some conflicts with the chunking refactor that have landed this week.
packages/next-swc/crates/next-core/src/next_shared/transforms/next_dynamic.rs
Outdated
Show resolved
Hide resolved
203f65a
to
cb63322
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.
Great work OJ!
Vc::cell(evaluatable_assets), | ||
Vc::cell(evaluatable_assets.clone()), |
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.
You can avoid the clone, by creating only one cell and use that for both calls:
let evaluatable_assets = Vc::cell(evaluatable_assets)
Closes WEB-1702
This PR implements initial support for the
next/dynamic
in Turbopack, more specifically resolving some hydration errors and other components boot up cases.Previously, turbopack had partial next/dynamic support via its own mode (https://github.com/vercel/next.js/pull/56389/files#diff-e1af4f79cb88a73f819a25443d15ed4b1ffabcbb879256caa59b751fad46d7c4L68), which does a transform against
next/dynamic
wrapped import to embed dynamically resolvable chunk ids like (https://github.com/vercel/next.js/blob/ad42b610c25b72561ad367b82b1c7383fd2a5dd2/packages/next-swc/crates/next-transform-dynamic/tests/fixture/wrapped-import/output-turbo-dev-server.js).However, since next.js relies on static path to the chunks to the dynamic import and passing those ids in between client-server to ensure component load (and avoid hydration errors), it doesn't work out of the box. This PR changes turbopack's behavior to closely mimic what current next.js's webpack plugin does, by
dynamic(import())
react-loadable-manifest.json
react-loadable-manifest
as well as next.js can use it to pass it between server-client context.I left a small comment to the implementation (https://github.com/vercel/next.js/pull/56389/files#diff-bf12ed2c69d0bc89a06884779da4ae44967eb8becada031dea12bedef28e2622R155) for the lifecycle of this feature in case to fix further.
This makes to pass most of the basic next-dynamic related integration tests, except if the import have webpack specific features like
next.js/test/development/basic/next-dynamic/pages/dynamic/multiple-modules.js
Line 5 in ad42b61