fix: preserve used re-exports under preserveModules (#9122)#9934
Merged
Conversation
2cb6dd0 to
25dd0e0
Compare
✅ Deploy Preview for rolldown-rs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for rolldown-rs canceled.
|
Merging this PR will not alter performance
Comparing Footnotes
|
hyf0
approved these changes
Jun 23, 2026
Member
Author
Merge activity
|
## What Fixes #9122. Under `preserveModules`, a module that re-exports an imported binding (`export { X } from './y'`) was dropping `X` from its own emitted file's export list when `X`'s canonical home was another module. Canonical resolution makes consumers bind the canonical symbol directly and bypass the facade, so the facade was tree-shaken out of this module's interface even though it was still consumed *through* it. ## How - **`compute_cross_chunk_links`**: emit each preserved user module's full declared interface; the `used_symbol_refs` filter keeps only the ones actually used. - **`include_statements`**: add `preserve_reexported_interfaces`, a post-pass run after the inclusion fixpoint. It re-marks a re-export facade as used (and includes its re-export statement so the cross-chunk import is generated) only when its canonical symbol survived tree-shaking. The pass is made **chain-granular** via `normal_symbol_exports_chain_map`: a facade is preserved only when it lies on the export chain of a *used* import — i.e. some module imports it *through* this module. This avoids resurrecting a side-effect-only `wrapper` that re-exports `foo` from `a` when a consumer imports `foo` straight from `a`. - The synthetic runtime module is excluded so its helpers stay demand-driven. Genuinely-unused exports stay tree-shaken, matching Rollup. ## Tests - `issues/9122` fixture: entry imports `StateCode`/`getX` through `wrapper` (`export { … } from './wrapper.js'`), so `wrapper` correctly keeps them. - `preserve_modules/shake_unrelated_reexport` fixture: covers the dropped case — an unrelated re-export facade is not resurrected. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
f77bfe9 to
7f03488
Compare
graphite-app Bot
pushed a commit
that referenced
this pull request
Jun 23, 2026
…) (#9937) ## What Adds a regression fixture for #6010. Under `preserveModules`, a named export reached **only** through an `export * as` namespace chain was dropped from its module's emitted export list. Reproduction (`index → two → three`): ```js // index.js export * as two from './two.js'; // two.js export * as three from './three.js'; // three.js export const myVariable = 'world'; ``` - **Before:** `dist/three.js` emitted `export { three_exports };` — `myVariable` dropped. - **After:** `dist/three.js` emits `export { myVariable, three_exports };` — matching the issue's expected output and Rollup. ## Fix location This is the same root cause as #9122 — the named export's home survives tree-shaking (the namespace getter references it) but it was missing from the module's declared interface. The fix lands in the stacked PR #9934 (`preserve_reexported_interfaces` post-pass + full declared-interface emission in `compute_cross_chunk_links`), which already produces the correct output for this namespace-chain variant. **This PR is test-only** — it adds the `issues/6010` fixture covering the `export * as` chain, a distinct scenario from #9122's `export { x } from` re-export facades. > Stacked on #9934. Verified: the fixture fails on `main` (drops `myVariable`) and passes with #9934's fix in place. Closes #6010 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Merged
shulaoda
added a commit
that referenced
this pull request
Jun 24, 2026
## [1.1.3] - 2026-06-24 ### 🐛 Bug Fixes - `defer_drop` crashes the browser main thread (#9942) by @shulaoda - camel-case: correct camel case for nested values (#9933) by @kb019 - cli: display --help options in camelCase (#9941) by @IWANABETHATGUY - preserve used re-exports under preserveModules (#9122) (#9934) by @IWANABETHATGUY - watch: make close reentrant in event callbacks (#9904) by @hyf0 - git for windows treats symlink files as regular files (#9915) by @AliceLanniste - dev: cancel pending full reload on build error (#9903) by @h-a-n-a - chunking: pass plugin meta to codeSplitting groups name function (#9267) by @Kyujenius - dev: serve assets emitted during HMR/lazy compile (vite#22596) (#9815) by @h-a-n-a - release: dry-run step no longer publishes binding packages (#9866) by @Boshen ### 🚜 Refactor - rolldown_common: model ModuleId as a classified Path/Virtual/Bare enum (#9927) by @Boshen - remove unused LegacyModuleIdx (#9872) by @shulaoda - remove unused StmtInfos::get_namespace_stmt_info (#9870) by @shulaoda - remove unused Module::as_external_mut (#9871) by @shulaoda - remove unused EcmaAst::is_body_empty (#9869) by @shulaoda - drop dead is_css_module handling in resolve_dependencies (#9867) by @shulaoda - drop redundant with_commonjs on cjs source type (#9868) by @shulaoda ### 📚 Documentation - clarify on drafting PRs (#9952) by @h-a-n-a - update contribution guidelines (#9944) by @fubhy - note Rust crates don't follow semver in AGENTS.md (#9905) by @IWANABETHATGUY - add feedback form (#9159) by @TheAlexLichter ### ⚡ Performance - utils: avoid allocation in default_sanitize_file_name for clean names (#9928) by @Boshen - binding: box once-per-build futures before spawn_future (#9864) by @Boshen - utils: avoid wasted allocation in legitimize_identifier_name (#9926) by @Boshen - rolldown: fuse the canonical-name dedup and insert in the renamer (#9900) by @Boshen - rolldown: probe the name map once in ConflictResolver::resolve (#9899) by @Boshen - cut two heap allocations from wrapped ESM init finalize (#9901) by @Boshen - rolldown_plugin_vite_reporter: hoist invariant out_dir prefix out of reporter loop (#9873) by @shulaoda - drop throwaway Vec in wrapped esm init stmt (#9878) by @shulaoda - borrow owner_filename in build-import-analysis AddDeps (#9874) by @shulaoda ### 🧪 Testing - cover preserveModules named export via namespace re-export (#6010) (#9937) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deps: update napi to v3.9.4 (#9954) by @shulaoda - reduce noise from CODEOWNERS for trival changes (#9953) by @h-a-n-a - deps: update mimalloc-safe to 0.1.64 (#9950) by @shulaoda - deps: update rollup submodule for tests to v4.62.2 (#9931) by @rolldown-guard[bot] - deps: test mimalloc-safe upstream-mimalloc switch in CI (#9930) by @shulaoda - rolldown_plugin_vite_build_import_analysis: remove unused v2 code path (#9917) by @shulaoda - rolldown_plugin_vite_manifest: remove unused is_enable_v2 code path (#9916) by @shulaoda - rolldown_plugin_vite_asset_import_meta_url: remove unexposed native vite plugin (#9896) by @shulaoda - rolldown_plugin_vite_asset: remove unexposed native vite plugin (#9895) by @shulaoda - rolldown_plugin_vite_css_post: remove unexposed native vite plugin (#9894) by @shulaoda - rolldown_plugin_vite_css: remove unexposed native vite plugin (#9893) by @shulaoda - rolldown_plugin_vite_html_inline_proxy: remove unexposed native vite plugin (#9892) by @shulaoda - rolldown_plugin_vite_html: remove unexposed native vite plugin (#9891) by @shulaoda - deps: update github actions (#9909) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.0.2 (#9910) by @renovate[bot] - deps: update npm packages (#9912) by @renovate[bot] - deps: update github actions to v7 (#9913) by @renovate[bot] - deps: update rolldown-plugin-dts to ^0.26.0 (#9897) by @renovate[bot] - remove rolldown_filter_analyzer crate (#9865) by @Boshen ### ❤️ New Contributors * @fubhy made their first contribution in [#9944](#9944) Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
This was referenced Jun 30, 2026
This was referenced Jun 30, 2026
graphite-app Bot
pushed a commit
that referenced
this pull request
Jun 30, 2026
…10020) (#10027) ### What Under `preserveModules`, a default-imported JSON module emitted a named export for **every** top-level key: ```js // dist/data.js var data_default = { aBeeZee: [...], Abel: [...] }; export { Abel, aBeeZee, data_default as default }; // ^^^^ ^^^^^^^ no backing binding ``` Those per-key `var` bindings are folded into the self-contained default object by the finalizer (`try_inline_json_module_prop`), so `Abel` / `aBeeZee` reference declarations that don't exist. Node's ESM linker then throws at load time: ``` SyntaxError: Export 'Abel' is not defined in module ``` Regression introduced by #9934 (preserveModules emits the complete declared interface). Reported in #10020; breaks `cloudflare/vinext`'s library build (default-imports `fallback-metrics-data.json`). ### Fix In the preserveModules full-interface emission (`compute_cross_chunk_links.rs`), skip a JSON key whose symbol is absent from `json_module_none_self_reference_included_symbol` — the keys reached only through the module's own default-export object, which the finalizer inlines away (leaving no standalone binding). Keys reached by a named import or as part of a JSON *entry*'s interface stay in the set, so JSON-entry / no-bundle output is unchanged. ### Test `crates/rolldown/tests/rolldown/issues/10020` reproduces the issue: `_test.mjs` throws the exact `SyntaxError` on the base and passes with the fix. Fixes #10020. --- > First of a 2-PR stack. The namespace-import case (`import * as ns from './x.json'`) is completed in the stacked follow-up.
shulaoda
pushed a commit
that referenced
this pull request
Jun 30, 2026
…10028) ### What Builds on the parent PR (#10020). A JSON module imported as a namespace (`import * as ns from './x.json'`) keeps every top-level key materialized — the finalizer only inlines JSON keys when the module namespace object is **not** included (`need_inline_json_prop`, see `finalizer_context.rs`). > **This is pre-existing behavior, not a regression.** On `main` a namespace-imported JSON chunk under `preserveModules` already exports its full interface correctly — `import('./dist/data.js')` exposes every top-level key. Unlike #10020 (which *is* a #9934 regression), there is no namespace bug on `main`. I verified the guard test passes on `main`. The reason this PR exists is purely to keep the stack consistent: the parent PR's #10020 narrowing (skip keys absent from `json_module_none_self_reference_included_symbol`) is too coarse for the namespace case — those keys are absent from that set yet still materialized, so the **parent PR alone** would drop them: ```js // parent PR (#10020) alone: only the namespace object survives export { data_exports }; // with this PR: pre-existing full interface preserved export { answer, data_exports, data_default as default, nested }; ``` ### Fix Mirror the finalizer's **full** inlining condition rather than the key-set alone: only drop a key when JSON property inlining is actually active — JSON, ESM exports, and the namespace object **not** included. The stack's net behavior then matches `main` for the namespace case. ### Test `crates/rolldown/tests/rolldown/misc/preserve_modules/json_namespace_preserves_exports`: `import * as` a JSON module under preserveModules and assert (via `_test.mjs`) that `import('./dist/data.js')` re-exposes every top-level key plus `default`. Verified: passes on `main`, fails on the parent PR alone, passes here. --- > Stacked on the #10020 fix.
graphite-app Bot
pushed a commit
that referenced
this pull request
Jul 1, 2026
#10056) ### What Builds on the parent PR (#10020). A JSON module imported as a namespace (`import * as ns from './x.json'`) keeps every top-level key materialized — the finalizer only inlines JSON keys when the module namespace object is **not** included (`need_inline_json_prop`, see `finalizer_context.rs`). > **This is pre-existing behavior, not a regression.** On `main` a namespace-imported JSON chunk under `preserveModules` already exports its full interface correctly — `import('./dist/data.js')` exposes every top-level key. Unlike #10020 (which *is* a #9934 regression), there is no namespace bug on `main`. I verified the guard test passes on `main`. The reason this PR exists is purely to keep the stack consistent: the parent PR's #10020 narrowing (skip keys absent from `json_module_none_self_reference_included_symbol`) is too coarse for the namespace case — those keys are absent from that set yet still materialized, so the **parent PR alone** would drop them: ```js // parent PR (#10020) alone: only the namespace object survives export { data_exports }; // with this PR: pre-existing full interface preserved export { answer, data_exports, data_default as default, nested }; ``` ### Fix Mirror the finalizer's **full** inlining condition rather than the key-set alone: only drop a key when JSON property inlining is actually active — JSON, ESM exports, and the namespace object **not** included. The stack's net behavior then matches `main` for the namespace case. ### Test `crates/rolldown/tests/rolldown/misc/preserve_modules/json_namespace_preserves_exports`: `import * as` a JSON module under preserveModules and assert (via `_test.mjs`) that `import('./dist/data.js')` re-exposes every top-level key plus `default`. Verified: passes on `main`, fails on the parent PR alone, passes here. --- > Stacked on the #10020 fix.
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.
What
Fixes #9122. Under
preserveModules, a module that re-exports an imported binding (export { X } from './y') was droppingXfrom its own emitted file's export list whenX's canonical home was another module. Canonical resolution makes consumers bind the canonical symbol directly and bypass the facade, so the facade was tree-shaken out of this module's interface even though it was still consumed through it.How
compute_cross_chunk_links: emit each preserved user module's full declared interface; theused_symbol_refsfilter keeps only the ones actually used.include_statements: addpreserve_reexported_interfaces, a post-pass run after the inclusion fixpoint. It re-marks a re-export facade as used (and includes its re-export statement so the cross-chunk import is generated) only when its canonical symbol survived tree-shaking. The pass is made chain-granular vianormal_symbol_exports_chain_map: a facade is preserved only when it lies on the export chain of a used import — i.e. some module imports it through this module. This avoids resurrecting a side-effect-onlywrapperthat re-exportsfoofromawhen a consumer importsfoostraight froma.Genuinely-unused exports stay tree-shaken, matching Rollup.
Tests
issues/9122fixture: entry importsStateCode/getXthroughwrapper(export { … } from './wrapper.js'), sowrappercorrectly keeps them.preserve_modules/shake_unrelated_reexportfixture: covers the dropped case — an unrelated re-export facade is not resurrected.🤖 Generated with Claude Code