Skip to content

fix(commonjs): pass exports to the wrapper when only __esModule is defined on it - #2046

Open
ItsRenjiii wants to merge 1 commit into
rollup:masterfrom
ItsRenjiii:fix/commonjs-wrapped-esmodule-exports
Open

ItsRenjiii wants to merge 1 commit into
rollup:masterfrom
ItsRenjiii:fix/commonjs-wrapped-esmodule-exports

Conversation

@ItsRenjiii

Copy link
Copy Markdown

Rollup Plugin Name: commonjs

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Reported in discussion #2044 (no issue filed).

Description

A top-level Object.defineProperty(exports, '__esModule', { value: true }) is recorded in topLevelDefineCompiledEsmExpressions, and its exports argument goes into skippedNodes, so it never sets uses.exports. When the module is wrapped for another reason (here, a module reference), wrapCode only passes the names marked as used. The defineProperty call is kept as written, so the wrapper ends up referencing a free exports:

(function (module) {
	Object.defineProperty(exports, "__esModule", { value: true });
	console.log('object');
} ());

In ES output this throws ReferenceError: exports is not defined in ES module scope. In CJS output it doesn't throw, because exports resolves to the output file's own binding, but __esModule never reaches the module's exports. The new test checks for exactly that.

This PR marks uses.exports (or uses.module, for Object.defineProperty(module.exports, ...)) for those calls whenever the module is wrapped. The wrapper then receives what the kept calls reference:

(function (module, exports) {
	Object.defineProperty(exports, "__esModule", { value: true });
	console.log('object');
} (m, m.exports));

Modules that aren't wrapped are unchanged.

Tests

  • New function fixture wrapped-define-compiled-esm. It fails on master because the dependency's exports lack __esModule, and passes with this change.
  • The reporter's exact build (format: 'es') throws the ReferenceError on master and prints object with this change.
  • The only snapshot change is the new fixture's entry. No existing output changes.
  • The steps from validate.yml all pass locally: prettier:check, pnpm --recursive build, lint:js and ci:coverage for ...[origin/master]. The commonjs suite (292 tests) passes on Node 20 and 24.

This is independent of #2045, which changes a different part of transform-commonjs.js.

🤖 Generated with Claude Code

…fined on it

A top-level `Object.defineProperty(exports, '__esModule', ...)` is skipped
during analysis, so it did not mark `exports` as used. When the module was
wrapped for another reason (such as a `module` reference), the wrapper only
received `module`, and the kept `defineProperty` call referenced a free
`exports`. In ES output this throws `ReferenceError: exports is not defined`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Reviews (1) · Last reviewed commit: "fix(commonjs): pass exports to the wrapp..." · Reviewed by Greptile

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.

1 participant