[turbopack] Simplify JS ChunkItem with a single impl in most cases#89548
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Tests Passed |
Merging this PR will not alter performance
Comparing Footnotes
|
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **437 kB** → **437 kB** ✅ -2 B81 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
5ae8915 to
c242537
Compare
ChunkItem with a single impl in most cases
c242537 to
0ee40d8
Compare
| fn content(self: Vc<Self>) -> Vc<EcmascriptChunkItemContent> { | ||
| panic!("content() should not be called"); | ||
| fn get_exports(&self) -> Vc<EcmascriptExports> { | ||
| // Merged modules inherit exports from their entry points |
There was a problem hiding this comment.
so this wouldn't be correct
i think it is ok, but a bail or a panic might be more useful
There was a problem hiding this comment.
Yeah, should be a panic.
get_exports should never be called on merged modules
lukesandberg
left a comment
There was a problem hiding this comment.
i guess after seeing this i do wonder if a macro for implementing chunk_item would be the better solution. The tradeoff is really the chunk_item_content api vs the EcmascriptChunkItem trait implementation being customized
ChunkItem with a single impl in most casesChunkItem with a single impl in most cases
0ee40d8 to
6afd193
Compare
6afd193 to
becdc84
Compare
…thod
The ChunkableModuleReference trait was removed in this PR. References that
need to be chunkable now implement the chunking_type() method on
ModuleReference to return Some(ChunkingType::Parallel { ... }) instead.

What
Removes the vast majority of
ChunkItemimplementations in favour of oneEcmascriptModuleChunkItem.Why
Almost all of the
ChunkItemimplementations are trivial and were, for the most part, just places where additional traits could be implemented.By removing all of the various, trivial implementations, this might open up opportunities to better implement other features across the entire codebase.
How
Most chunk items are migrated over to EcmascriptModuleChunkItem. The EcmascriptChunkPlaceable trait gains three new methods that modules can optionally override:
chunk_item_content()- generates the chunk item's JavaScript codechunk_item_content_ident()- returns the content identity for cache invalidationchunk_item_output_assets()- returns output assets the chunk item depends onThe generic EcmascriptModuleChunkItem delegates to these trait methods, eliminating the need for per-module wrapper structs.
Next Steps
This exposes a fairly obvious next step, which is to elide
ChunkItemcompletely, as mostas_chunk_item()functions are now just trivial calls toecmascript_chunk_item(Vc::upcast(*self), module_graph, *chunking_context).