Bug: Cannot read properties of null (reading 'moduleSideEffects')

[ptrxyz]

• Rollup Plugin Name: commonjs
• Rollup Plugin Version: 28.0.7
• Rollup Version: 4.52.5
• Operating System (or Browser): Linux (Arch btw)
• Node Version: Bun 1.3.0; also present on Node v24.9.0
• Link to reproduction (⚠️ read below): https://github.com/ptrxyz/rollup-bug-report

Expected Behavior

The link provided points to a minimal example, do this to reproduce:

• checkout the repo, npm install, npm build
• this should not fail

Actual Behavior

• i get the following error:

> Using @sveltejs/adapter-node
error during build:
TypeError: Cannot read properties of null (reading 'moduleSideEffects')
    at file:///tmp/test/node_modules/@rollup/plugin-commonjs/dist/es/index.js:854:77
    at Array.map (<anonymous>)
    at file:///tmp/test/node_modules/@rollup/plugin-commonjs/dist/es/index.js:833:31
    at async rewriteRequireExpressionsAndGetImportBlock (file:///tmp/test/node_modules/@rollup/plugin-commonjs/dist/es/index.js:1397:28)
    at async transformCommonjs (file:///tmp/test/node_modules/@rollup/plugin-commonjs/dist/es/index.js:1986:23)
    at async transform (file:///tmp/test/node_modules/rollup/dist/es/shared/node-entry.js:21139:16)
    at async ModuleLoader.addModuleSource (file:///tmp/test/node_modules/rollup/dist/es/shared/node-entry.js:21352:36)

Additional Information

The repo is a minimal SvelteKit application created with the sv create tool with a single added dependency that demonstrates the problem by simply importing it: (@paralleldrive/cuid): https://github.com/ptrxyz/rollup-bug-report/blob/72cf37c9bd17c5c1541da2aae2ba9faf1bf36e0b/src/routes/%2Bpage.server.ts#L2

The issue seems that CUID2 imports @noble/hashes as a peer dependency, specifically node_modules/@noble/hashes/cryptoNode.js. There you will find the offending import in line 12:

// node_modules/@noble/hashes/cryptoNode.js
...
const nc = require("node:crypto");
...

This require seems to break with RollUp since version >4.48 with the introduction of https://github.com/rollup/plugins/blob/1935e9e3e2fa1e68b25f5d16fd6007cf9fe205fb/packages/commonjs/src/resolve-require-sources.js#L196C11-L208C12

Note: I was not able to reproduce this bug when directly importing @noble/hashes into my app, so i suppose it has to do with the CUID2 detour?

[ptrxyz]

this might be related: #1912

[charliecreates]

Expand this to see my work.

• Reproduced the failure from ptrxyz/rollup-bug-report with plugin 28.0.7/Rollup 4.52.5; npm run build crashed in dist/es/index.js at rewriteRequireExpressionsAndGetImportBlock → transformCommonjs.
• Mapped the stack to source and identified the unsafe getModuleInfo(id).moduleSideEffects access when the dependency is external/unresolved (getModuleInfo(...) returns null).
• Confirmed via packages/commonjs/src/proxies.js and the getEsImportProxy(...) path that defaulting the side-effect signal is safe; verified the repro’s @paralleldrive/cuid pulls node:crypto, making the id external and explaining the null info.
• Implemented a null‑safe read of moduleInfo?.moduleSideEffects ?? true in packages/commonjs/src/resolve-require-sources.js, preserving Rollup’s tri‑state (true | false | 'no-treeshake') and avoiding the null access.
• Reviewed fixtures/snapshots in packages/commonjs/test to ensure no tests relied on non‑null getModuleInfo(...) for externals; purity of getDefaultExportFromCjs(...) remains intact.
• Re‑verified the reproduction: with the patched plugin, npm run build succeeds on the SvelteKit app; no TypeError, tree‑shaking of __require() unchanged.
• Ran pnpm -F @rollup/plugin-commonjs ci:test: exit 0, dist/cjs and dist/es built, no snapshot changes; pre‑commit lint passed cleanly.
• Validated the diff scope: only packages/commonjs/src/resolve-require-sources.js changed (+19/-6).
• Committed the fix as 8f298e1 with message fix(commonjs): avoid null access to moduleSideEffects for wrapped externals.
• Pushed branch ai-1913-bug-cannot-read-properties-of-null-reading; PR can be opened via new PR link.

[charliecreates]

Thanks for the clear repro — I reproduced the crash locally and tracked it down to how commonjs handles wrapped externals for node: built-ins.

Root cause

• When a wrapped CJS module requires a node: builtin (e.g., node:crypto), the transform generates an external proxy module (?commonjs-external). While building the import block, we were doing getModuleInfo(dependencyId).moduleSideEffects for that proxy. For externals, getModuleInfo(...) returns null at that point, so reading .moduleSideEffects threw.

Fix

• Guard getModuleInfo(...) for wrapped externals and default to “has side effects” when info is unavailable (we don’t add a /* @PURE */ annotation). This avoids the null access and is the conservative/safe choice.
• Preserve Rollup’s tri-state moduleSideEffects semantics (true | false | 'no-treeshake') whenever available.
• Keep per-dependency metadata in sync when we force wrapped CJS for node:* externals under strictRequires.

PR: #1914 (fix(commonjs): guard moduleSideEffects for wrapped externals)

Changes

• packages/commonjs/src/resolve-require-sources.js
  • Avoid null access to ModuleInfo for wrapped externals.
  • Skip getModuleInfo entirely for any ?commonjs-external id (it would be null anyway).
  • Default side-effects to true when info is unavailable; preserve Rollup’s tri-state when present.
  • Sync parentMeta.isRequiredCommonJS[dependencyId] when overriding isCommonJS for node:*.

Workarounds until release

• Mark the builtin as external in your app config, e.g. external: ['node:crypto'].
• Or temporarily pin @rollup/plugin-commonjs to a prior version that didn’t hit this path.

If you’re up for it, you can confirm the fix with the PR branch in your repro — but the included tests already cover strict-requires + external node: builtins.

Verification

# Scope: @rollup/plugin-commonjs
# Build
$ pnpm -F @rollup/plugin-commonjs build
# => success

# Tests (AVA)
$ pnpm -F @rollup/plugin-commonjs test -- --verbose
# => 280 tests passed (includes strict-requires-external-node-builtin)

# Type tests
$ pnpm -F @rollup/plugin-commonjs test:ts
# => success

Closes #1913.