Skip to content

Comments

fix(renamer): nested binding shadowing external module namespace in UMD/IIFE formats#8083

Merged
IWANABETHATGUY merged 2 commits intomainfrom
fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE
Jan 27, 2026
Merged

fix(renamer): nested binding shadowing external module namespace in UMD/IIFE formats#8083
IWANABETHATGUY merged 2 commits intomainfrom
fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE

Conversation

@Dunqing
Copy link
Collaborator

@Dunqing Dunqing commented Jan 27, 2026

Fixes #8058

In UMD/IIFE/CJS formats, external modules become factory function parameters. When a nested binding (e.g., constructor parameter) has the same name as an external module, it shadows the factory parameter, causing incorrect references.

Before (broken):

// Input                                                                                                                                                          
import Quill from 'quill';                                                                                                                                        
class Editor {                                                                                                                                                    
  constructor(quill) {                                                                                                                                            
    console.log(Quill.events); // Should reference external module                                                                                                
  }                                                                                                                                                               
}                                                                                                                                                                 
                                                                                                                                                                  
// Output (UMD) - quill.default incorrectly references constructor param                                                                                          
(function(exports, quill) {                                                                                                                                       
  quill = __toESM(quill);                                                                                                                                         
  class Editor {                                                                                                                                                  
    constructor(quill) {. // Shadows factory param!                                                                                                         
      console.log(quill.default.events); // Wrong reference                                                                                                       
    }                                                                                                                                                             
  }                                                                                                                                                               
})                                                                                                                                                                

After (fixed):

// Output (UMD) - nested param renamed to avoid shadowing                                                                                                         
(function(exports, quill) {                                                                                                                                       
  quill = __toESM(quill);                                                                                                                                         
  class Editor {                                                                                                                                                  
    constructor(quill$1) {  // Renamed to avoid conflict                                                                                                      
      console.log(quill.default.events); // Correct reference                                                                                                     
    }                                                                                                                                                             
  }                                                                                                                                                               
})              

Copilot AI review requested due to automatic review settings January 27, 2026 07:06
@netlify
Copy link

netlify bot commented Jan 27, 2026

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 4cb3f0b
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6978972f3e1d42000892633b
😎 Deploy Preview https://deploy-preview-8083--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a renaming bug where nested bindings (e.g. constructor parameters) could shadow external module namespaces that become wrapper/factory parameters in UMD/IIFE/CJS outputs, leading to incorrect references like quill.default resolving to a local parameter instead of the external module. It also adds a regression test to lock in the correct behavior for UMD and refactors the renamer to generally handle wrapper/factory parameter shadowing.

Changes:

  • Extend Renamer/NestedScopeRenamer to track whether the current format uses wrapper/factory parameters and to rename nested bindings that would shadow: (1) CJS wrapper parameters (exports, module) and (2) external module factory parameters in IIFE/UMD/CJS.
  • Simplify external namespace handling in deconflict_chunk_symbols by removing a special-case facade-symbol path and routing nested-scope shadowing through the new wrapper-parameter renaming logic.
  • Add a new integration test topic (umd_external_namespace_shadowing) with configuration, source, and snapshot artifacts, and register its hashed filename in the global snapshot.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/rolldown/src/utils/renamer.rs Adds has_factory_params to Renamer, adjusts name-availability logic, and introduces rename_bindings_shadowing_wrapper_params to rename nested bindings that would shadow CJS wrapper params or external module factory params.
crates/rolldown/src/utils/chunk/deconflict_chunk_symbols.rs Wires the new wrapper-parameter shadowing pass into the nested-scope renaming pipeline and removes a dedicated root-scope registration path for certain external facade namespaces.
crates/rolldown/tests/rolldown/topics/deconflict/umd_external_namespace_shadowing/test.js New regression test input that imports an external default (Quill) and defines a constructor parameter quill to exercise the shadowing case.
crates/rolldown/tests/rolldown/topics/deconflict/umd_external_namespace_shadowing/main.js Re-exports the test module to form the entry point for the new deconflict scenario.
crates/rolldown/tests/rolldown/topics/deconflict/umd_external_namespace_shadowing/_config.json Test configuration that marks quill and mod as externals and sets the output format to UMD with a named global.
crates/rolldown/tests/rolldown/topics/deconflict/umd_external_namespace_shadowing/artifacts.snap Snapshot of warnings and generated UMD output verifying that the external factory param (quill) remains the namespace while the nested constructor param is renamed (quill$1).
crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap Registers the new test directory’s output file and its hashed filename in the global filename-with-hash snapshot.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 27, 2026

