Skip to content

fix: preserve used re-exports under preserveModules (#9122)#9934

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/preserve-modules-reexport-9122
Jun 23, 2026
Merged

fix: preserve used re-exports under preserveModules (#9122)#9934
graphite-app[bot] merged 1 commit into
mainfrom
fix/preserve-modules-reexport-9122

Conversation

@IWANABETHATGUY

Copy link
Copy Markdown
Member

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

@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/preserve-modules-reexport-9122 branch from 2cb6dd0 to 25dd0e0 Compare June 23, 2026 07:20
@IWANABETHATGUY IWANABETHATGUY marked this pull request as draft June 23, 2026 07:20
@netlify

netlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 2cb6dd0
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a3a33a1f3f51f0008ef204e
😎 Deploy Preview https://deploy-preview-9934--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

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

@netlify

netlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 7f03488
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a3a4dddb9f7e70008038702

@codspeed-hq

codspeed-hq Bot commented Jun 23, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing fix/preserve-modules-reexport-9122 (f77bfe9) with main (b311823)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

IWANABETHATGUY commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jun 23, 9:11 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 23, 9:11 AM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
  • Jun 23, 9:16 AM UTC: Merged by the Graphite merge queue.

## 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)
@graphite-app graphite-app Bot force-pushed the fix/preserve-modules-reexport-9122 branch from f77bfe9 to 7f03488 Compare June 23, 2026 09:11
@graphite-app graphite-app Bot merged commit 7f03488 into main Jun 23, 2026
33 of 34 checks passed
@graphite-app graphite-app Bot deleted the fix/preserve-modules-reexport-9122 branch June 23, 2026 09:16
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)
@rolldown-guard rolldown-guard Bot mentioned this pull request Jun 24, 2026
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>
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

preserveModules drops re-exports of imported bindings

2 participants