fix: align module external remapping with webpack by JSerFeng · Pull Request #13802 · web-infra-dev/rspack

JSerFeng · 2026-04-22T17:11:24Z

Summary

align module external remapping with webpack by generating a remapped namespace object instead of using mangled bundle-local names as external property access
add focused Rust unit coverage for external remapping behavior in rspack_core
add two regression fixtures: the original output-module/issue-13733 reproducer and a migrated webpack node/output-module-external case

Root cause

When a module external fell back to namespace access in module output, Rspack mixed two different name domains:

the bundle-internal used_name produced by export mangling
the real export name exposed by the external module

That made namespace property access use mangled names like ns.a instead of the real external export name such as ns.EventEmitter.

Impact

This keeps module externals compatible with optimization.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 -- --nocapture
cargo test -p rspack_plugin_javascript --lib -- --nocapture
cargo test -p rspack_plugin_esm_library --lib -- --nocapture
cargo fmt --all --check
git diff --check
node --check tests/rspack-test/configCases/node/output-module-external/import.js
node --check tests/rspack-test/configCases/node/output-module-external/require.js
node --check tests/rspack-test/configCases/node/output-module-external/rspack.config.js
node --check tests/rspack-test/configCases/node/output-module-external/test.config.js
node --check tests/rspack-test/configCases/node/output-module-external/test.filter.js
node --check tests/rspack-test/configCases/node/output-module-external/stream.js
node -e "console.log(require('./tests/rspack-test/configCases/node/output-module-external/test.filter.js')())"

Notes

I could not run the new rspack-test config cases locally because this environment does not have pnpm or corepack. The migrated JS fixture was syntax-checked instead.

codspeed-hq · 2026-04-22T17:31:04Z

Merging this PR will not alter performance

✅ 34 untouched benchmarks

_{Comparing JSerFeng:fy/issue-13733-webpack-module-external-remap (8be4b76) with main (d5ce815)}

JSerFeng · 2026-04-23T02:56:18Z

@codex review

chatgpt-codex-connector

💡 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".

Copilot

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_core external module codegen.
Add Rust unit tests validating remapping behavior (including nested exports).
Add regression fixtures for output-module/issue-13733 and migrated node/output-module-external cases.

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.

Copilot · 2026-04-23T06:25:23Z

+    let return_x = runtime_template.returning_function("x", "");
+    format!(
+      "var x = {create_namespace_object};\nvar y = {};",
+      runtime_template.returning_function(&return_x, "x")
+    )


The remapping init code introduces very generic top-level identifiers (var x, var y). When this ExternalModule is concatenated (i.e. concatenation_scope is active), multiple concatenated modules share one scope, so these names can collide with other concatenated modules or other remapped externals and cause incorrect runtime behavior. Consider generating helper identifiers that are unique per external (e.g. derived from ident), or avoid introducing named variables by inlining the helper functions / using an IIFE, and update render_module_external_remapping to reference those unique helper names.

Copilot · 2026-04-23T06:25:23Z

+          let exports_info = compilation
+            .exports_info_artifact
+            .get_prefetched_exports_info(&self.identifier(), PrefetchExportsInfoMode::Full);


PrefetchExportsInfoMode::Full is documented as “should only be used in hash calculation” and it recursively prefetches nested exports info. Using it here in the module external codegen path can add noticeable overhead for builds with many externals. If nested exports data is only needed for the subset of exports that require remapping, consider switching to Default and lazily prefetching nested exports_info only when export_info.exports_info() is present (or otherwise limiting the prefetch scope), or add a justification comment if Full is required here.

Suggested change

let exports_info = compilation

.exports_info_artifact

.get_prefetched_exports_info(&self.identifier(), PrefetchExportsInfoMode::Full);

// Avoid recursively prefetching nested exports info in the external module

// codegen path; `Full` is intended for hash calculation and can add

// noticeable overhead for builds with many externals.

let exports_info = compilation

.exports_info_artifact

.get_prefetched_exports_info(&self.identifier(), PrefetchExportsInfoMode::Default);

Copilot · 2026-04-23T06:25:23Z

+  for (const [request, imported] of Object.entries(builtinImports)) {
+    expect(imported).toBeDefined();
+    expect(content).toMatch(
+      new RegExp(`import\\(\\s*['']${escapeRegExp(request)}['']\\s*\\)`)


The regexp used to assert presence of import(...) only matches single quotes (['']). Since the bundle output may use double quotes (especially after minification / formatting), this can make the test flaky. Consider using a quote class like ['"] (or otherwise matching both quote styles) so the assertion is robust across output formatting changes.

Suggested change

new RegExp(`import\$\\s*['']${escapeRegExp(request)}['']\\s*\$`)

new RegExp(`import\$\\s*['"]${escapeRegExp(request)}['"]\\s*\$`)

JSerFeng · 2026-04-23T07:04:46Z

@codex review

chatgpt-codex-connector · 2026-04-23T07:16:18Z

Codex Review: Didn't find any major issues. 🚀

ℹ️ 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".

fix(core): align module external remapping with webpack

d3eddc5

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

Comment thread crates/rspack_core/src/external_module.rs Outdated

chore(core): fix module external remapping ci

df7732e

JSerFeng marked this pull request as ready for review April 23, 2026 06:16

Copilot AI review requested due to automatic review settings April 23, 2026 06:16

Copilot started reviewing on behalf of JSerFeng April 23, 2026 06:16 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

JSerFeng changed the title ~~[codex] fix: align module external remapping with webpack~~ fix: align module external remapping with webpack Apr 23, 2026

github-actions Bot added the release: bug fix release: bug related release(mr only) label Apr 23, 2026

JSerFeng added 2 commits April 23, 2026 14:45

fix(core): avoid remapping directly used exports

a1854af

test: format output module external fixture

4baaae2

fix(core): keep module externals tree-shaking friendly

8be4b76

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: align module external remapping with webpack#13802

fix: align module external remapping with webpack#13802
JSerFeng wants to merge 5 commits intoweb-infra-dev:mainfrom
JSerFeng:fy/issue-13733-webpack-module-external-remap

JSerFeng commented Apr 22, 2026

Uh oh!

codspeed-hq Bot commented Apr 22, 2026 •

edited

Loading

Uh oh!

JSerFeng commented Apr 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

JSerFeng commented Apr 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	new RegExp(`import\\(\\s['']${escapeRegExp(request)}['']\\s\\)`)
	new RegExp(`import\\(\\s['"]${escapeRegExp(request)}['"]\\s\\)`)

Uh oh!

Conversation

JSerFeng commented Apr 22, 2026

Summary

Root cause

Impact

Validation

Notes

Uh oh!

codspeed-hq Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

JSerFeng commented Apr 23, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

JSerFeng commented Apr 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codspeed-hq Bot commented Apr 22, 2026 •

edited

Loading