Skip to content

fix(commonjs): unwrap require() of external CommonJS modules on Node >= 23 - #2027

Open
s1gr1d wants to merge 4 commits into
rollup:masterfrom
s1gr1d:patch-1
Open

s1gr1d wants to merge 4 commits into
rollup:masterfrom
s1gr1d:patch-1

Conversation

@s1gr1d

@s1gr1d s1gr1d commented Sep 29, 2026 •

Copy link
Copy Markdown

Rollup Plugin Name: {name}

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

Description

On Node 23+, requireReturnsDefault: 'auto' hands bundled CommonJS code a namespace object where plain Node would return module.exports.

The runtime helper getDefaultExportFromNamespaceIfNotNamed unwraps a required external module only when default is the sole key on its namespace. Since nodejs/node#53848 (Node 23), a CommonJS namespace always carries a second key, 'module.exports':

​js Object.keys(await import('mquery')); // Node 22: ['default'] // Node 23+: ['default', 'module.exports'] ​

So the check never passes, the requiring code receives the namespace, and calls like new mquery() throw mquery is not a constructor. Nuxt and SolidStart apps hit this through Nitro, which bundles database drivers while keeping the drivers' dependencies external (see getsentry/sentry-javascript#24775 for a full analysis).

The fix: when the namespace has a 'module.exports' key, return its value. Node puts the real module.exports behind that key, so this is exactly what require() would return, with no guessing. Namespaces without the key keep the current behavior, so nothing changes on Node 22 or for genuine ES modules.

@s1gr1d
s1gr1d requested a review from shellscape as a code owner September 29, 2026 15:06
@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Fixes CommonJS module unwrapping for Node 23 compatibility.

The PR appears safe to merge on the findings eligible for this review.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[External require in auto mode] --> B[Import namespace]
  B --> C{module.exports and default have the same value?}
  C -->|Yes, including NaN| D[Return module.exports]
  C -->|No| E{Default is the only key?}
  E -->|Yes| F[Return default]
  E -->|No| G[Return namespace]
Loading

Reviews (3) · Last reviewed commit: "address comment"

Comment thread packages/commonjs/src/helpers.js Outdated
Comment thread packages/commonjs/src/helpers.js Outdated
Comment thread packages/commonjs/src/helpers.js Outdated
Comment thread packages/commonjs/src/helpers.js Outdated
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