-
Notifications
You must be signed in to change notification settings - Fork 26.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
Consolidate manifest writing for Turbopack in a single function #59663
Consolidate manifest writing for Turbopack in a single function #59663
Conversation
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
@@ -552,7 +552,7 @@ async function startWatcher(opts: SetupOpts) { | |||
hmrBuilding = true | |||
} | |||
hmrPayloads.set(`${key}:${id}`, payload) | |||
hmrEventHappend = true | |||
hmrEventHappened = true |
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.
Noticed a typo here, corrected it.
Stats from current PRDefault BuildGeneral
Client Bundles (main, webpack)
Legacy Client Bundles (polyfills)
Client Pages
Client Build Manifests
Rendered Page Sizes
Edge SSR bundle Size
Middleware size
Next Runtimes
|
@@ -1041,6 +1041,18 @@ async function startWatcher(opts: SetupOpts) { | |||
) | |||
} | |||
|
|||
async function writeManifests(): Promise<void> { |
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.
@@ -1166,6 +1178,8 @@ async function startWatcher(opts: SetupOpts) { | |||
'edge' | |||
) | |||
await loadMiddlewareManifest('instrumentation', 'instrumentation') | |||
await writeManifests() |
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.
Noticed this bug where it didn't write the manifest with instrumentation
await writeBuildManifest(opts.fsChecker.rewrites) | ||
await writeAppBuildManifest() | ||
await writePagesManifest() | ||
await writeAppPathsManifest() | ||
await writeMiddlewareManifest() | ||
await writeActionManifest() | ||
await writeFontManifest() | ||
await writeLoadableManifest() | ||
await writeFallbackBuildManifest() |
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.
do we want to parallelize these? I have not idea if they have side effects so maybe not safe but seemingly the io parts could be done in parallel if they do not interact
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.
Overall we'll likely want to make these sync fs actually, but kept it as-is for now, can optimize later 👍
What?
I'm working on consolidating a bunch of the file writing related pieces in the Turbopack handling in the dev server so that it can be abstracted out as it's needed for
next build
too.These changes make sure that there is a single
writeManifests()
instead of picking specific manifests to write.We can optimize this later but for now the overhead of writing them to disk separately is negligible.
Closes NEXT-1884