fix(dev): apply __toCommonJS interop when CJS requires ESM in HMR finalizer by h-a-n-a · Pull Request #9261 · rolldown/rolldown

h-a-n-a · 2026-04-29T08:52:01Z

Summary

The HMR finalizer's try_rewrite_require had a // TODO: handle esm importee and emitted
loadExports(id) for both CJS and ESM importees, skipping the CJS-requires-ESM interop the scope
finalizer already does. Two symptoms:

require('./foo.json') returned { default: [...] } instead of the value, breaking htmlTags.map is not a function in lazy compilation in is-html package.
require('./foo.mjs') returned the raw namespace without __esModule: true, so a CJS bridge
re-exporting ESM mis-wrapped through downstream __toESM.

Test plan

New fixtures hmr/cjs_requires_esm_interop and hmr/cjs_bridge_esm_default — pin the rewrite
shape via snapshot and verify runtime via __esModule / .default assertions.
Confirmed both fail without the fix and pass with it. Full HMR + lazy suite (32 tests) green.

h-a-n-a · 2026-04-29T08:52:10Z

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.

codspeed-hq · 2026-04-29T09:11:50Z

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks¹

_{Comparing 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer (8db1bb3) with main (92c9230)}

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. ↩

Copilot

Pull request overview

This PR fixes CommonJS→ESM interop in the dev-mode HMR AST finalizer by aligning try_rewrite_require with the main (scope-hoisting) finalizer’s behavior when a CJS require() targets ESM (including JSON).

Changes:

Update HmrAstFinalizer::try_rewrite_require to emit __toCommonJS(loadExports(id)) for ESM require targets, and .default for JSON ESM targets.
Add new HMR fixtures to lock in the generated patch rewrite shape via snapshots.
Add runtime assertions ensuring CJS-bridge re-exports preserve __esModule / .default semantics across HMR patch application.

Reviewed changes

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

Show a summary per file

File	Description
crates/rolldown/src/hmr/hmr_ast_finalizer.rs	Implements CJS-requires-ESM (and JSON) interop wrapping in HMR `require()` rewriting.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/_config.json	Adds a devMode HMR fixture config for CJS-requires-ESM (+ JSON) coverage.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/main.js	Fixture entry that accepts the shim boundary and asserts the patch executed.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/shim.js	Fixture module exercising `require()` of JSON + ESM and asserting interop shape.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/shim.hmr-0.js	Patch-step variant used to generate an HMR update and verify runtime behavior.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/esm.js	ESM importee for the CJS `require()` interop assertions.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/data.json	JSON importee used to snapshot the JSON-specific rewrite shape.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/artifacts.snap	Snapshot verifying initial bundle output + HMR step output rewrite shapes.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/_config.json	Adds a devMode HMR fixture config for CJS-bridge→ESM-default coverage.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/main.js	Fixture verifying ESM default import stays correct after HMR patch via `__esModule`.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/bridge.js	CJS bridge that re-exports an ESM module via `require()`.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/bridge.hmr-0.js	Patch-step version to force HMR patch generation for the bridge module.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/esm.js	ESM module providing a default export consumed through the CJS bridge.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/artifacts.snap	Snapshot verifying the initial bundle + HMR patch rewrite uses `__toCommonJS` as intended.

netlify · 2026-04-29T15:54:26Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`fb0d3b6`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/69f313d4dc473c00080b0bb8

IWANABETHATGUY · 2026-04-29T15:59:28Z

I saw you requested a review, but you marked the PR as a draft. Could you mark it ready for review when you think it is ready to review?

Copilot

Pull request overview

Updates the dev-mode HMR AST finalizer so require(...) calls in HMR patches apply the same CJS→ESM interop behavior as the regular (non-HMR) finalizer, fixing incorrect runtime shapes when CJS requires ESM (including JSON/__esModule cases).

Changes:

