fix(cjs): evaluate conditional requires at the call site

changelog.d/10285-conditional-require-call-site.md

@@ -0,0 +1,8 @@
+Fixed CommonJS `require()` calls inside branches, ternaries, short-circuit
+operands, logical assignments, `try` blocks, `switch` cases and loop bodies
+being hoisted into eager imports. The required module now initializes when the
+`require` executes, as in Node, instead of before the requiring module's first
+statement — and not at all when the branch is never taken. Deferred targets
+initialize through the path-module registry, so side-effect-only modules run,
+a throwing `require` stays inside its `try`/`catch`, and deferred classes get
+their static fields.

crates/perry-codegen/src/expr/dyn_extern_i18n.rs

@@ -829,6 +829,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
         // checks work; calling those values via stored references would
         // need a separate runtime path that this commit doesn't add.
         Expr::ExternFuncRef { name, .. } => {
+            // A synthetic deferred require evaluates its dependency at the
+            // original call site. Do this before the class/namespace fast
+            // paths too: those values can depend on module initialization.
+            if name.starts_with("_lazyreq_") {
+                if let Some(source_prefix) = ctx.import_function_prefixes.get(name) {
+                    let init_fn = format!("{}__init", source_prefix);
+                    ctx.pending_declares
+                        .push((init_fn.clone(), crate::types::VOID, vec![]));
+                    ctx.block().call_void(&init_fn, &[]);
+                }
+            }
             // Imported class references (refs #420 / drizzle): when `name`
             // resolves to a class registered in `ctx.class_ids` (populated
             // from `opts.imported_classes` for imported classes too), emit
@@ -881,18 +892,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 }
             }
             if let Some(source_prefix) = ctx.import_function_prefixes.get(name).cloned() {
-                // Next.js lazy-require: a `_lazyreq_N` binding is the CJS require
-                // shim's handle to a FUNCTION-LOCAL `require('S')`. S is
-                // `Deferred` (never eager-initialized), so before reading its
-                // default-export getter, fire `<S>__init()` — idempotent, so
-                // re-reads cost a guard check. This is the moment Node would run
-                // S's module body: when `require('S')` is actually called.
-                if name.starts_with("_lazyreq_") {
-                    let init_fn = format!("{}__init", source_prefix);
-                    ctx.pending_declares
-                        .push((init_fn.clone(), crate::types::VOID, vec![]));
-                    ctx.block().call_void(&init_fn, &[]);
-                }
                 // Issue #678 followup: a V8-fallback import used as a value
                 // (rather than called directly) has no native singleton
                 // wrapper to point at — the `__perry_wrap_extern_*` for V8

crates/perry/src/commands/compile/cjs_wrap/deferred_requires.rs

@@ -0,0 +1,254 @@
+//! Preserve the evaluation boundary of conditional and function-local requires.
+
+use std::collections::{HashMap, HashSet};
+
+use swc_ecma_ast as ast;
+use swc_ecma_visit::{Visit, VisitWith};
+
+/// Synthetic imports collect the target, but must not evaluate it before a
+/// conditional branch or a function actually calls `require`. A specifier with
+/// any unconditional occurrence keeps the existing eager/alias-adoption path.
+/// Use the AST: brace scanning misses concise arrows, unbraced branches, and
+/// short-circuit expressions. On a parse failure retain the existing scanner's
+/// function-local classification (some CJS sources need wrapping to parse).
+pub(super) fn deferred_require_specs(source: &str) -> HashSet<String> {
+    let Ok(module) = perry_parser::parse_typescript(source, "requires.cjs") else {
+        return super::extract_requires::function_local_specs(source);
+    };
+    let mut visitor = Requires::default();
+    module.visit_with(&mut visitor);
+    visitor
+        .sites
+        .into_iter()
+        .filter_map(|(specifier, deferred)| deferred.then_some(specifier))
+        .collect()
+}
+
+#[derive(Default)]
+struct Requires {
+    deferred: bool,
+    sites: HashMap<String, bool>,
+}
+
+impl Requires {
+    fn defer(&mut self, visit: impl FnOnce(&mut Self)) {
+        let previous = self.deferred;
+        self.deferred = true;
+        visit(self);
+        self.deferred = previous;
+    }
+}
+
+impl Visit for Requires {
+    fn visit_call_expr(&mut self, call: &ast::CallExpr) {
+        if let ast::Callee::Expr(callee) = &call.callee {
+            if matches!(callee.as_ref(), ast::Expr::Ident(name) if name.sym == *"require") {
+                if let [arg] = call.args.as_slice() {
+                    if arg.spread.is_none() {
+                        if let ast::Expr::Lit(ast::Lit::Str(specifier)) = arg.expr.as_ref() {
+                            self.sites
+                                .entry(specifier.value.to_string_lossy().into_owned())
+                                .and_modify(|deferred| *deferred &= self.deferred)
+                                .or_insert(self.deferred);
+                        }
+                    }
+                }
+            }
+        }
+        call.visit_children_with(self);
+    }
+
+    fn visit_function(&mut self, function: &ast::Function) {
+        function.decorators.visit_with(self);
+        self.defer(|visitor| {
+            function.params.visit_with(visitor);
+            function.body.visit_with(visitor);
+        });
+    }
+
+    fn visit_arrow_expr(&mut self, arrow: &ast::ArrowExpr) {
+        self.defer(|visitor| arrow.visit_children_with(visitor));
+    }
+
+    fn visit_constructor(&mut self, constructor: &ast::Constructor) {
+        self.defer(|visitor| constructor.visit_children_with(visitor));
+    }
+
+    fn visit_getter_prop(&mut self, getter: &ast::GetterProp) {
+        getter.key.visit_with(self);
+        self.defer(|visitor| getter.body.visit_with(visitor));
+    }
+
+    fn visit_setter_prop(&mut self, setter: &ast::SetterProp) {
+        setter.key.visit_with(self);
+        self.defer(|visitor| setter.body.visit_with(visitor));
+    }
+
+    fn visit_if_stmt(&mut self, stmt: &ast::IfStmt) {
+        stmt.test.visit_with(self);
+        self.defer(|visitor| {
+            stmt.cons.visit_with(visitor);
+            stmt.alt.visit_with(visitor);
+        });
+    }
+
+    fn visit_cond_expr(&mut self, expr: &ast::CondExpr) {
+        expr.test.visit_with(self);
+        self.defer(|visitor| {
+            expr.cons.visit_with(visitor);
+            expr.alt.visit_with(visitor);
+        });
+    }
+
+    fn visit_bin_expr(&mut self, expr: &ast::BinExpr) {
+        expr.left.visit_with(self);
+        if matches!(
+            expr.op,
+            ast::BinaryOp::LogicalAnd | ast::BinaryOp::LogicalOr | ast::BinaryOp::NullishCoalescing
+        ) {
+            self.defer(|visitor| expr.right.visit_with(visitor));
+        } else {
+            expr.right.visit_with(self);
+        }
+    }
+
+    fn visit_try_stmt(&mut self, stmt: &ast::TryStmt) {
+        // In particular, a throwing require must stay inside its try/catch.
+        self.defer(|visitor| stmt.visit_children_with(visitor));
+    }
+
+    fn visit_assign_expr(&mut self, expr: &ast::AssignExpr) {
+        expr.left.visit_with(self);
+        if matches!(
+            expr.op,
+            ast::AssignOp::AndAssign | ast::AssignOp::OrAssign | ast::AssignOp::NullishAssign
+        ) {
+            self.defer(|visitor| expr.right.visit_with(visitor));
+        } else {
+            expr.right.visit_with(self);
+        }
+    }
+
+    fn visit_switch_stmt(&mut self, stmt: &ast::SwitchStmt) {
+        stmt.discriminant.visit_with(self);
+        self.defer(|visitor| stmt.cases.visit_with(visitor));
+    }
+
+    fn visit_while_stmt(&mut self, stmt: &ast::WhileStmt) {
+        stmt.test.visit_with(self);
+        self.defer(|visitor| stmt.body.visit_with(visitor));
+    }
+
+    fn visit_do_while_stmt(&mut self, stmt: &ast::DoWhileStmt) {
+        // Both halves are conditional: the body can `break` or `return` before
+        // the test runs, so `do { break } while (require("dep"))` never
+        // evaluates the require in Node.
+        self.defer(|visitor| {
+            stmt.body.visit_with(visitor);
+            stmt.test.visit_with(visitor);
+        });
+    }
+
+    fn visit_for_stmt(&mut self, stmt: &ast::ForStmt) {
+        stmt.init.visit_with(self);
+        stmt.test.visit_with(self);
+        self.defer(|visitor| {
+            stmt.update.visit_with(visitor);
+            stmt.body.visit_with(visitor);
+        });
+    }
+
+    fn visit_for_in_stmt(&mut self, stmt: &ast::ForInStmt) {
+        stmt.right.visit_with(self);
+        self.defer(|visitor| {
+            stmt.left.visit_with(visitor);
+            stmt.body.visit_with(visitor);
+        });
+    }
+
+    fn visit_for_of_stmt(&mut self, stmt: &ast::ForOfStmt) {
+        stmt.right.visit_with(self);
+        self.defer(|visitor| {
+            stmt.left.visit_with(visitor);
+            stmt.body.visit_with(visitor);
+        });
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::deferred_require_specs;
+
+    #[test]
+    fn preserves_conditional_and_function_evaluation_boundaries() {
+        for source in [
+            "if (enabled) require('dep');",
+            "if (enabled) { const dep = require('dep'); }",
+            "enabled ? require('dep') : 0;",
+            "enabled && require('dep');",
+            "enabled || require('dep');",
+            "enabled ?? require('dep');",
+            "value &&= require('dep');",
+            "value ||= require('dep');",
+            "value ??= require('dep');",
+            "try { require('dep'); } catch (e) {}",
+            "switch (value) { case 1: require('dep'); }",
+            "while (enabled) require('dep');",
+            "do { break; } while (require('dep'));",
+            "do { require('dep'); } while (enabled);",
+            "for (; enabled;) require('dep');",
+            "for (const item of items) require('dep');",
+            "for (const key in object) require('dep');",
+            "module.exports = () => require('dep');",
+            "function load(dep = require('dep',)) { return dep; }",
+            "module.exports = {get value() { return require('dep'); }};",
+        ] {
+            assert!(deferred_require_specs(source).contains("dep"), "{source}");
+        }
+    }
+
+    #[test]
+    fn unconditional_occurrences_keep_existing_eager_classification() {
+        for source in [
+            "const dep = require('dep');",
+            "if (require('dep')) {}",
+            "require('dep') && enabled;",
+            "const x = require('dep') + 1;",
+            "value = require('dep');",
+            "{ require('dep'); }",
+            "if (enabled) require('dep'); require('dep');",
+            "require('dep'); module.exports = () => require('dep');",
+        ] {
+            assert!(!deferred_require_specs(source).contains("dep"), "{source}");
+        }
+    }
+
+    #[test]
+    fn ignores_comments_strings_and_member_calls() {
+        assert!(deferred_require_specs(
+            "// require('dep')\nconst text = \"require('dep')\";\nif (enabled) other.require('dep');"
+        ).is_empty());
+    }
+
+    #[test]
+    fn wrapping_keeps_conditional_aliases_and_exports_inside_the_body() {
+        let source = "class Unrelated {}\nif (enabled) {\nconst dep = require('dep');\nexports.value = require('dep');\nconsole.log(dep);\n}\n";
+        let wrapped =
+            super::super::wrap::wrap_commonjs(source, std::path::Path::new("/fixture/index.cjs"));
+        assert!(
+            wrapped.contains("import _lazyreq_0 from 'dep';"),
+            "{wrapped}"
+        );
+        assert!(wrapped.contains("const dep = require('dep');"), "{wrapped}");
+        assert!(!wrapped.contains("const dep = _lazyreq_0;"), "{wrapped}");
+        assert!(
+            wrapped.contains("export const value = _cjs.value;"),
+            "{wrapped}"
+        );
+        assert!(
+            !wrapped.contains("export { _lazyreq_0 as value };"),
+            "{wrapped}"
+        );
+        perry_parser::parse_typescript(&wrapped, "wrapped.cjs").unwrap();
+    }
+}

crates/perry/src/commands/compile/cjs_wrap/mod.rs

@@ -38,6 +38,7 @@
 //!     switching; deeper indirection is rare and gets the no-op fallback.
 
 pub(crate) mod detect;
+mod deferred_requires;
 mod extract_exports;
 mod extract_requires;
 mod hoist_classes;
@@ -52,6 +53,7 @@ mod preamble_canary_tests;
 
 // Cross-sibling helpers — siblings reach for these via `use super::*;`.
 use detect::is_js_reserved_word;
+use deferred_requires::deferred_require_specs;
 use extract_exports::{
     extract_exports_from_source, extract_named_exports_from_require,
     extract_object_literal_exports_from_require, extract_single_module_exports_assignment,
@@ -60,7 +62,7 @@ use extract_exports::{
 // #8547: the stdlib-link decision needs the literal `require()` specifiers.
 pub(crate) use extract_requires::extract_require_specifiers;
 use extract_requires::{
-    extract_export_star_specs, extract_require_aliases_with_ranges, function_local_specs,
+    extract_export_star_specs, extract_require_aliases_with_ranges,
     identifier_is_declared_binding, identifier_is_reassigned,
 };
 use hoist_classes::{

crates/perry/src/commands/compile/cjs_wrap/tests.rs

@@ -747,9 +747,8 @@ exports.spawn = function spawn() { return terminalCtor; };
         Some("windows"),
     );
     assert!(
-        wrapped.contains("import _req_0 from './windowsTerminal';")
-            || wrapped.contains("import terminalCtor from './windowsTerminal';"),
-        "expected live Windows require to stay hoisted, got:\n{}",
+        wrapped.contains("import _lazyreq_0 from './windowsTerminal';"),
+        "expected live Windows require to remain collected and initialize in its branch, got:\n{}",
         wrapped
     );
     assert!(