Benchmarks Rust

  • target: main(f3f0778)
  • pr: fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE(4cb3f0b)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     71.1±2.03ms        ? ?/sec    1.02     72.8±1.81ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     75.5±2.22ms        ? ?/sec    1.04     78.5±2.58ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    100.2±2.12ms        ? ?/sec    1.03    102.8±1.68ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    111.1±1.50ms        ? ?/sec    1.02    113.6±1.50ms        ? ?/sec
bundle/bundle@threejs                                        1.00     36.2±2.14ms        ? ?/sec    1.00     36.1±0.64ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     40.5±0.58ms        ? ?/sec    1.02     41.5±0.54ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    360.6±3.07ms        ? ?/sec    1.02    366.3±5.68ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    416.7±3.51ms        ? ?/sec    1.02    423.6±4.07ms        ? ?/sec
scan/scan@rome_ts                                            1.01     80.3±1.66ms        ? ?/sec    1.00     79.7±1.77ms        ? ?/sec
scan/scan@threejs                                            1.00     27.5±0.41ms        ? ?/sec    1.00     27.7±0.44ms        ? ?/sec
scan/scan@threejs10x                                         1.01    287.3±5.11ms        ? ?/sec    1.00    283.6±4.17ms        ? ?/sec

@Dunqing Dunqing changed the title fix(renamer): nested binding shadowing external module namespace in UMD/IIFE format fix(renamer): nested binding shadowing external module namespace in UMD/IIFE formats Jan 27, 2026
@Dunqing Dunqing added the test: vite-tests Trigger Vite tests workflow on this PR label Jan 27, 2026
Copilot AI review requested due to automatic review settings January 27, 2026 08:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Dunqing
Copy link
Collaborator Author

Dunqing commented Jan 27, 2026

It looks like the Vite failure isn't related to this PR.

@Dunqing Dunqing force-pushed the fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE branch from cf8fc4b to a65f15b Compare January 27, 2026 09:19
@hyf0 hyf0 enabled auto-merge (squash) January 27, 2026 10:14
@hyf0 hyf0 force-pushed the fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE branch from a65f15b to 2b03e59 Compare January 27, 2026 10:14
Copilot AI review requested due to automatic review settings January 27, 2026 10:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@IWANABETHATGUY IWANABETHATGUY merged commit ceb89cb into main Jan 27, 2026
40 of 42 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the fix--nested-binding-shadowing-external-module-namespace-in--UMD/IIFE branch January 27, 2026 12:51
This was referenced Jan 28, 2026
shulaoda added a commit that referenced this pull request Jan 28, 2026
## [1.0.0-rc.2] - 2026-01-28

⚡ Lazy Barrel Optimization

- Skips compilation of unused re-export modules in side-effect-free barrel modules
- Particularly beneficial for projects importing from large component libraries

For large component libraries like Ant Design, when you import just one component:

```js
import { Button } from 'antd';
Button;
```

| Metric                    | Without lazy barrel | With lazy barrel |
| -------------------- | --------------------- | ---------------- |
| Modules compiled | 2986                       | 250              |
| Build time               | ~210ms                  | ~50ms            |

By enabling lazy barrel, Rolldown reduces the number of compiled modules by 90% and speeds up the build by 2-4x. 

Enable it in your config:

```js
export default {
  experimental: {
    lazyBarrel: true,
  },
};
```

For more details, see the https://rolldown.rs/in-depth/lazy-barrel-optimization

### 💥 BREAKING CHANGES