Adjust try_rewrite_require in the HMR AST finalizer to wrap loadExports(id) with __toCommonJS(...) (and JSON .default unwrapping where applicable), with a heuristic for lazy-export/CJS-shaped importees.
Add new HMR integration fixtures + snapshots to assert correct runtime behavior for CJS requiring ESM and for a CJS bridge module re-exporting ESM default.
Add snapshot coverage showing the exact rewritten patch code shape.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
crates/rolldown/src/hmr/hmr_ast_finalizer.rs	Implements HMR-side `require(...)` rewrite interop via `__toCommonJS` / JSON handling.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/_config.json	Enables devMode for the new HMR interop fixture.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/main.js	Sets up an HMR boundary and asserts the patch executed.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/shim.js	Exercises CJS `require` of JSON + ESM from an HMR-updated module.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/shim.hmr-0.js	Patch-step assertions for JSON value shape + `__esModule`/default interop.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/esm.js	ESM importee used by the fixture.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/data.json	JSON importee used by the fixture.
crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/artifacts.snap	Snapshot pinning rewritten output (initial bundle + HMR patch).
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/_config.json	Enables devMode for the CJS-bridge→ESM-default HMR fixture.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/main.js	Verifies initial + patched behavior when importing a CJS bridge that re-exports ESM.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/bridge.js	CJS bridge module under test.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/bridge.hmr-0.js	Patch payload for the bridge module.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/esm.js	ESM default-only module used by the bridge.
crates/rolldown/tests/rolldown/topics/hmr/cjs_bridge_esm_default/artifacts.snap	Snapshot pinning rewritten output (initial bundle + HMR patch).

h-a-n-a · 2026-04-29T16:10:51Z

I saw you requested a review, but you marked the PR as a draft. Could you mark it ready for review when you think it is ready to review?

I realized that I missed out a case. I will mark it ready when it's ready.

h-a-n-a · 2026-04-30T08:32:54Z

Merge activity

Apr 30, 8:32 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.
Apr 30, 8:32 AM UTC: h-a-n-a added this pull request to the Graphite merge queue.
Apr 30, 8:37 AM UTC: Merged by the Graphite merge queue.

