fix(commonjs): avoid hoisting dynamically required node: builtins under strictRequires

[charliecreates]

This fixes #1900 by ensuring that dynamically required Node built-ins (via node:*) are not hoisted when strictRequires wraps the requiring module.

Changes

• When the requiring module is wrapped (strictRequires or dynamic-require), treat external node:* specifiers as "wrapped" requires so the transform rewrites calls to a lazy __require().
• Add a dedicated proxy generator getExternalBuiltinRequireProxy(id) that exports a lazy __require implemented with createRequire(import.meta.url). This avoids top-level import 'node:*' and therefore avoids runtime crashes on older Node versions (e.g., node:sqlite on Node 20).
• Keep non-strict behavior unchanged so existing unresolved-import warnings (e.g., require('path')) are preserved.
• Tests: add fixture strict-requires-external-node-builtin.

Verification

# Build: success
$ pnpm -C packages/commonjs build

# Tests: 280/280 passed (includes new fixture)
$ pnpm -C packages/commonjs test

# Type tests: success
$ pnpm -C packages/commonjs test:ts

[charliecreates]

No issues identified in the provided diff since it only includes a snapshot update with binary changes omitted. Without visible source or snapshot content, there’s nothing actionable to review. If there are logic changes tied to this snapshot, they are not shown here.

Summary of changes

Snapshot updates

• Updated test snapshot: packages/commonjs/test/snapshots/function.js.snap.

No source code changes are visible in the provided diff; only snapshot output changed (details omitted).

[shellscape]

@CharlieHelps you need to rebase from upstream/master to pull in the workflow changes that mitigate the workflow run error

[charliecreates]

Expand this to see my work.

• Fetched latest refs from origin and inspected origin/master (head at 648fef6); working tree is clean.
• Rebased ai-1900-rollup-plugin-commonjs-strictrequires-shou onto origin/master without conflicts (“Successfully rebased and updated”), bringing in .github/workflows/* updates.
• Verified feature commit 7e38149 remains intact post-rebase; changes still scoped to packages/commonjs and the strictRequires behavior.

[shellscape]

@CharlieHelps have you identified any breaking changes here?

[charliecreates]

Expand this to see my work.

• Inspected changes in packages/commonjs/src/index.js: when isWrappedId(id, EXTERNAL_SUFFIX) and unwrapId(id, EXTERNAL_SUFFIX) starts with node:, return getExternalBuiltinRequireProxy(actualId) instead of getUnknownRequireProxy(...).
• Reviewed new getExternalBuiltinRequireProxy(id) in packages/commonjs/src/proxies.js, which emits a lazy __require() via createRequire(import.meta.url) to avoid hoisted ESM imports of built-ins.
• Reviewed packages/commonjs/src/resolve-require-sources.js adjustments: special-case externals under IS_WRAPPED_COMMONJS by checking isWrappedId(dependencyId, EXTERNAL_SUFFIX) and unwrapId(...) for node:*, ensuring they’re treated as wrapped.
• Confirmed new tests/fixtures: test/fixtures/function/strict-requires-external-node-builtin/* and snapshots test/snapshots/function.js.{md,snap} cover require('node:fs') under strictRequires, indicating expected lazy __require() behavior.
• Validated intent against commit 7e38149 message, matching "avoid hoisting dynamically required node: builtins under strictRequires".
• Analyzed behavior delta: previously getUnknownRequireProxy(...) could hoist import 'node:fs'; now wrapped modules defer loading via __require() at call time, aligning with CJS timing; impact is narrowly scoped to external node:* under wrapping.
• Assessed break risk: users relying on eager import timing of Node built-ins could observe later resolution, but built-ins typically lack init side effects; broader strictRequires users and non-node:* externals remain unaffected.

[charliecreates]

Rebased the branch onto the latest master and pushed so the updated workflow files are included.

• Branch: ai-1900-rollup-plugin-commonjs-strictrequires-shou
• New HEAD: 923b19e (rebased onto master at 648fef6)
• Push method: --force-with-lease

A fresh Actions run should kick off with the mitigations applied.

What I ran

# Update and rebase
git fetch --prune origin
git rebase origin/master
git push --force-with-lease origin ai-1900-rollup-plugin-commonjs-strictrequires-shou

# Package-level verification for the changed plugin
pnpm -C packages/commonjs build
pnpm -C packages/commonjs test
pnpm -C packages/commonjs test:ts

Verification

# Build: success
$ pnpm -C packages/commonjs build

# Tests: 280/280 passed
$ pnpm -C packages/commonjs test

# Type tests: success
$ pnpm -C packages/commonjs test:ts

Let me know if you want me to re-run anything else or scope checks differently.

[charliecreates]

Short answer: No breaking changes identified.

• Scope is very narrow: it only affects requires of node: builtins that occur inside modules wrapped by strictRequires (or dynamic-require). Non‑strict behavior and regular requires like require('path') are unchanged.
• Behavior becomes more correct, not broader: those builtins are now required lazily via a __require() helper instead of being hoisted to a top‑level ESM import. The only observable difference is error timing when a builtin doesn’t exist (e.g., node:sqlite on older Node) — it now throws when that code path runs, matching CJS semantics.
• Compatibility: the proxy uses import { createRequire } from 'node:module', which is supported in our stated engines (Node ≥16 or ≥14.17) and is already used elsewhere in this repo.

Notes

• This change avoids top‑level import 'node:*' for wrapped modules, which previously could crash at module init for builtins not present on a given Node version.
• Browser builds are not loosened by this; if such a require is actually included, the generated chunk still imports node:module and will fail as before. Unreachable branches (e.g., if (false) { require('node:sqlite') }) continue to be tree‑shaken; the new fixture strict-requires-external-node-builtin covers this.
• Semver: patch (bug fix with no intentional user‑facing API or default‑path behavior change).