[@rollup/plugin-commonjs] `import { createRequire } from 'node:module'` is injected automatically, breaking codes for edge runtimes

[sapphi-red]

• Rollup Plugin Name: commonjs
• Rollup Plugin Version: 28.0.7+
• Rollup Version: 4.52.5
• Operating System (or Browser): Windows, Linux
• Node Version: 22.18.0
• Link to reproduction (⚠️ read below): https://stackblitz.com/edit/rollup-repro-ddvmzobf?file=rollup.config.js

Expected Behavior

import { createRequire } from 'node:module' is not injected unless allowed by an option.

Actual Behavior

import { createRequire } from 'node:module' is injected automatically.

Additional Information

Edge runtimes like workerd and vercel edge runtime does not support module.createRequire (workers docs, vercel edge runtime docs). It isn't possible to polyfill this function as well.
This means the code generated cannot be executed by those runtimes.

related: #1909, vitejs/vite#20983, vitejs/vite-plugin-react#929 (comment), wakujs/waku#1731

[shellscape]

Sounds like we need to introduce an option to disable injecting that.

@CharlieHelps please investigate what steps would be necessary

[charliecreates]

Expand this to see my work.

• Captured the ask from @shellscape’s comment on #1924 (comment) as a configurable flag (inject: false) in rollup.config.js.
• Identified the injection logic in packages/commonjs/src/index.js (transform(...) with helper appends in renderChunk(...)) and confirmed it is the sole injection site.
• Sketched PluginOptions with inject?: boolean (default true) and threaded guards through transform/renderChunk; audited buildStart and resolve-require-sources.js to confirm no other injection paths.
• Drafted README.md docs with plugins: [plugin({ inject: false })] and noted backward compatibility: default inject: true preserves current output.
• Verified independence from strictRequires via packages/commonjs/src/utils.js#getStrictRequiresFilter(...).
• Located the snapshot block module-side-effects-external-node-builtin-wrapped in packages/commonjs/test/snapshots/function.js.md showing the injected require('node:module') and node_module.createRequire(...) lines, matching the behavior we’re gating with inject.
• Reviewed packages/commonjs/test/function.js to confirm the fixtures in packages/commonjs/test/fixtures/function drive snapshot generation and how config (_config.js) flows into plugin options.

[charliecreates]

Thanks, I dug into where that import is coming from and what it would take to make it opt‑out.

Summary

• The import is injected only for wrapped CommonJS modules that require external Node built‑ins (ids starting with node:). Code path:
  • packages/commonjs/src/resolve-require-sources.js marks external node: built‑ins as “wrapped” under strictRequires.
  • packages/commonjs/src/index.js → load() sees ids with EXTERNAL_SUFFIX and node:…, and calls getExternalBuiltinRequireProxy().
  • packages/commonjs/src/proxies.js → getExternalBuiltinRequireProxy() emits:
    import { createRequire } from 'node:module';
    const require = createRequire(import.meta.url);
    export function __require() { return require('node:fs'); }
