fix: preserve faster module concatenation semantics - #15257
LingyuCoder wants to merge 3 commits into
Conversation
📦 Binary Size-limit
❌ Size increased by 4.00KB from 68.15MB to 68.15MB (⬆️0.01%) |
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 5 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 popular-libsPath:
📁 react-10kPath:
📁 react-1kPath:
📁 react-5kPath:
📁 ui-componentsPath:
Generated by Rsdoctor GitHub Action |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b09b0e7f1d
ℹ️ 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".
| if let Some(concatenation_scope) = concatenation_scope.as_deref_mut() | ||
| && let Some(request) = request | ||
| { | ||
| // The primary request is emitted as a free expression. Preserve its | ||
| // identifiers so concatenated bindings cannot capture the external. | ||
| concatenation_scope.register_used_names_from_generated_code(request.primary()); |
There was a problem hiding this comment.
When faster module concatenation is enabled, this reservation only covers the var/promise/const/let/assign and catch-all arms. Earlier concatenatable arms also emit free identifiers—for example, externalsType: "window" emits window[...], while commonjs emits require(...). A top-level window or require binding from another concatenated module can therefore capture the generated external expression and change which value is loaded. Reserve the identifiers emitted by those explicit arms as well.
Useful? React with 👍 / 👎.
| .downcast_ref::<ProvideDependency>() | ||
| .expect("ProvideDependencyTemplate should only be used for ProvideDependency"); | ||
| let rendered_identifier = source.ensure_generated_top_level_symbol(dep.identifier.clone()); | ||
| let rendered_identifier = source.rebind_generated_global_symbol(dep.identifier.clone()); |
There was a problem hiding this comment.
When a ProvidePlugin request fails to resolve with bail: false, the connection lookup below returns None and the template intentionally leaves the original global reference untouched. Registering RebindGlobal before that check nevertheless reclassifies matching globals as a generated top-level binding; because no declaration is emitted, a name collision with another concatenated module can rename the surviving global reference and make it resolve to an undefined identifier. Delay rebinding until after the connection has been found.
Useful? React with 👍 / 👎.
1 participant
Summary
for-in/for-ofassignment heads cannot be represented safely by make-time scope metadata.Related links
Validation
pnpm run build:binding:devpnpm run test:unit(9224 passed, 4 skipped)cargo fmt --all --checkcargo check -p rspack_core -p rspack_plugin_javascriptrstackjs/build-tools-performanceprojects; complete output trees are byte-for-byte identicalChecklist