Skip to content

feat: support inlineConst for CJS exports accessed through module.exports#8976

Merged
graphite-app[bot] merged 1 commit into
mainfrom
04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports
Apr 2, 2026
Merged

feat: support inlineConst for CJS exports accessed through module.exports#8976
graphite-app[bot] merged 1 commit into
mainfrom
04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports

Conversation

@h-a-n-a

@h-a-n-a h-a-n-a commented Apr 1, 2026

Copy link
Copy Markdown
Member

Summary

When .default represents module.exports, properties accessed through it (e.g. ns.default.foo) can now be resolved and inlined by looking up the next property in the CJS module's exports.

Test plan

  • Extended esm_import_cjs_with_esmodule_flag fixture with ns.default.foo, ns.foo, named imports, and default imports
  • Added esm_import_cjs_with_esmodule_flag_non_const fixture for non-constant exports to verify the non-inlined path
  • All existing cjs_compat and inline_const tests pass

h-a-n-a commented Apr 1, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds CJS-interop support so that when .default represents module.exports (not exports.default), subsequent property accesses (e.g. ns.default.foo) can be resolved to the corresponding CJS export and eligible for inlineConst.

Changes:

  • Extend link-stage member-expression resolution to treat *.default.<prop> as a CJS export access when .default is module.exports.
  • Expand the existing esm_import_cjs_with_esmodule_flag fixture to cover ns.default.foo, ns.foo, and various import forms.
  • Add a new esm_import_cjs_with_esmodule_flag_non_const fixture to ensure non-constant CJS exports don’t get inlined.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Adds special-case resolution for .default-as-module.exports to enable resolving/inlining the next property as a CJS export.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag/main.js Extends assertions to cover default/named imports and .default.foo access behavior.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag/cjs.cjs Adds exports.foo to provide an additional CJS export for resolution/inlining.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag/cjs2.cjs Adds a module.exports = ... case to test .default behavior when module.exports is non-object.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag/artifacts.snap Updates snapshot to reflect new imports and inlining behavior.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/_config.json New fixture config enabling optimization.inlineConst.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/package.json New fixture set to Node ESM mode (\"type\": \"module\").
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/main.js New assertions ensuring non-constant exports aren’t inlined.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/cjs.cjs New non-constant exports.default via JSON.parse to test non-inlined path.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/cjs2.cjs New non-constant module.exports via JSON.parse to test non-inlined path.
crates/rolldown/tests/rolldown/cjs_compat/esm_import_cjs_with_esmodule_flag_non_const/artifacts.snap New snapshot for the non-const fixture.
Comments suppressed due to low confidence (1)

crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs:626

  • When resolving ns.default.<prop> where .default is treated as module.exports, the code still unconditionally depended_refs.push(m.symbol_ref) (and marks it written on assignment). Here m.symbol_ref is the CJS export for the current property (default), but that export is not actually what gets read/written in the module.exports case. This can force-include exports.default statements and/or incorrectly disable const inlining for default due to unrelated writes like ns.default.foo = .... Consider only adding m.symbol_ref to depended_refs/written_cjs_exports when that symbol is the actual resolution target; for the module.exports path, depend on the resolved next property (or namespace) instead.
                      target_commonjs_exported_symbol = Some((m.symbol_ref, is_default));
                    }
                    depended_refs.push(m.symbol_ref);
                    // If this member expression is a write (e.g. `cjs.c = 'abcd'`), the
                    // CJS exported symbol should not be inlined as a constant since its
                    // value may change at runtime.
                    if member_expr_ref.is_write {
                      written_cjs_exports.push(m.symbol_ref);
                    }

Comment thread crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Outdated
@codspeed-hq

codspeed-hq Bot commented Apr 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports (eb1ab09) with main (bdf2fcf)2

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.

  2. No successful run was found on main (eb1ab09) during the generation of this report, so bdf2fcf was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@h-a-n-a h-a-n-a force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch from b69cbfd to fb1f2f0 Compare April 1, 2026 04:13
