From 38ca50dd593099faf83fc21834e773037b75ce95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 18 Sep 2026 21:38:55 +0000 Subject: [PATCH 1/2] fix(cjs): defer conditional CommonJS require() init instead of hoisting (#10437) Perry's CJS->ESM wrap turned every literal require('S') in a wrapped file into a hoisted static import, eager-initializing the target regardless of whether the surrounding control flow ever reaches the call. function_local_specs only kept a require() lazy when every call site sat inside a function body; a top-level if/for/while/switch/try/ &&/?: guard (including pg's own if (forceNative) { require('./native') }) still forced eager init. Broaden the classification to also cover a control-flow block (if/for/while/switch/catch/with/else/try/do/finally) and a braceless/operator equivalent (cond && require(...), cond ? require(...) : x, for (...) require(...) with no block) -- matching Node's actual 'loads only when control flow reaches it' semantics. An ordinary object literal, class body, or bare grouping block does not count (the common module.exports = { fs: require('fs') } barrel shape stays eager), and a process.platform === '' guard (node-pty's Windows/Unix terminal split) is exempted since the platform is a compile-time-known build target, not a runtime unknown. This was the sole remaining blocker compiling pg from source: pg crashed at init with Cannot find module 'pg-native' even though its guarding forceNative check was false. Fixes #10437. --- .../compile/cjs_wrap/extract_requires.rs | 181 ++++++++++++++++-- .../src/commands/compile/collect_modules.rs | 22 ++- .../_helpers/gap10437_cjs_lazy_require.cjs | 88 +++++++++ test-files/_helpers/gap10437_counter.cjs | 2 + .../_helpers/gap10437_native_rethrow.cjs | 11 ++ test-files/_helpers/gap10437_side_a.cjs | 2 + test-files/_helpers/gap10437_side_b.cjs | 2 + test-files/_helpers/gap10437_side_c.cjs | 2 + test-files/_helpers/gap10437_side_d.cjs | 2 + test-files/_helpers/gap10437_side_e.cjs | 2 + test-files/_helpers/gap10437_side_f.cjs | 2 + test-files/_helpers/gap10437_side_g.cjs | 2 + test-files/_helpers/gap10437_side_h.cjs | 2 + ...st_gap_cjs_conditional_require_deferred.ts | 20 ++ 14 files changed, 311 insertions(+), 29 deletions(-) create mode 100644 test-files/_helpers/gap10437_cjs_lazy_require.cjs create mode 100644 test-files/_helpers/gap10437_counter.cjs create mode 100644 test-files/_helpers/gap10437_native_rethrow.cjs create mode 100644 test-files/_helpers/gap10437_side_a.cjs create mode 100644 test-files/_helpers/gap10437_side_b.cjs create mode 100644 test-files/_helpers/gap10437_side_c.cjs create mode 100644 test-files/_helpers/gap10437_side_d.cjs create mode 100644 test-files/_helpers/gap10437_side_e.cjs create mode 100644 test-files/_helpers/gap10437_side_f.cjs create mode 100644 test-files/_helpers/gap10437_side_g.cjs create mode 100644 test-files/_helpers/gap10437_side_h.cjs create mode 100644 test-files/test_gap_cjs_conditional_require_deferred.ts diff --git a/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs b/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs index cdcb336a1b..5e7b061c15 100644 --- a/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs +++ b/crates/perry/src/commands/compile/cjs_wrap/extract_requires.rs @@ -293,19 +293,40 @@ pub fn identifier_is_declared_binding(source: &str, name: &str) -> bool { false } -/// Next.js lazy-require classification (single forward pass). Returns the set -/// of specifiers whose EVERY `require('')` call site is lexically inside -/// a FUNCTION body — never at module top level, and never inside a top-level -/// control-flow block that runs at module load. Node loads such a module -/// lazily (only when the enclosing function runs), so Perry must not eager-init -/// it. +/// Deferred-require classification (single forward pass). Returns the set of +/// specifiers whose EVERY `require('')` call site is NOT guaranteed to +/// run the moment the module loads — a function body (never called, or called +/// later: #Next.js lazy-require), a control-flow block that may not run every +/// time its enclosing scope runs (`if`/`for`/`while`/`switch`/`catch`/`with`/ +/// `else`/`try`/`do`/`finally`), or a braceless/operator-guarded equivalent of +/// the same thing (`cond && require(...)`, `cond ? require(...) : x`, `for +/// (...) require(...)` with no block). Node only ever loads such a module when +/// control flow actually reaches the call, so Perry must not eager-init it +/// either (issue #10437: `pg` guards its optional `pg-native` binding exactly +/// this way, behind `if (forceNative) { require('./native') }`). /// -/// Conservative by construction: a spec with any top-level call site (including -/// top-level `if`/`for`/`try` blocks, which execute during module evaluation) -/// is excluded and keeps the default eager behavior. A misclassification is -/// self-correcting at runtime — the require shim triggers the target's init -/// when `require()` is actually called — so this only governs eager-init-loop -/// membership. +/// An ordinary object literal (`{ key: require(...) }`), a class body, or a +/// bare grouping block do NOT count — their contents run unconditionally +/// whenever the enclosing statement/expression is reached, same as top level, +/// so nesting inside one of those must not flip a spec to lazy (that would be +/// the common `module.exports = { fs: require('fs'), path: require('path') }` +/// barrel-export shape, which really is eager). +/// +/// The ternary ALTERNATE arm (`cond ? x : require(...)`) is deliberately NOT +/// matched — a bare `:` immediately before `require(` is indistinguishable +/// from an object-literal property value or a `switch` case label without a +/// real parse, and guessing wrong there risks the same barrel-export +/// misclassification the object-literal exclusion above avoids. That shape +/// keeps the conservative eager default (a known, narrow gap — not in scope +/// for #10437's reproduction). +/// +/// A false POSITIVE here (treating a genuinely-unconditional require as +/// conditional) is harmless: the require shim still triggers the target's +/// init at the exact point the call is lexically reached, which for an +/// unconditional call is essentially the same moment eager pre-init would +/// have run it. A false NEGATIVE (missing a genuinely-conditional call) is +/// the actual bug class — the target loads (and can throw) before its +/// guarding condition was ever evaluated. /// /// Brace/paren scanning runs on a comment/string/regex-masked copy (same /// length, code structure preserved) so literal braces never corrupt the scope @@ -337,30 +358,65 @@ pub fn function_local_specs(source: &str) -> std::collections::HashSet { return HashSet::new(); } + // #10437 followup: a spec whose ONLY conditionality is a + // `process.platform === /!== ''` if/else guard (either branch — + // e.g. node-pty's `./windowsTerminal` / `./unixTerminal` split) must NOT + // be downgraded to lazy by the broader control-flow classification below. + // The platform is a build TARGET resolved at compile time, not a runtime + // unknown — `wrap_commonjs_for_target`'s `inactive_platform_guarded_requires` + // already prunes the dead branch's spec outright for a known target, and + // the live branch's spec keeps the eager `_req_N` classification it had + // before this fix. Treating a compile-time-resolved platform check as + // conditional the way a genuinely runtime-unknown check (env var, + // arbitrary function result) is would only add needless deferral, not + // fix a bug — #10437 is about conditions Perry cannot resolve at compile + // time. + let platform_guarded_specs = process_platform_guarded_specs(source); + let mbytes = masked.as_bytes(); let is_ident = |c: u8| c == b'_' || c == b'$' || c.is_ascii_alphanumeric(); let control_keywords = ["if", "for", "while", "switch", "catch", "with", "else"]; + // Bare-keyword control blocks with no parens (`try {`, `else {`, `do {`, + // `} finally {`) — as opposed to an object literal / class body / plain + // grouping block, whose opening `{` is also not preceded by `)`/`=>` but + // whose contents are NOT conditional (see doc comment above). + let bare_control_keywords = ["try", "else", "do", "finally"]; #[derive(PartialEq)] enum Scope { + /// Function/method/arrow/IIFE body: reachability depends on whether, + /// and when, the function is ever called. Function, + /// A control-flow block that may not run every time its enclosing + /// scope runs. Block, + /// Anything else brace-delimited whose contents run unconditionally + /// when reached (object literal, class body, bare grouping block). + /// Nesting here does not itself make an enclosed `require()` + /// conditional. + Other, } let mut scopes: Vec = Vec::new(); - // spec → (seen any site, all sites so far in-function). + // spec → (seen any site, all sites so far conditionally-reached). let mut state: HashMap<&str, (bool, bool)> = HashMap::new(); let mut next_site = 0usize; - let in_function = |scopes: &[Scope]| scopes.contains(&Scope::Function); + let gates_reachability = |scopes: &[Scope]| { + scopes + .iter() + .any(|s| matches!(s, Scope::Function | Scope::Block)) + }; let mut i = 0usize; while i < mbytes.len() { // Record any require site at this offset before processing the char. while next_site < sites.len() && sites[next_site].0 == i { let (_, spec) = sites[next_site]; - let here = in_function(&scopes); + let conditional = !platform_guarded_specs.contains(spec) + && (gates_reachability(&scopes) + || site_is_conditionally_guarded(&masked, mbytes, i, &is_ident)); let e = state.entry(spec).or_insert((false, true)); e.0 = true; - e.1 = e.1 && here; + e.1 = e.1 && conditional; next_site += 1; } match mbytes[i] { @@ -381,7 +437,18 @@ pub fn function_local_specs(source: &str) -> std::collections::HashSet { Scope::Function } } else { - Scope::Block + // Not preceded by `)` or `=>`: a bare control keyword + // (`try`/`else`/`do`/`finally`) is conditional; an object + // literal, class body, or plain grouping block is not. + let mut w = p; + while w > 0 && is_ident(mbytes[w - 1]) { + w -= 1; + } + if bare_control_keywords.iter().any(|k| *k == &masked[w..p]) { + Scope::Block + } else { + Scope::Other + } }; scopes.push(kind); } @@ -395,16 +462,19 @@ pub fn function_local_specs(source: &str) -> std::collections::HashSet { // Any sites at EOF offset (defensive). while next_site < sites.len() { let (_, spec) = sites[next_site]; + let conditional = !platform_guarded_specs.contains(spec) + && (gates_reachability(&scopes) + || site_is_conditionally_guarded(&masked, mbytes, mbytes.len(), &is_ident)); let e = state.entry(spec).or_insert((false, true)); e.0 = true; - e.1 = e.1 && in_function(&scopes); + e.1 = e.1 && conditional; next_site += 1; } state .into_iter() - .filter_map(|(spec, (seen, all_in_fn))| { - if seen && all_in_fn { + .filter_map(|(spec, (seen, all_conditional))| { + if seen && all_conditional { Some(spec.to_string()) } else { None @@ -413,6 +483,77 @@ pub fn function_local_specs(source: &str) -> std::collections::HashSet { .collect() } +/// Is the `require(` call whose match starts at masked-source offset +/// `call_start` reached only conditionally by a nearby operator or a +/// braceless control-flow header, even though it has no enclosing `{ }` +/// scope of its own? Brace-scope tracking (above) can't see these shapes: +/// `cond && require(...)` / `cond || require(...)` / `cond ?? require(...)`, +/// the ternary CONSEQUENT arm `cond ? require(...) : x`, a braceless arrow +/// `() => require(...)`, and a braceless control-flow body — `if (...) +/// require(...)`, `for (...) require(...)`, `while (...) require(...)`, +/// `else require(...)`, `do require(...)`. +fn site_is_conditionally_guarded( + masked: &str, + mbytes: &[u8], + call_start: usize, + is_ident: &impl Fn(u8) -> bool, +) -> bool { + let mut p = call_start; + while p > 0 && (mbytes[p - 1] as char).is_whitespace() { + p -= 1; + } + if p == 0 { + return false; + } + if p >= 2 { + let two = &masked[p - 2..p]; + if two == "&&" || two == "||" || two == "??" || two == "=>" { + return true; + } + } + // Ternary consequent (`cond ? require(...) : x`) — a lone `?`, not the + // second char of `??` (already handled above). + if mbytes[p - 1] == b'?' && !(p >= 2 && mbytes[p - 2] == b'?') { + return true; + } + // Braceless control-flow header: `if (...)`, `for (...)`, `while (...)` + // immediately followed by the require call (no block). + if mbytes[p - 1] == b')' { + let head = matched_open_head(masked, mbytes, p - 1, is_ident); + return matches!(head.as_str(), "if" | "for" | "while"); + } + // Bare `else`/`do` immediately before, with no parens and no block. + let mut w = p; + while w > 0 && is_ident(mbytes[w - 1]) { + w -= 1; + } + matches!(&masked[w..p], "else" | "do") +} + +/// Every `require('')` specifier textually inside EITHER branch of a +/// `if (process.platform === /!== '') { … } else { … }` guard. +/// Mirrors the pattern `wrap.rs`'s `inactive_platform_guarded_requires` +/// matches to prune the DEAD branch's spec for a known build target — this +/// helper is target-independent and returns BOTH branches' specs, so the +/// LIVE branch's spec (which `inactive_platform_guarded_requires` keeps) can +/// be exempted from the general conditional-require classification above. +fn process_platform_guarded_specs(source: &str) -> std::collections::HashSet { + let re = perry_perex::tooling::Regex::new( + r#"(?s)if\s*\(\s*process\.platform\s*(?:===|!==)\s*['"][^'"]+['"]\s*\)\s*\{(?P.*?)\}\s*else\s*\{(?P.*?)\}"#, + ) + .unwrap(); + let mut specs = std::collections::HashSet::new(); + for cap in re.captures_iter(source) { + if let Some(then) = cap.name("then") { + specs.extend(extract_require_specifiers(then.as_str())); + } + if let Some(els) = cap.name("else") { + specs.extend(extract_require_specifiers(els.as_str())); + } + } + specs +} + /// Given the index of a `)` in the masked source, walk back to its matching /// `(` and return the identifier/keyword immediately before that `(`. fn matched_open_head( diff --git a/crates/perry/src/commands/compile/collect_modules.rs b/crates/perry/src/commands/compile/collect_modules.rs index 320eb65545..a179e14cc5 100644 --- a/crates/perry/src/commands/compile/collect_modules.rs +++ b/crates/perry/src/commands/compile/collect_modules.rs @@ -1777,15 +1777,19 @@ fn collect_module_one( } } - // Next.js lazy-require: the CJS→ESM wrap names a binding `_lazyreq_N` when - // every `require('S')` call site is inside a function body (lazy in Node). - // Tag the import so `classify_eager_modules` leaves the target Deferred — - // matching Node, which only loads such a module when the enclosing function - // runs (e.g. jsonwebtoken, required only inside Next.js's request handlers). - // The require shim triggers the target's `__init` on first `require()`, so - // an over-eager classification is self-correcting at runtime. Limited to - // Perry-compiled (`NativeCompiled`) targets — native stdlib / V8 modules - // have their own init paths. + // Deferred require (#10437, originally the Next.js lazy-require case): the + // CJS→ESM wrap names a binding `_lazyreq_N` when every `require('S')` call + // site is NOT guaranteed to run the moment the module loads — inside a + // function body (lazy in Node: jsonwebtoken, required only inside Next.js's + // request handlers), or inside a top-level control-flow block / braceless + // equivalent that may never run (`if (forceNative) { require('./native') }`, + // pg's optional native binding). Tag the import so `classify_eager_modules` + // leaves the target Deferred — matching Node, which only loads such a + // module when control flow actually reaches the call. The require shim + // triggers the target's `__init` at that same call site, so an over-eager + // classification is self-correcting at runtime (it just runs a bit early). + // Limited to Perry-compiled (`NativeCompiled`) targets — native stdlib / + // V8 modules have their own init paths. { for import in &mut hir_module.imports { if import.type_only diff --git a/test-files/_helpers/gap10437_cjs_lazy_require.cjs b/test-files/_helpers/gap10437_cjs_lazy_require.cjs new file mode 100644 index 0000000000..62ecf6644f --- /dev/null +++ b/test-files/_helpers/gap10437_cjs_lazy_require.cjs @@ -0,0 +1,88 @@ +'use strict' +// #10437: CommonJS `require()` outside a function is hoisted and run +// unconditionally at module init, including inside `if (false)` and other +// branches that never run. Every require below except H is inside a branch +// that never executes; only H's side-effect module should ever load, and it +// should load exactly at the point control flow reaches it (between the +// "before taken branch" and "after taken branch" log lines) — not before +// the first statement, and not before H's guarding condition was evaluated. +// +// This is the shape pg 8.22.0 hits verbatim: `lib/index.js` guards an +// optional native binding behind `if (forceNative) { require('./native') }`, +// and `./native` transitively requires the optional, often-uninstalled +// `pg-native`. The crash-form section below reproduces that two-hop shape +// with a target that genuinely does not resolve on disk. + +console.log('start') + +// A: literal false +if (false) { + require('./gap10437_side_a.cjs') +} +// B: short-circuit +false && require('./gap10437_side_b.cjs') +// C: runtime-false env check (pg's `if (forceNative)` shape) +if (process.env.PERRY_GAP10437_UNSET_C) { + require('./gap10437_side_c.cjs') +} +// D: ternary arm not taken +const d = process.env.PERRY_GAP10437_UNSET_D ? require('./gap10437_side_d.cjs') : 'd-skipped' +// E: switch case not taken +switch (1) { + case 2: + require('./gap10437_side_e.cjs') +} +// F: loop body never runs +for (let i = 0; i < 0; i++) require('./gap10437_side_f.cjs') +// G: function never called (already correctly deferred pre-#10437) +function never() { + return require('./gap10437_side_g.cjs') +} + +console.log('before taken branch') + +// H: the taken branch — must load exactly here, not earlier. +if (true) { + require('./gap10437_side_h.cjs') +} + +console.log('after taken branch, d=' + d) + +// Caching: two conditional requires of the SAME module must run the side +// effect once and return the SAME exports object both times. +let capA = null +let capB = null +if (true) { + capA = require('./gap10437_counter.cjs') +} +if (true) { + capB = require('./gap10437_counter.cjs') +} +console.log('cache same=' + (capA === capB) + ' n=' + capA.n) + +// Crash-form (pg-native shape): an optional native binding behind an unset +// env check, whose target itself unconditionally (but inside a +// non-swallowing try/catch) requires a module that does not exist on disk. +// Pre-fix this crashed the whole program with "Cannot find module" even +// though the guarding env var was never set. +let impl = 'js' +if (process.env.PERRY_GAP10437_USE_NATIVE) { + impl = require('./gap10437_native_rethrow.cjs') +} +console.log('impl=' + impl) + +// A genuinely missing module behind a try/catch that SWALLOWS the error, +// itself nested inside a condition that never runs. +let fallback = 'default' +if (process.env.PERRY_GAP10437_UNSET_FALLBACK) { + try { + fallback = require('./gap10437_does_not_exist.cjs') + } catch (e) { + fallback = 'caught' + } +} +console.log('fallback=' + fallback) + +console.log('end') + +module.exports = { never: never } diff --git a/test-files/_helpers/gap10437_counter.cjs b/test-files/_helpers/gap10437_counter.cjs new file mode 100644 index 0000000000..35643ed461 --- /dev/null +++ b/test-files/_helpers/gap10437_counter.cjs @@ -0,0 +1,2 @@ +console.log('counter evaluated') +module.exports = { n: 1 } diff --git a/test-files/_helpers/gap10437_native_rethrow.cjs b/test-files/_helpers/gap10437_native_rethrow.cjs new file mode 100644 index 0000000000..76e34c552c --- /dev/null +++ b/test-files/_helpers/gap10437_native_rethrow.cjs @@ -0,0 +1,11 @@ +'use strict' +// pg 8.22.0 lib/native/client.js:3-10 shape: an optional native addon, +// required unconditionally once this file's own init runs, wrapped in a +// try/catch that RE-THROWS rather than swallowing. +var Native +try { + Native = require('./gap10437_missing_optional_dep.cjs') +} catch (e) { + throw e +} +module.exports = Native diff --git a/test-files/_helpers/gap10437_side_a.cjs b/test-files/_helpers/gap10437_side_a.cjs new file mode 100644 index 0000000000..5efe46f1b2 --- /dev/null +++ b/test-files/_helpers/gap10437_side_a.cjs @@ -0,0 +1,2 @@ +console.log('side_a evaluated') +module.exports = 'a' diff --git a/test-files/_helpers/gap10437_side_b.cjs b/test-files/_helpers/gap10437_side_b.cjs new file mode 100644 index 0000000000..3d552351d4 --- /dev/null +++ b/test-files/_helpers/gap10437_side_b.cjs @@ -0,0 +1,2 @@ +console.log('side_b evaluated') +module.exports = 'b' diff --git a/test-files/_helpers/gap10437_side_c.cjs b/test-files/_helpers/gap10437_side_c.cjs new file mode 100644 index 0000000000..e72b064d6c --- /dev/null +++ b/test-files/_helpers/gap10437_side_c.cjs @@ -0,0 +1,2 @@ +console.log('side_c evaluated') +module.exports = 'c' diff --git a/test-files/_helpers/gap10437_side_d.cjs b/test-files/_helpers/gap10437_side_d.cjs new file mode 100644 index 0000000000..aa22fe700e --- /dev/null +++ b/test-files/_helpers/gap10437_side_d.cjs @@ -0,0 +1,2 @@ +console.log('side_d evaluated') +module.exports = 'd' diff --git a/test-files/_helpers/gap10437_side_e.cjs b/test-files/_helpers/gap10437_side_e.cjs new file mode 100644 index 0000000000..4a43a95ec8 --- /dev/null +++ b/test-files/_helpers/gap10437_side_e.cjs @@ -0,0 +1,2 @@ +console.log('side_e evaluated') +module.exports = 'e' diff --git a/test-files/_helpers/gap10437_side_f.cjs b/test-files/_helpers/gap10437_side_f.cjs new file mode 100644 index 0000000000..c886b3edd4 --- /dev/null +++ b/test-files/_helpers/gap10437_side_f.cjs @@ -0,0 +1,2 @@ +console.log('side_f evaluated') +module.exports = 'f' diff --git a/test-files/_helpers/gap10437_side_g.cjs b/test-files/_helpers/gap10437_side_g.cjs new file mode 100644 index 0000000000..2c5c7003eb --- /dev/null +++ b/test-files/_helpers/gap10437_side_g.cjs @@ -0,0 +1,2 @@ +console.log('side_g evaluated') +module.exports = 'g' diff --git a/test-files/_helpers/gap10437_side_h.cjs b/test-files/_helpers/gap10437_side_h.cjs new file mode 100644 index 0000000000..3a0b41452a --- /dev/null +++ b/test-files/_helpers/gap10437_side_h.cjs @@ -0,0 +1,2 @@ +console.log('side_h evaluated') +module.exports = 'h' diff --git a/test-files/test_gap_cjs_conditional_require_deferred.ts b/test-files/test_gap_cjs_conditional_require_deferred.ts new file mode 100644 index 0000000000..01d2549078 --- /dev/null +++ b/test-files/test_gap_cjs_conditional_require_deferred.ts @@ -0,0 +1,20 @@ +// #10437: CommonJS `require()` outside a function is hoisted and run +// unconditionally at module init, whatever the surrounding control flow. +// Perry loaded every `require('')` in a CJS file before the +// file's first statement ran, so a branch that never runs (`if (false)`, a +// false env check, `&&`, `?:`, `switch`, a loop that never iterates) still +// loaded its module, and a module reached via a taken branch loaded before +// the statements preceding it. +// +// The crash form is `pg` 8.22.0: `lib/index.js` guards its optional native +// binding behind `if (forceNative) { require('./native') }`, and `./native` +// requires the optional, often-uninstalled `pg-native`. Every program using +// `pg` crashed at init with `Cannot find module 'pg-native'` even though +// `forceNative` was false. `./_helpers/gap10437_cjs_lazy_require.cjs` +// reproduces the full variant matrix (A-H from the issue, plus require +// caching and a swallowed try/catch around a genuinely missing module) in +// one file so the expected interleaving with its own `console.log` calls is +// unambiguous. +import mod from "./_helpers/gap10437_cjs_lazy_require.cjs"; + +console.log("typeof never=" + typeof mod.never); From b1ba0caf5fe6a1a2425e6eb1841f3b9e52e8ef0d Mon Sep 17 00:00:00 2001 From: Perry Bot Date: Fri, 18 Sep 2026 22:45:40 +0000 Subject: [PATCH 2/2] changelog: #10674 --- .../10674-cjs-conditional-require-deferred.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 changelog.d/10674-cjs-conditional-require-deferred.md diff --git a/changelog.d/10674-cjs-conditional-require-deferred.md b/changelog.d/10674-cjs-conditional-require-deferred.md new file mode 100644 index 0000000000..7b1c487441 --- /dev/null +++ b/changelog.d/10674-cjs-conditional-require-deferred.md @@ -0,0 +1,36 @@ +### Fixed + +- **CommonJS `require()` outside a function is no longer hoisted past the + control flow that guards it.** Perry's CJS→ESM wrap turned every + literal `require('S')` in a wrapped file into a static `import` at the + top of the module and eager-initialized the target — even when the call + sat inside `if (false)`, a false env check, `&&`/`??`, a ternary arm, a + `switch` case, or a loop that never iterates. A module reached only + through such a branch loaded (and could throw) at program start, + regardless of whether the branch ever ran; a module reached through a + taken branch loaded before the statements preceding it. This was the + sole remaining blocker compiling `pg` from source: `lib/index.js` guards + its optional native binding behind `if (forceNative) { require('./native') }`, + and `./native` requires the often-uninstalled `pg-native` — every + program using `pg` crashed at init with `Cannot find module 'pg-native'` + even though `forceNative` was false. + `cjs_wrap::extract_requires::function_local_specs` now classifies a + `require()` call site as deferred (Node's actual "loads only when + control flow reaches it" semantics) whenever it sits inside a + control-flow block (`if`/`for`/`while`/`switch`/`catch`/`try`/`else`/ + `do`/`finally`) or a braceless/operator equivalent (`cond && + require(...)`, `cond ? require(...) : x`, `for (...) require(...)` with + no block) — not only inside a function body as before. An ordinary + object literal or class body still does not count, so the common + `module.exports = { fs: require('fs'), path: require('path') }` barrel + shape stays eager. A `process.platform === ''` guard (the + node-pty Windows/Unix terminal split) is exempted from the broader + reclassification and keeps its existing eager treatment — the platform + is a compile-time-known build target, not a runtime unknown, and + `wrap_commonjs_for_target`'s dead-branch pruning already resolves it. + +Verified end-to-end: `pg` now compiles, links, and runs from real source +under `perry.compilePackages`, reaching a real TCP connect attempt with no +`pg-native` crash. + +Fixes #10437.