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

Turbopack: migrate client references to single-graph-traversal #73322

Merged
merged 10 commits into from
Dec 11, 2024

Conversation

mischnic
Copy link
Contributor

@mischnic mischnic commented Nov 28, 2024

Closes PACK-3536

@ijjk ijjk added created-by: Turbopack team PRs by the Turbopack team. Turbopack Related to Turbopack with Next.js. labels Nov 28, 2024
@mischnic mischnic changed the title migrate client references to single-traversal Turbopack : migrate client references to single-graph-traversal Nov 28, 2024
@mischnic mischnic changed the title Turbopack : migrate client references to single-graph-traversal Turbopack: migrate client references to single-graph-traversal Nov 28, 2024
@ijjk
Copy link
Member

ijjk commented Nov 28, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Nov 28, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
buildDuration 39.2s 39.9s ⚠️ +778ms
buildDurationCached 37.5s 31.2s N/A
nodeModulesSize 409 MB 409 MB ⚠️ +1.15 kB
nextStartRea..uration (ms) 892ms 850ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
1187-HASH.js gzip 50.4 kB 50.4 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.3 kB 5.3 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 232 B 235 B N/A
main-HASH.js gzip 33.8 kB 33.7 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.44 kB 4.43 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.34 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.59 kB 3.59 kB
Client Build Manifests
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
_buildManifest.js gzip 747 B 745 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
index.html gzip 524 B 522 B N/A
link.html gzip 539 B 537 B N/A
withRouter.html gzip 520 B 519 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
edge-ssr.js gzip 127 kB 127 kB N/A
page.js gzip 202 kB 202 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
middleware-b..fest.js gzip 671 B 667 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31 kB 31 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
523-experime...dev.js gzip 322 B 322 B
523.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 322 kB 322 kB
app-page-exp..prod.js gzip 127 kB 127 kB
app-page-tur..prod.js gzip 140 kB 140 kB
app-page-tur..prod.js gzip 135 kB 135 kB
app-page.run...dev.js gzip 312 kB 312 kB
app-page.run..prod.js gzip 122 kB 122 kB
app-route-ex...dev.js gzip 37.1 kB 37.1 kB
app-route-ex..prod.js gzip 25.1 kB 25.1 kB
app-route-tu..prod.js gzip 25.1 kB 25.1 kB
app-route-tu..prod.js gzip 24.9 kB 24.9 kB
app-route.ru...dev.js gzip 38.7 kB 38.7 kB
app-route.ru..prod.js gzip 24.9 kB 24.9 kB
pages-api-tu..prod.js gzip 9.56 kB 9.56 kB
pages-api.ru...dev.js gzip 11.4 kB 11.4 kB
pages-api.ru..prod.js gzip 9.56 kB 9.56 kB
pages-turbo...prod.js gzip 21.3 kB 21.3 kB
pages.runtim...dev.js gzip 27 kB 27 kB
pages.runtim..prod.js gzip 21.3 kB 21.3 kB
server.runti..prod.js gzip 915 kB 915 kB
Overall change 2.35 MB 2.35 MB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js mischnic/single-traversal-client-references Change
0.pack gzip 2.04 MB 2.04 MB N/A
index.pack gzip 71.2 kB 72.1 kB ⚠️ +928 B
Overall change 71.2 kB 72.1 kB ⚠️ +928 B
Diff details
Diff for main-HASH.js

Diff too large to display

Commit: 3f5ca85

@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from da8ef0c to 699e475 Compare November 28, 2024 17:43
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 258f18e to 9551e4a Compare November 28, 2024 17:44
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 699e475 to 356c01e Compare November 29, 2024 09:17
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 9551e4a to 293356b Compare November 29, 2024 09:17
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 356c01e to 4860977 Compare December 2, 2024 10:13
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch 2 times, most recently from aca380f to 5f3993d Compare December 2, 2024 11:25
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 4860977 to 3fab151 Compare December 3, 2024 12:10
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 5f3993d to 8336bfa Compare December 3, 2024 12:10
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 3fab151 to e7db9dc Compare December 3, 2024 15:17
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 8336bfa to d63879a Compare December 3, 2024 15:17
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from e7db9dc to 41613ab Compare December 4, 2024 15:26
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from d63879a to d7231dd Compare December 4, 2024 15:26
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 41613ab to 85864d7 Compare December 4, 2024 16:12
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from d7231dd to 5f0fcb0 Compare December 4, 2024 16:12
@mischnic mischnic requested a review from sokra December 4, 2024 18:46
@mischnic mischnic marked this pull request as ready for review December 4, 2024 18:46
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from 85864d7 to f001e9d Compare December 4, 2024 18:46
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 5f0fcb0 to 61c8e09 Compare December 4, 2024 18:46
@ijjk ijjk added the type: next label Dec 4, 2024
@mischnic mischnic force-pushed the mischnic/mischnic/single-traversal-server-actions branch from f001e9d to cecd490 Compare December 5, 2024 12:09
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch 3 times, most recently from 155b5ed to d3f5611 Compare December 10, 2024 15:43
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch 3 times, most recently from 5051ae5 to 1051e12 Compare December 10, 2024 21:04
@mischnic
Copy link
Contributor Author

@sokra I had to rework the order in which client references are collected in the "Collect client references in correct order" commit 1051e12
We rely on the order of the client_references: Vec for CSS client reference ordering. So now, that is using topological order as well.

Comment on lines +533 to +535
// The previous iterations above (might) have added the entry node, but not actually visited it.
visited_modules.remove(&entry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can that happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually intentional:
In the dev case, the graphs for each layout segment contain the rsc_entry (to be able to easily traverse them all with the same entry), which is added manually (and doesn't actually has its references visited).
And the visited_modules here are simply sourced from the nodes in the graph.

So to actually make this very last traversal do something, it has to remove the entry again.

@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 1051e12 to 3960bda Compare December 11, 2024 08:55
@mischnic mischnic force-pushed the mischnic/single-traversal-client-references branch from 3960bda to 3f5ca85 Compare December 11, 2024 11:30
@mischnic mischnic merged commit 8c66f84 into canary Dec 11, 2024
110 of 115 checks passed
@mischnic mischnic deleted the mischnic/single-traversal-client-references branch December 11, 2024 12:13
mischnic added a commit that referenced this pull request Dec 17, 2024
The client reference manifest has to contain all client references for
all layout segments. Currently it just contains all client references
but they are attributed to the first server component that uses it.

Reverts the approach of #73322 and
use the old implementation again
mischnic added a commit that referenced this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Bypass Graphite Optimization Ignore Graphite CI optimizations, run the full CI suite. https://graphite.dev/docs/stacking-and-ci created-by: Turbopack team PRs by the Turbopack team. Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants