fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743

IWANABETHATGUY · 2025-10-29T07:58:28Z

Current issue:
https://repl.rolldown.rs/#eNplUctOwzAQ/JWVT6mIXHHEqEeOiANHzMFNNsGVY6d+FKQq/87aTlFET/a+Zmdmr2xg4sompS0/hfy1TPyFLeso0tPsfITJ9TB4N4FkfO+TQcmepV2LKgSkZ61b16OoqdIkbedsiDCbNGoLB7hKCxmwlXahem3lIXrdxZdzUqaprZx6+IRRtfD4tKNO/CnrehxUMjdAyhNVZCL6hEvLjD5uxKxR1UJ4xJxXmJCZ5P2MhrKizdQtrGOVfnbgAB7PSXtssg2ELVnmJe09cta438PH2/GEXQRXns+inBQJ8qm6yA3aMX7BA7yTAXZsCGq3JskggM6jiihgSLaL2llodhWdSkTM0VrjRiJknOqxhwqbaQEsq8VFojOmd9+W09Sgx63a+0oV/s/vstWlOKcobhQG5ydFoWTdKUhWGAPMHummF3wttgQB+TaFEPFhC7G55NuQrhDZ8gst29p6

The binding finalization only considers when the required CJS module is in a common chunk,
so it always converts

const mod = require('./lib')

to

const mod = require_rule$1.require_rule()

This did not consider if the wrap_ref is exported via entry cjs module, which may generate

Object.defineProperty(exports, 'default', {
  enumerable: true,
  get: function () {
    return require_rule();
  }
});

this pr fixed this scenario.

IWANABETHATGUY · 2025-10-29T07:58:45Z

refactor: pre compute exports #6755
fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add the label graphite: merge 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.

netlify · 2025-10-29T07:59:04Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`34ea4af`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/69023b240172b80008cfc5d9

github-actions · 2025-10-29T08:13:41Z

Benchmarks Rust

target: main(1feacc0)
pr: 10-29-fix_5930(34ea4af)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.03     63.9±1.03ms        ? ?/sec    1.00     62.2±0.95ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.03     70.7±1.15ms        ? ?/sec    1.00     68.8±1.20ms        ? ?/sec
bundle/bundle@rome_ts                                        1.01    108.0±2.17ms        ? ?/sec    1.00    106.6±1.30ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    121.5±1.34ms        ? ?/sec    1.00    120.0±2.06ms        ? ?/sec
bundle/bundle@threejs                                        1.02     39.3±2.27ms        ? ?/sec    1.00     38.4±0.53ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     42.6±0.48ms        ? ?/sec    1.00     42.1±0.56ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    393.8±3.72ms        ? ?/sec    1.00    392.0±6.15ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    455.7±3.33ms        ? ?/sec    1.00    453.7±4.18ms        ? ?/sec
scan/scan@rome_ts                                            1.00     85.5±1.33ms        ? ?/sec    1.02     86.9±1.37ms        ? ?/sec
scan/scan@threejs                                            1.01     29.3±1.94ms        ? ?/sec    1.00     28.9±0.41ms        ? ?/sec
scan/scan@threejs10x                                         1.00    300.8±4.21ms        ? ?/sec    1.01    303.1±3.94ms        ? ?/sec

Copilot

Pull Request Overview

This PR adds support for the preserveModules configuration option and fixes a bug related to handling CJS entry modules in code splitting scenarios. The key issue was that when referencing a CJS entry module from another chunk, the code was incorrectly calling the wrapper function instead of accessing it directly.

Added preserveModules configuration option support across testing infrastructure
Fixed CJS entry module handling by introducing FinalizedExprProcessHint to track when symbols reference CJS entry modules
Added test case for issue #5930 demonstrating the fix with both bundled and preserve-modules variants

Reviewed Changes

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

Show a summary per file

File	Description
crates/rolldown_testing_config/src/config_variant.rs	Added `preserve_modules` field to ConfigVariant struct and implemented serialization/config application
crates/rolldown_testing/_config.schema.json	Added JSON schema definition for `preserveModules` configuration option
crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap	Added snapshot entry for the new test case 5930
crates/rolldown/tests/rolldown/issues/5930/rule.js	Test file: CommonJS module that requires another module and exports metadata
crates/rolldown/tests/rolldown/issues/5930/main.js	Test file: Entry point that imports and validates the CJS module behavior
crates/rolldown/tests/rolldown/issues/5930/lib.js	Test file: Simple CommonJS module exporting an empty object
crates/rolldown/tests/rolldown/issues/5930/artifacts.snap	Snapshot showing expected output for both default and preserve-modules variants
crates/rolldown/tests/rolldown/issues/5930/_config.json	Test configuration specifying CJS format and preserve-modules variant
crates/rolldown/src/module_finalizers/rename.rs	Updated to handle new tuple return type from finalized_expr_for_symbol_ref
crates/rolldown/src/module_finalizers/mod.rs	Core fix: Added FinalizedExprProcessHint to track CJS entry modules and avoid incorrect wrapper calls
crates/rolldown/src/module_finalizers/impl_visit_mut.rs	Updated all call sites to handle new tuple return type from finalized_expr_for_symbol_ref

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

crates/rolldown/src/module_finalizers/mod.rs

graphite-app · 2025-10-29T16:04:25Z

Merge activity

Oct 29, 4:04 PM UTC: IWANABETHATGUY added this pull request to the Graphite merge queue.
Oct 29, 4:19 PM UTC: Merged by the Graphite merge queue.

…ject (#6743) closed #5930 Current issue: https://repl.rolldown.rs/#eNplUctOwzAQ/JWVT6mIXHHEqEeOiANHzMFNNsGVY6d+FKQq/87aTlFET/a+Zmdmr2xg4sompS0/hfy1TPyFLeso0tPsfITJ9TB4N4FkfO+TQcmepV2LKgSkZ61b16OoqdIkbedsiDCbNGoLB7hKCxmwlXahem3lIXrdxZdzUqaprZx6+IRRtfD4tKNO/CnrehxUMjdAyhNVZCL6hEvLjD5uxKxR1UJ4xJxXmJCZ5P2MhrKizdQtrGOVfnbgAB7PSXtssg2ELVnmJe09cta438PH2/GEXQRXns+inBQJ8qm6yA3aMX7BA7yTAXZsCGq3JskggM6jiihgSLaL2llodhWdSkTM0VrjRiJknOqxhwqbaQEsq8VFojOmd9+W09Sgx63a+0oV/s/vstWlOKcobhQG5ydFoWTdKUhWGAPMHummF3wttgQB+TaFEPFhC7G55NuQrhDZ8gst29p6 The binding finalization only considers when the required CJS module is in a common chunk, so it always converts ``` const mod = require('./lib') ``` to ```js const mod = require_rule$1.require_rule() ``` This did not consider if the wrap_ref is exported via entry cjs module, which may generate ``` Object.defineProperty(exports, 'default', { enumerable: true, get: function () { return require_rule(); } }); ``` this pr fixed this scenario.

@sapphi-red

## [1.0.0-beta.46] - 2025-11-03 ### 💥 BREAKING CHANGES - default `preserveEntrySignatures` to `'exports-only'` to align with Rollup (#6723) by @Copilot ### 🚀 Features - link to docs for commonjs prefer_builtin_feature diagnostic (#6793) by @sapphi-red - rolldown_plugin_build_import_analysis: align partial logic (#6789) by @shulaoda - improve `EVAL` warning message (#6776) by @sapphi-red - rolldown: oxc v0.96.0 (#6774) by @Boshen - rolldown_plugin_build_import_analysis: align partial logic (#6773) by @shulaoda - skip `__toESM` when not needed to reduce output size (#6751) by @Copilot - improve `EMPTY_IMPORT_META` warning message (#6761) by @sapphi-red - rolldown_plugin_build_import_analysis: align no preload logic (#6762) by @shulaoda - rolldown_plugin_vite_css_post: align `RemovedPureCSSFilesCache` logic (#6745) by @shulaoda - rolldown_plugin_build_import_analysis: initialize `generateBundle` logic (#6744) by @shulaoda - experimental: introduce `freeExternalMemory` to free external memory immediately (#6721) by @hyf0 ### 🐛 Bug Fixes - plugins: wrap replacePlugin with makeBuiltinPluginCallable (#6782) by @huang-julien - debug: make sure `this.resolve` is also tracked under corresponding session (#6798) by @hyf0 - legal comments above directives are not preserved (#6787) by @shulaoda - panic when rendering exports to other chunks (#6765) by @Copilot - transform/inject: escape special characters in import source (#6778) by @Copilot - handle default exports from CJS modules correctly (#6767) by @IWANABETHATGUY - rolldown_plugin_build_import_analysis: correct aligned logic (#6768) by @shulaoda - duplicate default export when using both import and require (#6764) by @Copilot - handle arbitrary module namespace identifiers in `preserveEntrySignatures: 'allow-extension'` (#6753) by @Copilot - TypeError when loading CJS files after bundling mixed CJS+TS project (#6743) by @IWANABETHATGUY - add warnings for import.meta.dirname/filename/url in UMD and IIFE formats (#6747) by @Copilot - rolldown_plugin_manifest: should keep `names` field (#6742) by @shulaoda - legal comments above import statements are not preserved (#6717) by @shulaoda - remove redundant node check in dynamic entry graph construction (#6730) by @IWANABETHATGUY ### 🚜 Refactor - remove unnecessary `#[napi(gettter)]`, use function call directly (#6694) by @hyf0 - explain the reason if memory doesn't get dropped by `freeExternalMemory` (#6781) by @hyf0 - pre compute exports (#6755) by @IWANABETHATGUY - dev: receive an interface for DevRuntime rather than WebSocket directly (#6734) by @sapphi-red - dev: fix types for the runtime file and expose it properly (#6731) by @sapphi-red ### 📚 Documentation - fix magicstring credit (#6812) by @TheAlexLichter - in-depth/bundling-cjs: wrap some sections with caveats section (#6796) by @sapphi-red - in-depth/bundling-cjs: add "Ambiguous `default` import from CJS modules" section (#6795) by @sapphi-red - in-depth/bundling-cjs: make `esmExternalRequirePlugin` link more prominent (#6792) by @sapphi-red - add explanation about direct eval (#6775) by @sapphi-red - add "Non-ESM Output Formats" page (#6760) by @sapphi-red - use oxc for benchmark comparison in native magic string doc (#6740) by @sapphi-red - polish `cleanDir` option (#6741) by @hyf0 - fix image in native magic string doc (#6739) by @sapphi-red - update description about `attachDebugInfo: 'none'` (#6738) by @IWANABETHATGUY - add warning about cleanDir behavior with multiple configs (#6735) by @Copilot ### 🧪 Testing - skip paths function test (#6771) by @IWANABETHATGUY - rust/dev: add test about recover from initial build error (#6567) by @hyf0 ### ⚙️ Miscellaneous Tasks - correct prepare release binding file (#6817) by @shulaoda - deps: update github-actions (#6806) by @renovate[bot] - deps: lock file maintenance npm packages (#6814) by @renovate[bot] - use correct node version (#6809) by @shulaoda - deps: lock file maintenance rust crates (#6815) by @renovate[bot] - deps: update npm packages (major) (#6813) by @renovate[bot] - deps: update dependency dprint-typescript to v0.95.12 (#6805) by @renovate[bot] - fix typo (#6801) by @iiio2 - fix typo in is_import_expr_ignored_by_comment method name (#6797) by @sapphi-red - deps: update crate-ci/typos action to v1.39.0 (#6794) by @renovate[bot] - deps: upgrade Rust to v1.91.0 and fix new clippy lints (#6785) by @Copilot - allow dprint to format `packages/rolldown/tests` except diagnostics directory (#6772) by @Copilot - deps: update dependency tsdown to v0.15.12 (#6759) by @renovate[bot] - deps: update dependency rolldown-plugin-dts to v0.17.3 (#6758) by @renovate[bot] - test/dev: collect multiple build outputs for each step (#6736) by @hyf0 - rolldown: change the NAPI-RS binding file to the cjs (#6688) by @Brooooooklyn - test: automatically run extra test with `preserve_entry_signatures: 'allow-extension'` (#6727) by @Copilot - deps: update dependency rolldown-plugin-dts to v0.17.2 (#6732) by @renovate[bot] - fix node validation (#6728) by @shulaoda - deps: lock file maintenance rust crates (#6636) by @renovate[bot] - deps: update dependency tsdown to v0.15.11 (#6725) by @renovate[bot] ### ❤️ New Contributors * @huang-julien made their first contribution in [#6782](#6782) Co-authored-by: shulaoda <[email protected]>

github-actions bot assigned IWANABETHATGUY Oct 29, 2025

IWANABETHATGUY force-pushed the 10-29-fix_5930 branch 2 times, most recently from 585d78d to e981bc2 Compare October 29, 2025 14:43

IWANABETHATGUY changed the title ~~fix: 5930~~ fix: TypeError when loading CJS files after bundling mixed CJS+TS project Oct 29, 2025

IWANABETHATGUY marked this pull request as ready for review October 29, 2025 14:45

IWANABETHATGUY force-pushed the 10-29-fix_5930 branch from e981bc2 to 8406bc7 Compare October 29, 2025 14:49

hyf0 requested a review from Copilot October 29, 2025 15:37

Copilot AI reviewed Oct 29, 2025

View reviewed changes

crates/rolldown/src/module_finalizers/mod.rs Show resolved Hide resolved

crates/rolldown/src/module_finalizers/mod.rs Show resolved Hide resolved

hyf0 reviewed Oct 29, 2025

View reviewed changes

crates/rolldown/src/module_finalizers/mod.rs Show resolved Hide resolved

hyf0 approved these changes Oct 29, 2025

View reviewed changes

IWANABETHATGUY mentioned this pull request Oct 29, 2025

refactor: pre compute exports #6755

Merged

graphite-app bot force-pushed the 10-29-fix_5930 branch from 8406bc7 to 34ea4af Compare October 29, 2025 16:04

graphite-app bot merged commit 34ea4af into main Oct 29, 2025
27 checks passed

graphite-app bot deleted the 10-29-fix_5930 branch October 29, 2025 16:19

github-actions bot mentioned this pull request Nov 3, 2025

release: v1.0.0-beta.46 #6818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743

fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743

Uh oh!

IWANABETHATGUY commented Oct 29, 2025 •

edited

Loading

Uh oh!

IWANABETHATGUY commented Oct 29, 2025 •

edited

Loading

Uh oh!

netlify bot commented Oct 29, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 29, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

graphite-app bot commented Oct 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743

fix: TypeError when loading CJS files after bundling mixed CJS+TS project #6743

Uh oh!

Conversation

IWANABETHATGUY commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IWANABETHATGUY commented Oct 29, 2025 • 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!

netlify bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs canceled.

Uh oh!

github-actions bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

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!

graphite-app bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IWANABETHATGUY commented Oct 29, 2025 •

edited

Loading

IWANABETHATGUY commented Oct 29, 2025 •

edited

Loading

netlify bot commented Oct 29, 2025 •

edited

Loading

github-actions bot commented Oct 29, 2025 •

edited

Loading

graphite-app bot commented Oct 29, 2025 •

edited

Loading