…alizer (#9261) ## Summary The HMR finalizer's `try_rewrite_require` had a `// TODO: handle esm importee` and emitted `loadExports(id)` for both CJS and ESM importees, skipping the CJS-requires-ESM interop the scope finalizer already does. Two symptoms: 1. `require('./foo.json')` returned `{ default: [...] }` instead of the value, breaking `htmlTags.map is not a function` in lazy compilation in `is-html` package. 2. `require('./foo.mjs')` returned the raw namespace without `__esModule: true`, so a CJS bridge re-exporting ESM mis-wrapped through downstream `__toESM`. ## Test plan - [x] New fixtures `hmr/cjs_requires_esm_interop` and `hmr/cjs_bridge_esm_default` — pin the rewrite shape via snapshot and verify runtime via `__esModule` / `.default` assertions. - [x] Confirmed both fail without the fix and pass with it. Full HMR + lazy suite (32 tests) green.

h-a-n-a · 2026-04-30T08:38:48Z

This only resolved the case where lazy-export modules (JSON files, for example) are defined in the first run. If these modules are newly imported from the HMR patch. It's very likely to fail.

@h-a-n-a

## [1.0.0] - 2026-05-07 ### 🐛 Bug Fixes - dev/lazy: lazily compiled modules should be watched (#9301) by @h-a-n-a - implement dynamic dominator merge logic (#9270) by @TheAlexLichter - dev: apply __toCommonJS interop when CJS requires ESM in HMR finalizer (#9261) by @h-a-n-a ### 🚜 Refactor - ecma_ast: tighten allocator access to enforce Sync invariant (#9278) by @IWANABETHATGUY - scan_stage: remove stmt_infos field from EcmaView (#9276) by @IWANABETHATGUY - link_stage: detach stmt_infos from EcmaView (#9274) by @IWANABETHATGUY - link_stage: detach depended_runtime_helper from EcmaView to remove unsafe (#9265) by @IWANABETHATGUY - link_stage: remove unsafe in determine_module_exports_kind (#9253) by @IWANABETHATGUY ### 📚 Documentation - getting-started: remove RC warning for 1.0.0 release (#9310) by @shulaoda - getting-started: update version references for 1.0.0 release (#9309) by @shulaoda - add Vite+ tab to getting-started snippets (#9285) by @shulaoda - lazy-barrel: clarify own-exports behavior for import-then-export records (#9298) by @shulaoda - restructure top navigation around Learn vs Reference (#9284) by @shulaoda - builtin-plugins: add bundle analyzer plugin docs (#9292) by @shulaoda - design doc for reference_needed_symbols (#9264) by @IWANABETHATGUY ### ⚡ Performance - devtools: write logs on a background thread (#9219) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - mark esbuild/ts/parameter_props_use_define_for_class_fields_true as passed (#9308) by @sapphi-red - deps: upgrade oxc to 0.129.0 (#9297) by @shulaoda - deps: update rollup submodule for tests to v4.60.3 (#9294) by @sapphi-red - deps: update test262 submodule for tests (#9295) by @sapphi-red - ai: add rolldown REPL decode skill (#9245) by @Dunqing

@h-a-n-a

## [1.0.0] - 2026-05-07 ### 🐛 Bug Fixes - dev/lazy: lazily compiled modules should be watched (rolldown#9301) by @h-a-n-a - implement dynamic dominator merge logic (rolldown#9270) by @TheAlexLichter - dev: apply __toCommonJS interop when CJS requires ESM in HMR finalizer (rolldown#9261) by @h-a-n-a ### 🚜 Refactor - ecma_ast: tighten allocator access to enforce Sync invariant (rolldown#9278) by @IWANABETHATGUY - scan_stage: remove stmt_infos field from EcmaView (rolldown#9276) by @IWANABETHATGUY - link_stage: detach stmt_infos from EcmaView (rolldown#9274) by @IWANABETHATGUY - link_stage: detach depended_runtime_helper from EcmaView to remove unsafe (rolldown#9265) by @IWANABETHATGUY - link_stage: remove unsafe in determine_module_exports_kind (rolldown#9253) by @IWANABETHATGUY ### 📚 Documentation - getting-started: remove RC warning for 1.0.0 release (rolldown#9310) by @shulaoda - getting-started: update version references for 1.0.0 release (rolldown#9309) by @shulaoda - add Vite+ tab to getting-started snippets (rolldown#9285) by @shulaoda - lazy-barrel: clarify own-exports behavior for import-then-export records (rolldown#9298) by @shulaoda - restructure top navigation around Learn vs Reference (rolldown#9284) by @shulaoda - builtin-plugins: add bundle analyzer plugin docs (rolldown#9292) by @shulaoda - design doc for reference_needed_symbols (rolldown#9264) by @IWANABETHATGUY ### ⚡ Performance - devtools: write logs on a background thread (rolldown#9219) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - mark esbuild/ts/parameter_props_use_define_for_class_fields_true as passed (rolldown#9308) by @sapphi-red - deps: upgrade oxc to 0.129.0 (rolldown#9297) by @shulaoda - deps: update rollup submodule for tests to v4.60.3 (rolldown#9294) by @sapphi-red - deps: update test262 submodule for tests (rolldown#9295) by @sapphi-red - ai: add rolldown REPL decode skill (rolldown#9245) by @Dunqing Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>

github-actions Bot assigned h-a-n-a Apr 29, 2026

h-a-n-a changed the base branch from 04-28-fix_dev_lazy_fix_exports_of_lazy_requests_in_lazy_chunks to graphite-base/9261 April 29, 2026 08:54

h-a-n-a force-pushed the graphite-base/9261 branch from 739a535 to e7ee6d7 Compare April 29, 2026 08:55

h-a-n-a force-pushed the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch from 4116a1a to 7a8b095 Compare April 29, 2026 08:55

h-a-n-a changed the base branch from graphite-base/9261 to main April 29, 2026 08:55

h-a-n-a marked this pull request as ready for review April 29, 2026 09:05

h-a-n-a requested review from IWANABETHATGUY, Copilot and shulaoda April 29, 2026 09:05

Copilot started reviewing on behalf of h-a-n-a April 29, 2026 09:06 View session

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/rolldown/src/hmr/hmr_ast_finalizer.rs Outdated

h-a-n-a marked this pull request as draft April 29, 2026 09:36

h-a-n-a force-pushed the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch from 7a8b095 to 9308578 Compare April 29, 2026 15:53

h-a-n-a requested a review from Copilot April 29, 2026 15:56

Copilot AI reviewed Apr 29, 2026

View reviewed changes

Comment thread crates/rolldown/src/hmr/hmr_ast_finalizer.rs Outdated

Comment thread crates/rolldown/tests/rolldown/topics/hmr/cjs_requires_esm_interop/shim.js Outdated

Comment thread crates/rolldown/src/hmr/hmr_ast_finalizer.rs

h-a-n-a force-pushed the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch from 9308578 to 90e83d2 Compare April 29, 2026 16:09

h-a-n-a force-pushed the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch from 90e83d2 to 8db1bb3 Compare April 29, 2026 16:18

h-a-n-a marked this pull request as ready for review April 29, 2026 16:31

shulaoda approved these changes Apr 30, 2026

View reviewed changes

IWANABETHATGUY approved these changes Apr 30, 2026

View reviewed changes

h-a-n-a mentioned this pull request Apr 30, 2026

wip: disable node_modules watch #9275

Closed

h-a-n-a added the graphite: merge-when-ready label Apr 30, 2026

graphite-app Bot force-pushed the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch from 8db1bb3 to fb0d3b6 Compare April 30, 2026 08:33

graphite-app Bot merged commit fb0d3b6 into main Apr 30, 2026
33 checks passed

graphite-app Bot deleted the 04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer branch April 30, 2026 08:37

graphite-app Bot removed the graphite: merge-when-ready label Apr 30, 2026

This was referenced May 6, 2026

release: v1.0.0-rc.19 #9300

Closed

release: v1.0.0 #9311

Merged

shulaoda mentioned this pull request May 7, 2026

release: v1.0.0 #9314

Merged

Kyujenius mentioned this pull request May 20, 2026

fix(finalizer): unwrap oxc-runtime helper namespace on CJS require #9472

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dev): apply __toCommonJS interop when CJS requires ESM in HMR finalizer#9261

