feat(commonjs)!: revert PR #1909 and add requireNodeBuiltins option

[Copilot]

Summary

Successfully implemented the requireNodeBuiltins option to address issue rollup#1924 by reverting PR rollup#1909's automatic createRequire injection and making it opt-in.

Completed Tasks

• Understand the current implementation from PR fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires rollup/plugins#1909
• Add new option requireNodeBuiltins (boolean, default: false) to types
• Update index.js to read the option and pass it through
• Modify resolve-require-sources.js to check option before applying special behavior
• Revert default behavior (when option is false/undefined) - node: builtins treated as regular externals
• Update existing tests to explicitly enable option where needed
• Update README.md with option documentation
• Run linting, build and tests - all passing (282/282 tests)
• Address code review feedback - use explicit boolean checks and default values
• Security scan - no vulnerabilities found
• Add requireNodeBuiltins: false variant tests (3 new test fixtures)
• Generate snapshots for new test fixtures (285/285 tests passing)

Test Coverage

Created 3 new test fixtures to verify default behavior (requireNodeBuiltins: false):

• strict-requires-external-node-builtin-default - Tests dead code elimination without createRequire
• strict-requires-auto-external-node-builtin-default - Tests hoisting with strictRequires: auto
• module-side-effects-external-node-builtin-wrapped-default - Tests side effects handling

All snapshots generated and all tests passing (285/285).

Implementation Details

Option: requireNodeBuiltins: boolean (default: false)

Behavior:

• false (default): External Node built-ins like node:fs are treated as regular externals, avoiding the node:module dependency. This is safe for edge runtimes.
• true: Uses createRequire(import.meta.url) for lazy loading, matching PR fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires rollup/plugins#1909 behavior. Adds node:module dependency.

Files Modified:

• packages/commonjs/types/index.d.ts - Added type definition
• packages/commonjs/src/index.js - Read option and apply conditionally
• packages/commonjs/src/resolve-require-sources.js - Conditional special handling
• packages/commonjs/README.md - Comprehensive documentation
• Test fixtures - Updated to explicitly enable option where needed + 3 new variant tests
• Snapshots - Generated for all tests

Testing

• All 285 tests passing (282 original + 3 new)
• Linting passing with no new warnings
• CodeQL security scan: 0 alerts
• Manual verification of both default and enabled behaviors

Next Steps

Ready for maintainer review and merge.

Original prompt

Revert rollup#1909 and reapply with the new option to ensure that the behavior is same when the option is set. See rollup#1924 and the comments for the reason to do so. Follow https://github.com/sapphi-red/rollup-plugins/blob/master/.charlie/instructions/pull-requests.md when creating the pull request.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[sapphi-red]

@copilot add requireNodeBuiltins: false variant tests for the 3 tests you changed

[Copilot]

@copilot add requireNodeBuiltins: false variant tests for the 3 tests you changed

Added 3 new test fixtures with requireNodeBuiltins: false:

• strict-requires-external-node-builtin-default
• strict-requires-auto-external-node-builtin-default
• module-side-effects-external-node-builtin-wrapped-default

These test the default behavior where node: builtins are treated as regular externals without createRequire injection. Snapshots will need to be generated locally (non-CI environment).

Commit: 92634ff

[sapphi-red]

@copilot re-run tests to generate the snapshot and commit it

[Copilot]

@copilot re-run tests to generate the snapshot and commit it

Snapshots generated successfully. All 285 tests passing (282 original + 3 new variant tests).

Generated snapshots for:

• module-side-effects-external-node-builtin-wrapped-default
• strict-requires-auto-external-node-builtin-default
• strict-requires-external-node-builtin-default

Commit: bd43a20