@h-a-n-a h-a-n-a force-pushed the 04-01-fix_correct_inlining_based_on_module_s_def_format_and_esmodule_flag branch from 809c72c to 1c6712f Compare April 1, 2026 04:13
@h-a-n-a h-a-n-a force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch 2 times, most recently from 6bb9820 to d6eb912 Compare April 1, 2026 04:27
@graphite-app graphite-app Bot changed the base branch from 04-01-fix_correct_inlining_based_on_module_s_def_format_and_esmodule_flag to graphite-base/8976 April 1, 2026 05:35
@graphite-app graphite-app Bot force-pushed the graphite-base/8976 branch from 1c6712f to 76ce0cf Compare April 1, 2026 05:40
@graphite-app graphite-app Bot force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch from d6eb912 to cb7e7ad Compare April 1, 2026 05:40
@graphite-app graphite-app Bot changed the base branch from graphite-base/8976 to main April 1, 2026 05:40
@graphite-app graphite-app Bot force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch from cb7e7ad to f1e6e0d Compare April 1, 2026 05:40
@netlify

netlify Bot commented Apr 1, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit eb1ab09
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69ce0fc178803d0008c4c99d
😎 Deploy Preview https://deploy-preview-8976--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.

Comment thread crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Outdated
@h-a-n-a h-a-n-a force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch 2 times, most recently from 88bfdbd to e033274 Compare April 2, 2026 06:37
@graphite-app

graphite-app Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Merge activity

…orts (#8976)

## Summary

When `.default` represents `module.exports`, properties accessed through it (e.g. `ns.default.foo`) can now be resolved and inlined by looking up the next property in the CJS module's exports.

## Test plan
- Extended `esm_import_cjs_with_esmodule_flag` fixture with `ns.default.foo`, `ns.foo`, named imports, and default imports
- Added `esm_import_cjs_with_esmodule_flag_non_const` fixture for non-constant exports to verify the non-inlined path
- All existing `cjs_compat` and `inline_const` tests pass
@graphite-app graphite-app Bot force-pushed the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch from e033274 to eb1ab09 Compare April 2, 2026 06:42
@graphite-app graphite-app Bot merged commit eb1ab09 into main Apr 2, 2026
31 checks passed
@graphite-app graphite-app Bot deleted the 04-01-feat_support_inlineconst_for_cjs_exports_accessed_through_module.exports branch April 2, 2026 06:46
This was referenced Apr 8, 2026
shulaoda added a commit that referenced this pull request Apr 8, 2026
## [1.0.0-rc.14] - 2026-04-08

### 🚀 Features

- rust: add `disable_panic_hook` feature to disable the panic hook (#9023) by @sapphi-red
- support inlineConst for CJS exports accessed through module.exports (#8976) by @h-a-n-a

### 🐛 Bug Fixes

- rolldown_plugin_vite_import_glob: normalize resolved alias path to prevent double slashes (#9032) by @shulaoda
- rolldown_plugin_vite_import_glob: follow symlinks in file scanning (#9000) by @Copilot
- wrap CJS entry modules for IIFE/UMD when using exports/module (#8999) by @IWANABETHATGUY
- emit separate __toESM bindings for mixed ESM/CJS external imports (#8987) by @IWANABETHATGUY
- tree-shake dead dynamic imports to side-effect-free CJS modules (#8529) by @sapphi-red
- skip inlining stale CJS export constants on module.exports reassignment (#8990) by @IWANABETHATGUY

### 🚜 Refactor

- generator: migrate ecma formatting from npx oxfmt to vp fmt (#9022) by @shulaoda
- generator: replace npx oxfmt with vp fmt for ecma formatting (#9021) by @shulaoda

### 📚 Documentation

- contrib-guide: mention that running tests on older Node.js version will have different stat results (#8996) by @claude

### ⚙️ Miscellaneous Tasks

- deps: update npm packages (#9002) by @renovate[bot]
- deps: update dependency @napi-rs/cli to v3.6.1 (#9034) by @renovate[bot]
- deps: upgrade oxc to 0.124.0 (#9018) by @shulaoda
- deps: update test262 submodule for tests (#9010) by @sapphi-red
- deps: update dependency oxfmt to ^0.44.0 (#9012) by @renovate[bot]
- deps: update dependency vite to v8.0.5 [security] (#9009) by @renovate[bot]
- deps: update dependency vite-plus to v0.1.16 (#9008) by @renovate[bot]
- deps: update rust crates (#9003) by @renovate[bot]
- deps: update github-actions (#9004) by @renovate[bot]
- deps: update dependency lodash-es to v4.18.1 [security] (#8992) by @renovate[bot]
- deps: update crate-ci/typos action to v1.45.0 (#8988) by @renovate[bot]
- upgrade oxc npm packages to 0.123.0 (#8985) by @shulaoda

### ◀️ Revert

- "chore(deps): update dependency oxfmt to ^0.44.0 (#9012)" (#9019) by @shulaoda

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

3 participants