fix(dev): apply __toCommonJS interop when CJS requires ESM in HMR finalizer#9261
graphite-app[bot] merged 1 commit into
mainfrom
04-29-fix_dev_apply___tocommonjs_interop_when_cjs_requires_esm_in_hmr_finalizer

h-a-n-a commented Apr 29, 2026 •

edited

Loading

Uh oh!

h-a-n-a commented Apr 29, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

netlify Bot commented Apr 29, 2026 •

edited

Loading

Uh oh!

IWANABETHATGUY commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

h-a-n-a commented Apr 30, 2026 •

edited by graphite-app Bot

Loading

Uh oh!

Uh oh!

h-a-n-a commented Apr 30, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

h-a-n-a commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

h-a-n-a commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

netlify Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs canceled.

Uh oh!

IWANABETHATGUY commented Apr 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

h-a-n-a commented Apr 30, 2026 • edited by graphite-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

h-a-n-a commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

h-a-n-a commented Apr 29, 2026 •

edited

Loading

h-a-n-a commented Apr 29, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 29, 2026 •

edited

Loading

netlify Bot commented Apr 29, 2026 •

edited

Loading

h-a-n-a commented Apr 30, 2026 •

edited by graphite-app Bot

Loading

h-a-n-a commented Apr 30, 2026 •

edited

Loading