diff --git a/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs b/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs index a80ab80e7e..cbdd2b3777 100644 --- a/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs +++ b/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs @@ -32,9 +32,19 @@ pub(crate) fn register_native_fetch_and_streams( // / unresolvable specifiers fall through to the legacy compile-time // refusal in `expr_call::intrinsics::try_require_literal`. if let Some(init_expr) = &decl.init { - if let Some(module_name) = require_resolvable_native_specifier(init_expr) { - register_require_namespace_binding(ctx, name, &module_name); - return true; + // #8342: inside a CJS-wrapped module the wrap's synthetic + // `function require(...)` (with a `createRequire`-backed built-in arm) + // shadows the bare global `require`. Don't steal `let x = + // require("process")` into a native-module namespace binding here — the + // native namespace isn't initialized in a CJS-wrapped module, so `x` + // would be undefined at runtime (`ReferenceError: node_process is not + // defined`). Let the call flow through to the synthetic require, which + // resolves builtins via `createRequire` (see `cjs_wrap::wrap`). + if !require_is_shadowed_by_local(ctx) { + if let Some(module_name) = require_resolvable_native_specifier(init_expr) { + register_require_namespace_binding(ctx, name, &module_name); + return true; + } } } diff --git a/crates/perry-hir/src/destructuring/var_decl_sources.rs b/crates/perry-hir/src/destructuring/var_decl_sources.rs index 32e6552e72..132254ee44 100644 --- a/crates/perry-hir/src/destructuring/var_decl_sources.rs +++ b/crates/perry-hir/src/destructuring/var_decl_sources.rs @@ -55,6 +55,26 @@ pub(crate) fn require_resolvable_native_specifier(init: &ast::Expr) -> Option bool { + ctx.lookup_local("require").is_some() + || ctx.lookup_func("require").is_some() + || ctx.lookup_imported_func("require").is_some() +} + /// #5216: the canonical (`node:`-stripped) native module name for a require /// specifier `raw`, iff it resolves to a Perry-supported native/Node-builtin /// module; otherwise `None`. `node:`-prefixed specifiers must name a real Node @@ -149,6 +169,17 @@ pub(super) fn register_destructured_stream_ctors( return Vec::new(); }; + // #8342: inside a CJS-wrapped module the wrap's synthetic + // `function require(...)` shadows the bare global `require`, and its + // built-in arm resolves `require("process")` etc. via `createRequire` at + // runtime. Don't register destructured members as native-module aliases + // here — the native namespace isn't initialized in a CJS-wrapped module, + // so the bindings would be undefined at runtime. Let the destructure run + // off the runtime `require(...)` call result instead. + if require_is_shadowed_by_local(ctx) && require_literal_specifier(init).is_some() { + return Vec::new(); + } + // #5216: `const { createInterface } = require("readline")` — when the RHS is // a `require("")` literal, register EVERY destructured member // as a native named member, exactly as `import { createInterface } from diff --git a/crates/perry/src/commands/compile/cjs_wrap/tests.rs b/crates/perry/src/commands/compile/cjs_wrap/tests.rs index cf1a6235ae..d0f9aedfbb 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/tests.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/tests.rs @@ -1933,3 +1933,53 @@ fn hoist_keeps_inheritance_chain_with_iife_local_parent_together() { hoisted2 ); } + +// #8342: a CJS-wrapped module that does `let node_process = require("process"); +// node_process = __toESM(node_process, 1)` (the rolldown-bundled +// `@socketsecurity/lib` shape) must NOT hoist a static +// `import _req_N from 'process'` for the Node.js built-in. The codegen does not +// initialize native-module import bindings inside CJS-wrapped modules, so the +// hoisted binding would be undefined at runtime and the HIR's destructuring +// var-decl pass would steal `node_process` into a native-module namespace +// binding (dropping the runtime local) — producing +// `ReferenceError: node_process is not defined`. The wrap must instead keep the +// body's `let node_process = require("process")` declaration and route the +// built-in through the synthetic require's `createRequire` arm. +#[test] +fn cjs_wrap_builtin_require_not_hoisted_as_static_import() { + let src = "let node_process = require(\"process\");\n\ + node_process = __toESM(node_process, 1);\n\ + module.exports = { platform: node_process.platform };\n"; + let path = PathBuf::from("/tmp/x/external-pack.js"); + let wrapped = wrap_commonjs(src, &path); + + // (1) No static ESM import of the built-in is emitted. The only import + // from a `node:` spec should be the wrap's own `createRequire` helper. + assert!( + !wrapped.contains("from 'process'"), + "built-in `process` must not be hoisted as a static import; got:\n{wrapped}" + ); + assert!( + !wrapped.contains("import _req_"), + "no synthetic `_req_N` import should be emitted for built-ins; got:\n{wrapped}" + ); + + // (2) The body's `let node_process = require(\"process\")` declaration + // survives (is not blanked) so it runs inside the IIFE and calls the + // synthetic require at runtime. + assert!( + wrapped.contains("let node_process = require(\"process\");"), + "the built-in require declaration must stay in the IIFE body; got:\n{wrapped}" + ); + + // (3) The synthetic require's per-spec case for `process` resolves via + // `createRequire`, not by returning the (non-existent) import local. + assert!( + wrapped.contains("__perry_cjs_create_require("), + "the built-in require case must use createRequire; got:\n{wrapped}" + ); + assert!( + !wrapped.contains("return _req_0;"), + "the built-in require case must not reference the dropped import local; got:\n{wrapped}" + ); +} diff --git a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs index b54187335e..c0dbcd8841 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/wrap.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/wrap.rs @@ -152,7 +152,20 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( if !dead_platform_requires.is_empty() { require_specs.retain(|spec| !dead_platform_requires.contains(spec)); } - + // #sdxgen: Identify Node.js built-in requires (`require("process")`, + // `require("os")`, etc.) so the synthetic `require` function can resolve + // them via `createRequire` at runtime instead of relying on the hoisted + // static import binding (which the codegen does not initialize for + // native modules inside CJS-wrapped modules). + let builtin_requires: Vec = require_specs + .iter() + .filter(|spec| { + let normalized = spec.strip_prefix("node:").unwrap_or(spec); + let base = normalized.split('/').next().unwrap_or(normalized); + perry_hir::is_node_builtin_module(base) + }) + .cloned() + .collect(); // Issue #652: hoist top-level `class X { ... }` declarations OUT of the // IIFE so the consumer's `import { X } from "pkg"` resolves to the real // class instead of a runtime property access on `_cjs.X`. @@ -284,6 +297,17 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( // Don't adopt a function-local alias — keep it lazy (see above). continue; } + // #sdxgen: Don't adopt aliases for Node.js built-in modules. The + // codegen doesn't initialize native-module import bindings inside + // CJS-wrapped modules, so an adopted alias would be undefined at + // runtime. Keeping the alias un-adopted means the declaration stays + // in the IIFE body and `require("process")` goes through the + // synthetic require, which resolves builtins via createRequire. + let normalized = spec.strip_prefix("node:").unwrap_or(spec); + let base = normalized.split('/').next().unwrap_or(normalized); + if perry_hir::is_node_builtin_module(base) { + continue; + } if import_local_names.iter().any(|n| n == alias) { continue; } @@ -332,6 +356,13 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( let imports = require_specs .iter() .zip(import_local_names.iter()) + // #8342: don't emit a static `import _req_N from 'process'` for Node.js + // built-in specs. The codegen does not initialize native-module import + // bindings inside CJS-wrapped modules, so the binding would be dropped + // by the HIR / left undefined at runtime. Builtins resolve through the + // synthetic require's `createRequire` arm instead (see `require_cases`), + // which never references the import local. + .filter(|(spec, _)| !builtin_requires.contains(spec)) .map(|(spec, local)| { // #4904: Node's underscore-prefixed internal http modules are // require-only re-exports of the public `http` surface @@ -391,7 +422,7 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( .into_iter() .map(|property| { format!( - "if (childBefore && childBefore.loaded === false) process.emitWarning(\"Accessing non-existent property '{property}' of module exports inside circular dependency\"); " + "if (childBefore && childBefore.loaded === false) globalThis.process?.emitWarning?.(\"Accessing non-existent property '{property}' of module exports inside circular dependency\"); " ) }) .collect::() @@ -406,12 +437,26 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( } else { None }; - let required_value = if needs_runtime_record { + let required_value = if builtin_requires.contains(spec) { + // #sdxgen: For Node.js built-in modules, resolve via createRequire + // at runtime instead of the hoisted import binding (which the + // codegen does not initialize for native modules in CJS-wrapped + // modules). createRequire calls js_create_native_module_namespace + // under the hood — the same path Node.js uses for require("process"). + format!("{link_child}return __perry_cjs_create_require({:?})(specifier);", source_path.to_string_lossy()) + } else if needs_runtime_record { runtime_require.clone().unwrap_or_else(|| format!("return {local};")) } else { format!("{link_child}return {local};") }; - if require_site_in_try(source, spec) { + if builtin_requires.contains(spec) { + // #8342: builtins have no static import binding (we skip emitting + // one above), so never reference `{local}` here — always go through + // the `createRequire`-backed `required_value`. The try-site + // `typeof {local} === 'boolean'` sentinel guard does not apply + // (builtins are never the pruned-build TRUE sentinel). + format!(" if (specifier === '{spec}') {{ {required_value} }}") + } else if require_site_in_try(source, spec) { format!( " if (specifier === '{spec}') {{ if (typeof {local} === 'boolean') \ throw __perry_cjs_require_error('error', 'MODULE_NOT_FOUND', \ @@ -703,6 +748,14 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( // immutable module-scope `const alias = _req_N;` (the const write // would throw) nor strip its declaration below. .filter(|(alias, _, _)| !identifier_is_reassigned(source, alias)) + // #8342: don't surface `const alias = _req_N;` for Node.js built-in + // specs — we no longer emit a static `import _req_N from ''` + // (the codegen doesn't initialize native-module import bindings in + // CJS-wrapped modules), so `_req_N` doesn't exist. The body's own + // ` alias = require('')` stays (builtins are excluded + // from the blanking filter below) and resolves through the synthetic + // require's `createRequire` arm at runtime. + .filter(|(_, spec, _)| !builtin_requires.contains(spec)) .filter_map(|(alias, spec, _range)| { let idx = require_specs.iter().position(|s| s == spec)?; // When the alias is already the spec's import local name @@ -722,6 +775,14 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( .into_iter() .filter(|(_, spec, _)| require_specs.iter().any(|s| s == spec)) .filter(|(alias, _, _)| !identifier_is_reassigned(source, alias)) + // #sdxgen: Don't blank alias declarations for Node.js built-in + // modules — let them stay in the IIFE body and resolve through + // the synthetic require (which uses createRequire for builtins). + .filter(|(_, spec, _)| { + let normalized = spec.strip_prefix("node:").unwrap_or(spec); + let base = normalized.split('/').next().unwrap_or(normalized); + !perry_hir::is_node_builtin_module(base) + }) .map(|(_, _, range)| range) .collect::>(); (lines, ranges) @@ -902,6 +963,13 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset( if (typeof specifier !== 'string') throw __perry_cjs_require_error('type', 'ERR_INVALID_ARG_TYPE', 'The "id" argument must be of type string.'); if (specifier === '') throw __perry_cjs_require_error('type', 'ERR_INVALID_ARG_VALUE', 'The argument "id" must be a non-empty string.'); {require_cases} + // #sdxgen: Node.js built-in modules that were NOT hoisted as static + // imports (see the builtin_requires filter above). Resolve them via + // createRequire at runtime, which calls js_create_native_module_namespace + // under the hood — the same path Node.js uses for require("process"). + if (__perry_cjs_require_is_builtin(specifier)) {{ + return __perry_cjs_create_require({module_path_literal})(specifier); + }} // Runtime `require(path)` of a module Perry AOT-compiled but that is // only reachable via a computed path. Next's webpack runtime uses both // absolute page paths and relative chunk paths (`./chunks/` + id). diff --git a/crates/perry/tests/cjs_wrap_builtin_require.rs b/crates/perry/tests/cjs_wrap_builtin_require.rs new file mode 100644 index 0000000000..940e1086a7 --- /dev/null +++ b/crates/perry/tests/cjs_wrap_builtin_require.rs @@ -0,0 +1,170 @@ +//! #8342: a CJS-wrapped module that does a bare top-level +//! `require("process")` (and other Node.js built-ins) must resolve the +//! built-in to the real namespace at runtime via the wrap's synthetic +//! `createRequire`-backed require arm. +//! +//! Pre-fix the HIR's destructuring `var`/`let`/`const` pass intercepted +//! `let node_process = require("process")` BEFORE call lowering and stole it +//! into a native-module namespace binding (`register_require_namespace_binding` +//! → `remove_local_binding`), mirroring `import * as node_process from +//! "process"`. But the codegen does not initialize native-module import +//! bindings inside CJS-wrapped modules, so `node_process` resolved to nothing +//! at runtime — `ReferenceError: node_process is not defined` — which blocked +//! `sdxgen --help` (the rolldown-bundled `@socketsecurity/lib` external-pack.js +//! starts with `let node_process = require("process"); node_process = +//! __toESM(node_process, 1)`). +//! +//! The fix gates the destructuring native-require fast path on `require` being +//! the bare global (not shadowed by the wrap's synthetic `function require`), +//! and the wrap no longer hoists a static `import _req_N from ''` for +//! built-in specs. The body's `require("")` call flows through to the +//! synthetic require, whose per-spec case resolves via `createRequire`. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +/// Compile a CJS entry (`.cjs` so the wrap is applied) and run it, returning +/// stdout. Asserts both the compile and the run succeed. +fn compile_and_run_cjs(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.cjs"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// The simplest witness: a bare `const p = require("process")` in a CJS-wrapped +/// module. Pre-fix this threw `ReferenceError: p is not defined` when the body +/// read `p.platform`. +#[test] +fn cjs_wrap_bare_builtin_require_resolves() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run_cjs( + dir.path(), + r#" +const p = require("process"); +const os = require("os"); +const path = require("path"); +console.log("platform:", p.platform); +console.log("cpus:", typeof os.cpus); +console.log("join:", typeof path.join, path.join("a", "b")); +"#, + ); + let platform = std::env::consts::OS; + let expected_platform = match platform { + "macos" => "darwin", + "linux" => "linux", + "windows" => "win32", + _ => platform, + }; + assert_eq!( + stdout, + format!( + "platform: {expected_platform}\ncpus: function\njoin: function a/b\n" + ) + ); +} + +/// The exact sdxgen/rolldown shape: `let node_process = require("process"); +/// node_process = __toESM(node_process, 1)` — the alias is REASSIGNED, so the +/// wrap can't adopt it, and the HIR's destructuring pass stole it into a +/// native-module namespace binding (dropping the runtime local). Pre-fix: +/// `ReferenceError: node_process is not defined` on every invocation. +#[test] +fn cjs_wrap_rolldown_toesm_builtin_require_resolves() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run_cjs( + dir.path(), + r#" +var __toESM = (mod, isNodeMode) => { + if (mod && typeof mod === "object" && mod.__esModule) return mod; + var target = {}; + Object.defineProperty(target, "default", { value: mod, enumerable: true }); + return Object.assign(target, mod); +}; +let node_process = require("process"); +node_process = __toESM(node_process, 1); +let node_os = require("os"); +node_os = __toESM(node_os, 1); +console.log("node_process.platform:", node_process.platform); +console.log("node_os.cpus:", typeof node_os.cpus); +module.exports = { platform: node_process.platform }; +"#, + ); + let platform = std::env::consts::OS; + let expected_platform = match platform { + "macos" => "darwin", + "linux" => "linux", + "windows" => "win32", + _ => platform, + }; + assert_eq!( + stdout, + format!( + "node_process.platform: {expected_platform}\nnode_os.cpus: function\n" + ) + ); +} + +/// Destructured built-in require: `const { platform } = require("process")` +/// in a CJS-wrapped module. The destructuring native-require fast path must +/// also bail when `require` is shadowed by the wrap's synthetic require, so +/// `platform` binds from the runtime `require("process")` result instead of a +/// native-module alias that isn't initialized in a CJS-wrapped module. +#[test] +fn cjs_wrap_destructured_builtin_require_resolves() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run_cjs( + dir.path(), + r#" +const { platform, arch } = require("process"); +const { join } = require("path"); +console.log("platform:", platform); +console.log("arch:", typeof arch); +console.log("join:", join("a", "b")); +"#, + ); + let platform = std::env::consts::OS; + let expected_platform = match platform { + "macos" => "darwin", + "linux" => "linux", + "windows" => "win32", + _ => platform, + }; + assert_eq!( + stdout, + format!( + "platform: {expected_platform}\narch: string\njoin: a/b\n" + ) + ); +}