- expose `\0rolldown/runtime` in transform hook (#8068) by @hyf0
- rename `rolldown:runtime` to `\0rolldown/runtime.js` (#8067) by @hyf0

### 🚀 Features

- remove inlined constants in smart mode (#8085) by @sapphi-red
- allow more options for `this.emitFile` with `type: 'prebuilt-chunk'` (#8062) by @sapphi-red
- warn when both code and postBanner contain shebang (#8039) by @Copilot

### 🐛 Bug Fixes

- update the links to Rolldown docs in the error messages (#8103) by @sapphi-red
- handle tsconfig.json load errors (#8105) by @sapphi-red
- include inlined constants in namespace object (#8099) by @sapphi-red
- vite test ci (#8084) by @IWANABETHATGUY
- renamer: nested binding shadowing external module namespace in UMD/IIFE formats (#8083) by @Dunqing
- deduplicate ESM chunk imports by canonical symbol (#8059) by @IWANABETHATGUY
- refine side-effect detection for BigInt and RegExp (#8060) by @IWANABETHATGUY
- rust: use string literal span for `new URL` error diagnostic (#8043) by @valadaptive
- rust: use ModuleType::Asset for `new URL` imports (#8035) by @valadaptive
- CJS-ESM interop - property assignment on CJS module exports (#8006) by @IWANABETHATGUY
- eliminate the facade chunk if the dynamic entry module has been merged into common chunk (#8046) by @IWANABETHATGUY
- Inlining dynamic imports broken with multiple entry points (#8037) by @IWANABETHATGUY
- devtools: revert `Chunk#id` to `Chunk#chunk_id` (#8040) by @hyf0
- invert `__exportAll` parameter logic to reduce default output size (#8036) by @Copilot
- `</script` tag search should be case insensitive (#8033) by @IWANABETHATGUY
- use directory name as-is for the variable name even if the name contained `.` (#8029) by @Copilot
- dev/lazy: remove unnecessary rewrite from top level `this` to `undefined` (#8020) by @hyf0
- dev/lazy: should keep lazy entries imports for patch file (#8019) by @hyf0
- `output.generatedCode.preset: 'es2015'` was not set by default (#8026) by @sapphi-red
- node: align option validator to types (#8023) by @sapphi-red
- node: allow `output.strictExecutionOrder` by the option validator (#8022) by @sapphi-red
- types: return `this` from on / off methods of `RolldownWatcher` (#8015) by @sapphi-red

### 🚜 Refactor

- rolldown_plugin_vite_dynamic_import_vars: remove v1 implementation (#8096) by @shulaoda
- rolldown_plugin_vite_import_glob: remove v1 implementation (#8095) by @shulaoda
- lazy-barrel: restructure lazy barrel implementation (#8070) by @shulaoda
- remove `use_built_ins` and `use_spread` from internal JSX options (#8079) by @sapphi-red
- remove `experimental.transformHiresSourcemap` (#8055) by @Copilot
- rust: use `is_data_url` more consistently (#8042) by @valadaptive
- use `FxIndexMap` to store EntryPoint (#8032) by @IWANABETHATGUY
- node: add type checks that ensures validator schema is up to date with types (#8024) by @sapphi-red

### 📚 Documentation

- link to vite plugin registry (#8086) by @sapphi-red
- lazy-barrel: improve documentation and enable in sidebar (#8072) by @shulaoda
- add more examples and details (#8054) by @sapphi-red
- in-depth: add dead code elimination page (#8007) by @sapphi-red
- update status from beta to release candidate (#8012) by @shulaoda

### ⚡ Performance

- run inline-const pass for modules that are affected by inlining (#8064) by @sapphi-red

### 🧪 Testing

- lazy-barrel: use package.json sideEffects instead of plugin hook (#8077) by @shulaoda
- lazy-barrel: enable tests and add treeshake-behavior cases (#8071) by @shulaoda

### ⚙️ Miscellaneous Tasks

- deps: update crate-ci/typos action to v1.42.3 (#8087) by @renovate[bot]
- deps: update rollup submodule for tests to v4.56.0 (#8073) by @sapphi-red
- deps: update oxc to v0.111.0 (#8063) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.21.6 (#8076) by @renovate[bot]
- deps: update test262 submodule for tests (#8074) by @sapphi-red
- deps: update crate-ci/typos action to v1.42.2 (#8069) by @renovate[bot]
- deps: update oxc apps (#8066) by @renovate[bot]
- remove `{@include ./foo.md}` from d.ts files (#8056) by @sapphi-red
- deps: update dependency oxlint-tsgolint to v0.11.2 (#8057) by @renovate[bot]
- deps: update github-actions (#8050) by @renovate[bot]
- deps: update npm packages (#8051) by @renovate[bot]
- deps: update rust crates (#8049) by @renovate[bot]
- debug: add IdxExt debug trait for human-readable index debugging (#8045) by @IWANABETHATGUY
- deps: update dependency rolldown-plugin-dts to v0.21.5 (#8034) by @renovate[bot]
- deps: update oxc resolver to v11.16.4 (#8031) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.21.4 (#8030) by @renovate[bot]
- deps: update dependency rust to v1.93.0 (#8018) by @renovate[bot]
- archive 2025 beta changelog (#8014) by @shulaoda
- update release workflow version pattern from beta to rc (#8013) by @shulaoda

### ❤️ New Contributors

* @valadaptive made their first contribution in [#8043](#8043)

Co-authored-by: shulaoda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: vite-tests Trigger Vite tests workflow on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build result have duplicate variable names

2 participants