fix: align module external remapping with webpack#13802
Merged
SyMind merged 15 commits intoMay 6, 2026
Merged
Conversation
Contributor
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3eddc5975
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns Rspack’s module externals behavior with webpack when optimization.mangleExports is enabled by generating a remapped namespace object for external property access, preventing mangled bundle-local names from being used against real external exports. Adds targeted Rust unit coverage and new rspack-test fixtures to prevent regressions (incl. #13733).
Changes:
- Implement module-external namespace remapping logic in
rspack_coreexternal module codegen. - Add Rust unit tests validating remapping behavior (including nested exports).
- Add regression fixtures for
output-module/issue-13733and migratednode/output-module-externalcases.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rspack_core/src/external_module.rs | Implements module-external remapping logic and adds focused Rust unit tests. |
| tests/rspack-test/configCases/output-module/issue-13733/test.config.js | Adds bundle selection for the new regression case. |
| tests/rspack-test/configCases/output-module/issue-13733/rspack.config.js | Reproducer config for #13733 (module output + mangleExports + chunk merge). |
| tests/rspack-test/configCases/output-module/issue-13733/index.js | Asserts emitted bundle output and runtime behavior stay correct. |
| tests/rspack-test/configCases/output-module/issue-13733/a.js | Minimal ESM import that exercises named module externals. |
| tests/rspack-test/configCases/output-module/issue-13733/b.js | Async import side module to force chunk merge scenario. |
| tests/rspack-test/configCases/node/output-module-external/test.filter.js | Filters test execution based on Node support for createRequire. |
| tests/rspack-test/configCases/node/output-module-external/test.config.js | Adds bundle selection for migrated node builtin externals case. |
| tests/rspack-test/configCases/node/output-module-external/stream.js | Helper module for stream assertion in the require-based test. |
| tests/rspack-test/configCases/node/output-module-external/rspack.config.js | Configures outputModule build and injects NODE_VERSION. |
| tests/rspack-test/configCases/node/output-module-external/require.js | Migrated require-based node builtin external coverage. |
| tests/rspack-test/configCases/node/output-module-external/import.js | Migrated dynamic-import-based node builtin external coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…k-module-external-remap # Conflicts: # crates/rspack_core/src/external_module.rs
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52e71a8947
ℹ️ 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".
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Contributor
|
📝 Benchmark detail: Open
Base persistent cache hit rate:
|
Contributor
Contributor
ahabhgk
previously approved these changes
May 6, 2026
ahabhgk
approved these changes
May 6, 2026
SyMind
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
moduleexternal remapping with webpack by generating a remapped namespace object instead of using mangled bundle-local names as external property accessrspack_coreoutput-module/issue-13733reproducer and a migrated webpacknode/output-module-externalcaseRoot cause
When a
moduleexternal fell back to namespace access in module output, Rspack mixed two different name domains:used_nameproduced by export manglingThat made namespace property access use mangled names like
ns.ainstead of the real external export name such asns.EventEmitter.Impact
This keeps
moduleexternals compatible withoptimization.mangleExports, concatenation, and namespace fallback behavior, while matching webpack's remapping strategy more closely.Fixes #13733.
Validation
cargo test -p rspack_core module_external_remapping --lib -- --nocapturecargo test -p rspack_plugin_javascript --lib -- --nocapturecargo test -p rspack_plugin_esm_library --lib -- --nocapturecargo fmt --all --checkgit diff --checknode --check tests/rspack-test/configCases/node/output-module-external/import.jsnode --check tests/rspack-test/configCases/node/output-module-external/require.jsnode --check tests/rspack-test/configCases/node/output-module-external/rspack.config.jsnode --check tests/rspack-test/configCases/node/output-module-external/test.config.jsnode --check tests/rspack-test/configCases/node/output-module-external/test.filter.jsnode --check tests/rspack-test/configCases/node/output-module-external/stream.jsnode -e "console.log(require('./tests/rspack-test/configCases/node/output-module-external/test.filter.js')())"Notes
rspack-testconfig cases locally because this environment does not havepnpmorcorepack. The migrated JS fixture was syntax-checked instead.