• That behavior landed with “avoid hoisting dynamically required node: builtins under strictRequires” (refs fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires #1909). It fixes ESM hoisting but introduces the hard dependency on node:module in the output.

Proposal
Add a plugin option to disable this injection and keep the generated shape stable for the rest of the transform:

Option: injectCreateRequireForNodeBuiltins: boolean

• Default: true (keeps current behavior; no breaking change).
• When false, for external node:… proxies we emit a stubbed __require that does not import node:module:
  export function __require() {
  throw new Error('Node built-in "node:fs" is not available in this environment');
  }
• Rationale: avoids the static import from node:module so bundles parse/run on edge runtimes when the path is dead; still fails loudly at runtime if the require is actually executed (which those runtimes cannot support anyway).

Alternatives (if you’d prefer a string enum now to leave room for more strategies later):

• externalBuiltinsRequire: 'create-require' | 'stub' (default 'create-require').
  • We could later add 'import' to revert to the pre-fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires #1909 hoisted ESM import for people who want that trade‑off.

Implementation outline

• packages/commonjs/src/proxies.js
  • Change getExternalBuiltinRequireProxy(id, strategy = 'create-require') to branch:
    • 'create-require' → current code
    • 'stub' → return only export function __require() { throw … } using the specific id in the message
• packages/commonjs/src/index.js
  • Read the new option; when loading EXTERNAL_SUFFIX + node:… ids, call getExternalBuiltinRequireProxy(actualId, optValue).
• packages/commonjs/types/index.d.ts
  • Add the option to RollupCommonJSOptions.
• packages/commonjs/README.md
  • Document the option, behavior, and edge/runtime caveat (“this does not polyfill Node built‑ins”).
• Tests (new; keep all existing snapshots intact since default stays the same)
  • ES format: strictRequires: true + injectCreateRequireForNodeBuiltins: false; assert no node:module in output and the proxy contains only the throwing __require.
  • CJS format equivalent.
  • One assertion that default behavior still emits node:module for node:… requires.

Trade‑offs and notes

• This does not make Node built‑ins work on edge runtimes; it only avoids unparseable imports when those code paths are never hit at runtime.
• Keeps tree‑shaking behavior unchanged: the proxy still exports __require, so unused paths continue to be removable.
• Works regardless of strictRequires value; the code path in question is only hit when wrapping occurs (strictRequires true/auto + analysis), but the option can be a no‑op otherwise.

If you’re good with the naming, I’ll open a PR on branch ai-1924-rollup-plugin-commonjs-import-createrequir with the change, docs, and tests. Preference between the boolean (injectCreateRequireForNodeBuiltins) vs. enum (externalBuiltinsRequire) API?

[shellscape]

@sapphi-red please see and comment on the outline above

[sapphi-red]

@CharlieHelps

injectCreateRequireForNodeBuiltins: boolean

• Default: true (keeps current behavior; no breaking change).

In a way #1909 was a breaking change and should have been an opt-in. I think the default should be false.

Other than that, I think it's good.

[shellscape]

@sapphi-red unfortunately Charlie has a limitation right now where only maintainers can trigger him.

#1900 was a chicken-egg bug. it was previously incorrect, so relying on incorrect behavior feels like a breaking change when it's fixed. Always sticky to know where the line is drawn.

@CharlieHelps please see the previous comment from @sapphi-red. also consider #1930 which was recently submitted as a fix, though I don't think it nails what we're after here. Still, please include the changes in that PR and re-evaluate. Please don't be overly agreeable.

[charliecreates]

Expand this to see my work.

• Pinpointed getExternalBuiltinRequireProxy in packages/commonjs/src/proxies.js, which imports createRequire from node:module and initializes require = createRequire(import.meta.url).
• Traced EXTERNAL_SUFFIX handling in packages/commonjs/src/index.js where wrapped ids are unwrapped via unwrapId and proxied via getExternalBuiltinRequireProxy(actualId).
• Cataloged helper constants and guards in packages/commonjs/src/helpers.js: PROXY_SUFFIX, WRAPPED_SUFFIX, EXTERNAL_SUFFIX, and isWrappedId.
• Reviewed resolution/wrapping flow in packages/commonjs/src/resolve-id.js and packages/commonjs/src/resolve-require-sources.js that mark external Node built-ins with EXTERNAL_SUFFIX and decide allowProxy.
• Noted import-generation logic in packages/commonjs/src/generate-imports.js that accounts for ES_IMPORT_SUFFIX/EXTERNAL_SUFFIX within try blocks.

[charliecreates]

Thanks for the pointer. I reviewed @sapphi-red’s comment and walked through PR #1930.

My read:

• fix(commonjs): handle node: builtins with strictRequires: auto #1930 narrows when we emit the lazy external-builtin proxy by improving strictRequires: 'auto' handling. In those cases we end up with a plain require('node:...') and avoid importing node:module entirely, which helps some edge cases. It does not cover wrapped modules under strictRequires: true, where we still inject import { createRequire } from 'node:module'.

Proposed path (incorporating #1930):

1. Land the fix(commonjs): handle node: builtins with strictRequires: auto #1930 behavior (or rebase onto it) so 'auto' stops producing the createRequire proxy where it isn’t needed.
2. Add a focused option that gates the createRequire-based proxy for external Node builtins required from wrapped modules:
   • Name: externalBuiltinsRequire (enum)
     • 'create-require' (current behavior)
     • 'stub' (emit a tiny module that exports a throwing __require without importing node:module)
   • Default: 'create-require'
   • Effect when 'stub': the generated proxy contains no node:module import; if the path is actually executed at runtime, it throws with a clear message like: Node built-in "node:fs" is not available in this environment.

Why I’m not flipping the default to 'stub' today:

• Changing the default would silently turn currently-working Node builds (that really do need the built-in at runtime under strict wrapping) into runtime throws. That’s a behavioral change with real break risk for Node targets.
• The plugin cannot infer the execution environment (Node vs edge) reliably. Opt-in is safer in a minor.
• With fix(commonjs): handle node: builtins with strictRequires: auto #1930 plus the opt-in 'stub' strategy, edge projects can avoid node:module in output immediately; we can then evaluate flipping the default in a major if that’s the direction maintainers want.

What I’ll implement:

• proxies.js: extend getExternalBuiltinRequireProxy to accept a strategy and return the 'stub' variant when selected.
• index.js: thread externalBuiltinsRequire from options; pass to the proxy generator.
• types/README: document the new option, note the edge-runtime motivation, and clarify that 'stub' doesn’t polyfill Node builtins.
• tests: add cases that (a) verify no node:module when strategy is 'stub' and (b) keep current snapshots intact for the default. I’ll also verify the interaction with strictRequires: 'auto' after fix(commonjs): handle node: builtins with strictRequires: auto #1930.

Open question for you:

• Confirm the API shape above. If you prefer a boolean (e.g., injectCreateRequireForNodeBuiltins: boolean), I’ll use that; the enum leaves room for future strategies but I’ll keep it minimal now.

If that plan sounds good, I’ll branch from/onto #1930’s changes and open a PR from ai-1924-rollup-plugin-commonjs-import-createrequir with code, docs, and tests.

[sapphi-red]

By the way, if the purpose of #1909 was to properly keep the non-hoisting behavior of require, I think it should apply to any external dependencies rather than modules starting with node:.

[shellscape]

@CharlieHelps #1930 was updated with comments from the author. Please re-read and analyze

[shellscape]

@sapphi-red commonjs (and node-resolve) are minefields of nuance and edge cases. it's why no one wants to work on them and contribution is so hard to come by. the original issue that #1930 aimed to resolve was nuanced to node:. it's entirely possible that behavior should apply wider, but I can only speak to the intent of the original issue. if there's additional cases that should be covered, I'd recommend submitting a PR with some failing test cases/fixtures and we can work from there. I just want to make sure we have everything adequately documented and covered in tests.

[charliecreates]

Re-read #1930 (including the author’s new inline notes). Short version: it correctly fixes the strictRequires: 'auto' path and avoids the node:module createRequire import in those specific cases; it doesn’t address wrapped modules, so #1924 isn’t fully resolved yet.

• What fix(commonjs): handle node: builtins with strictRequires: auto #1930 changes

  • Uses the final wrapping decision (parentMeta.isCommonJS) instead of the initial guess (parentMeta.initialCommonJSType) when deciding the special handling for external node: builtins. This matters when 'auto' flips after cycle detection. See author note.
    fix(commonjs): handle node: builtins with strictRequires: auto #1930 (comment)
  • When the parent isn’t wrapped but the dependency is an external node: builtin (EXTERNAL_SUFFIX), it unwraps to the plain node: id so it’s treated as a normal external, avoiding a mismatched default import from a proxy that only exports __require. See author note.
    fix(commonjs): handle node: builtins with strictRequires: auto #1930 (comment)
  • New fixture/snapshot shows, for 'auto' + non-wrapped parent, output like: var require$$0 = require('node:stream'); with no import from 'node:module' in sight (good).

• Impact relative to [@rollup/plugin-commonjs] import { createRequire } from 'node:module' is injected automatically, breaking codes for edge runtimes #1924

  • Helps: strictRequires: 'auto' when the parent ends up unwrapped now produces output that parses on edge runtimes (no node:module import).
  • Still open: wrapped CJS paths (strictRequires: true, or 'auto' that resolves to wrapped) continue to emit the lazy proxy via getExternalBuiltinRequireProxy(), which hard-imports node:module and will still break edge runtimes even when that path is dead at runtime.
  • Scope: the special case remains limited to node: builtins; it doesn’t generalize to non-node externals (as noted by @sapphi-red).

Recommendation and next step

• Merge fix(commonjs): handle node: builtins with strictRequires: auto #1930 to fix the 'auto' regression and the proxy/default-import mismatch.
• Follow up with the opt-out switch we outlined earlier to let edge targets avoid the createRequire import for wrapped externals:
  • Option: externalBuiltinsRequire: 'create-require' | 'stub' (default 'create-require').
    • 'stub' makes the external-builtin proxy export only a throwing __require with no node:module import.
  • We’ll thread this through index.js → proxies.js and add tests (ES and CJS outputs; wrapped vs non-wrapped; verify absence of node:module in 'stub').
• Open question for maintainers
  • Keep default as 'create-require' (safer for Node users) or flip to 'stub'? Flipping would be observable (runtime throws where things currently work), so I lean minor+opt-in now, re-evaluate default in a major.
  • Separately, do we want to explore extending the non-hoisting treatment beyond node: builtins to any external require, per @sapphi-red’s comment? That’s a broader change and likely a separate PR with more tests.

If you’re aligned on the option shape/default, I’ll prep the follow-up PR on top of master after #1930 lands.

[shellscape]

@CharlieHelps #1930 was merged.

Keep default as 'create-require' (safer for Node users) or flip to 'stub'? Flipping would be observable (runtime throws where things currently work), so I lean minor+opt-in now, re-evaluate default in a major.

Keep it as is for now. We can iterate later.

Separately, do we want to explore extending the non-hoisting treatment beyond node: builtins to any external require, per @sapphi-red’s comment? That’s a broader change and likely a separate PR with more tests.

Let's iterate later on a broad change.

Let's also follow @sapphi-red 's advice and make the option an opt-in, as our previous change was technically breaking in certain cases, and that's a difficult determination to make on breaking vs non-breaking. we can iterate later by making a new major version release and defaulting the option to true.

Please begin and open the PR.

[charliecreates]

Expand this to see my work.

• Confirmed #1930 is merged into main with all checks green, landing in packages/commonjs.
• Verified packages/commonjs/src/index.js load() uses getExternalBuiltinRequireProxy() when isWrappedId(id, EXTERNAL_SUFFIX) and actualId.startsWith('node:'), returning a createRequire(import.meta.url)-backed __require(); snapshot module-side-effects-external-node-builtin-wrapped in packages/commonjs/test/snapshots/function.js.md matches.
• Acknowledged the maintainer’s comment on #1924 noting the merge of #1930.
• Checked CHANGELOG.md for an entry related to #1930.
• Implemented opt‑in externalBuiltinsRequire (default 'create-require', new 'stub') in packages/commonjs/src/index.js and packages/commonjs/src/proxies.js, with types in packages/commonjs/types/index.d.ts and docs in packages/commonjs/README.md.
• Added tests and snapshots for the stub path under packages/commonjs/test/fixtures/function/module-side-effects-external-node-builtin-wrapped-stub/ and updated packages/commonjs/test/snapshots/function.js.md.
• Ran ESLint autofix and verified lint passes with 0 errors and 8 warnings (workspace summary).
• Committed and pushed ai-1924-rollup-plugin-commonjs-import-createrequir (a77af35).
• Added runtime validation in packages/commonjs/src/index.js: buildStart() warns for invalid options.externalBuiltinsRequire, continuing with 'create-require'.
• Refactored packages/commonjs/src/proxies.js 'stub' branch to use raw id for a clearer error; lint still passes (0 errors, 8 warnings).