fix(esm-lib): mark entry deps exports as used to fix shared chunk splitting#13421
fix(esm-lib): mark entry deps exports as used to fix shared chunk splitting#13421JSerFeng wants to merge 4 commits intoweb-infra-dev:mainfrom
Conversation
…ircular imports When SplitChunksPlugin extracts shared modules (e.g. lib.js with `export * from './a'`) into a shared chunk, tree-shaking may leave the re-exported modules (a.js, b.js) in their respective entry chunks. This caused the shared chunk to generate circular `import` statements back to entry chunks. Add `pull_module_into_non_entry_chunks` which runs before `ensure_entry_exports` during optimize_chunks. It moves modules from entry chunks into non-entry initial (shared) chunks when those modules are referenced by the shared chunk's modules, breaking the cycle. Also fix a `break` bug in `ensure_entry_exports` that only marked one entry chunk as dirty when multiple entries were affected.
Verifies that when multiple entries import different exports from a shared lib module (which uses `export * from`), the re-exported modules are correctly placed in the shared chunk and each entry imports only its needed exports without circular chunk dependencies.
…chunks - Sort chunk keys and module identifiers for deterministic iteration, preventing nondeterministic output when multiple shared chunks compete for the same module. - Filter out DynamicImport/DynamicImportEager edges so that lazily imported modules are not eagerly pulled into shared chunks. - Disconnect entry-module mapping when moving a module that is registered as an entry module, keeping the chunk graph consistent.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eade997b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for dep_id in module.get_dependencies() { | ||
| if let Some(dep_module) = module_graph.module_identifier_by_dependency_id(dep_id) { | ||
| if !entry_modules.contains(dep_module) { | ||
| exports_info_artifact | ||
| .get_exports_info_data_mut(dep_module) |
There was a problem hiding this comment.
Avoid marking every direct entry dependency as fully used
This loop applies set_used_in_unknown_way(None) to every target of module.get_dependencies(), not just re-export edges. In exports_info_setter::set_used_in_unknown_way that upgrades all named exports plus other_exports_info to Unknown, and later link.rs only drops exports whose state is Unused. So an entry like import { foo } from './helper'; export { api } will now keep all of helper's exports live in the emitted ESM-library chunk, even if only foo is needed. That is a broad tree-shaking regression for normal implementation-detail imports; this should be limited to the specific re-export dependencies that need to stay active.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Fixes an ESM-library edge case where per-runtime export usage caused re-export connections to be pruned, preventing shared modules from being placed into shared chunks during chunk optimization.
Changes:
- Mark entry modules and their direct dependencies as “used in unknown way” during
finish_modulesto keep re-export connections active across runtimes. - Fix
ensure_entry_exportsso it marks all relevant dirty entry chunks (not just the first match). - Add a regression test covering multi-entry shared re-exports and the expected ESM output shape.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/side-effect.js | Adds a side-effect module used by the shared library to exercise side-effects optimization interplay. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/rspack.config.js | Defines a multi-entry build (main, entry2) for the regression scenario. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/lib.js | Shared library that re-exports from a and b, reproducing the per-runtime re-export pruning case. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/entry1.js | Entry that consumes/export a and asserts runtime behavior via ESM self-import. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/entry2.js | Second entry that consumes/exports b. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/a.js | Provides a export for the shared re-export chain. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/b.js | Provides b export for the shared re-export chain. |
| tests/rspack-test/esmOutputCases/split-chunks/multi-entry-shared-reexport/snapshots/esm.snap.txt | Snapshot asserting the expected chunking + clean ESM re-exports. |
| crates/rspack_plugin_esm_library/src/plugin.rs | Marks entry deps’ exports as used in unknown way to keep re-export connections active across runtimes. |
| crates/rspack_plugin_esm_library/src/optimize_chunks.rs | Removes an early break so all dirty entry chunks are collected correctly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will not alter performance
Comparing Footnotes
|
…itting Root cause: `export * from './a'` creates a lazy dependency (set_lazy) when the target module is side-effect-free. The lazy connection's activity depends on per-runtime export usage — if entry2 doesn't use export `a`, the lib→a connection is inactive for entry2's runtime. This causes `a.js` to only appear in entry1's chunk, failing the minChunks:2 threshold for SplitChunksPlugin extraction. Fix: in finish_modules, also mark the direct dependencies of entry modules as "used in unknown way" (all runtimes). This keeps re-export connections active across all runtimes so modules like a.js/b.js appear in all entry chunks and get correctly extracted to the shared chunk by SplitChunksPlugin. This replaces the previous pull_module_into_non_entry_chunks approach which moved modules between chunks post-split (treating the symptom rather than the root cause).
4eade99 to
c0ad962
Compare
Summary
finish_modules, mark the direct dependencies of entry modules as "used in unknown way" (all runtimes), so that re-export connections remain active and modules are correctly deduplicated byRemoveDuplicateModulesPlugin.ensure_entry_exportsbreak bug that only marked one dirty entry chunk.esmOutputCases/split-chunks/multi-entry-shared-reexport.Root Cause
export * from './a'creates a lazyESMImportSideEffectDependencywhen the target module is side-effect-free. The lazy connection's activity depends on per-runtime export usage:lib.js'saexport is used →lib→aactive,lib→binactivelib.js'sbexport is used →lib→bactive,lib→ainactiveDuring
build_chunk_graph,a.jsonly appears in entry1's chunk andb.jsonly in entry2's chunk.RemoveDuplicateModulesPluginonly deduplicates modules present in multiple chunks, soa.js/b.jsare not extracted. Meanwhilelib.js(present in both) is extracted to a shared chunk — but it referencesa.js/b.jswhich remain in their respective entry chunks, creating circular imports.Fix
Mark direct dependencies of entry modules' exports as used in all runtimes during
finish_modules. This keeps re-export connections active across all runtimes, soa.jsandb.jsappear in both entry chunks and get correctly deduplicated byRemoveDuplicateModulesPlugininto the shared chunk alongsidelib.js.Test plan
esmOutputCases/split-chunks/multi-entry-shared-reexport