fix(cjs): expose live exports at CommonJS cycle re-entry

changelog.d/10178-cjs-cycle-exports.md

@@ -0,0 +1,12 @@
+Fix CommonJS cycle re-entry after `module.exports` is replaced, including
+esbuild's `__export` / `__toCommonJS` getter exports and semver's exported
+Comparator class. Partial publication now retains the module record, so the
+existing require adapter reads its current exports instead of the initial
+empty object. This adds no getter enumeration, copies, allocations, or extra
+publication calls to ordinary CommonJS initialization.
+
+Generated circular-dependency warnings check whether the property actually
+exists before warning, without invoking accessors. Regression coverage imports
+two esbuild-style CommonJS modules from ESM, checks the module views during
+cycle re-entry, calls through the cycle, and verifies live bindings and empty
+stderr. Related: #10178, #10107.

crates/perry-codegen/src/lower_call/native/native_runtime_branch.rs

@@ -109,8 +109,8 @@
                     &[(DOUBLE, &from), (DOUBLE, &specifier)],
                 ));
             }
-            // Next.js wall 54: publish a CJS module's partial exports before
-            // its body so same-thread recursive requires can observe them.
+            // Publish the CJS record before its body so same-thread recursive
+            // requires see its current exports, including replacements.
             "registerPathModulePartial" => {
                 let path = args.first().map_or_else(
                     || Ok(double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED))),

crates/perry-runtime/src/module_require.rs

@@ -440,10 +440,9 @@ fn registered_path_module_value(path: &str) -> Option<f64> {
         .map(f64::from_bits)
 }
 
-/// The registry holds whatever the CommonJS wrapper published: the module
-/// RECORD once the wrapper reaches its tail, or bare partial exports while a
-/// cycle is still initializing. Generated `require` sites want the exports in
-/// both cases.
+/// Generated wrappers publish the module RECORD at both the partial and final
+/// boundaries. Read its current exports so replacements made before a cycle
+/// re-entry are visible. Bare values remain supported for other publishers.
 fn path_module_exports(bits: u64) -> f64 {
     let value = f64::from_bits(bits);
     cjs_record_exports(value).unwrap_or(value)
@@ -863,7 +862,7 @@ pub unsafe extern "C" fn js_register_path_init(path_ptr: *const u8, path_len: i6
     }
 }
 
-/// Codegen FFI: publish a CommonJS module's initial `exports` object before
+/// Codegen FFI: publish a CommonJS module's record before
 /// executing its body. This is visible only to recursive loads by the owning
 /// thread; concurrent callers wait for [`js_register_path_module`] and the
 /// generated initializer to complete.

crates/perry-runtime/src/module_require/path_registry.rs

@@ -183,7 +183,8 @@ impl PathModuleRegistry {
         true
     }
 
-    /// Publish the initial CommonJS `exports` object before the wrapper body.
+    /// Publish the CommonJS module record before the wrapper body. The exports
+    /// adapter unwraps its current `.exports` on each read, including cycles.
     /// Only same-thread recursive loads may observe it; unrelated waiters stay
     /// parked while the status is `Initializing`.
     pub(super) fn register_partial_exports(&self, key: String, exports: u64) -> bool {

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

@@ -204,6 +204,13 @@ fn path_module_wrap_publishes_partial_then_final_exports_and_tracks_undefined()
         .rfind("__perry_register_path_module(")
         .expect("CJS wrapper must publish its final module.exports value");
     assert!(partial < body && body < final_publish, "{wrapped}");
+    assert!(
+        wrapped.contains(&format!(
+            "__perry_register_path_module_partial({:?}, __cjs_module);",
+            path.to_string_lossy()
+        )),
+        "cycle readers must follow module.exports replacements\n{wrapped}"
+    );
     // #8040: both the value lookup and the presence probe must consult the
     // SAME resolved specifier. A computed relative request is joined against
     // the module's directory before either call (`__perry_path_spec`), so a

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

@@ -467,16 +467,21 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset(
                             cyclic_missing_property_names(source, source_path, spec, target)
                                 .into_iter()
                                 .map(|property| {
+                                    // #10178: this scan only nominates possible
+                                    // misses. __export helpers and class statics
+                                    // can already be present on a replacement
+                                    // module.exports. Check the returned value
+                                    // without invoking the exported getter.
                                     format!(
-                                        "if (childBefore && childBefore.loaded === false) globalThis.process?.emitWarning?.(\"Accessing non-existent property '{property}' of module exports inside circular dependency\"); "
+                                        "if (childBefore && childBefore.loaded === false && required != null && (typeof required === 'object' || typeof required === 'function') && !('{property}' in required)) globalThis.process?.emitWarning?.(\"Accessing non-existent property '{property}' of module exports inside circular dependency\"); "
                                     )
                                 })
                                 .collect::<String>()
                         } else {
                             String::new()
                         };
                         format!(
-                            "const childBefore = require.cache[{path:?}]; {warnings}globalThis.__perry_cjs_pending_parent = module; let required; try {{ required = __perry_require_path_module({path:?}); }} finally {{ globalThis.__perry_cjs_pending_parent = undefined; }} {link_child}return required;",
+                            "const childBefore = require.cache[{path:?}]; globalThis.__perry_cjs_pending_parent = module; let required; try {{ required = __perry_require_path_module({path:?}); }} finally {{ globalThis.__perry_cjs_pending_parent = undefined; }} {warnings}{link_child}return required;",
                             path = target.to_string_lossy(),
                         )
                     })
@@ -979,10 +984,12 @@ pub(in crate::commands::compile) fn wrap_commonjs_with_body_offset(
     // Node populates `module.parent` before the body evaluates, so link it
     // here rather than at the tail's registry publication.
     __perry_link_path_module_parent(__cjs_module);
-    // Publish the initial exports before user code. The runtime exposes them
-    // only to same-thread recursive loads; concurrent first callers wait for
-    // the final record registration at the bottom of this wrapper.
-    __perry_register_path_module_partial({module_path_literal}, __cjs_module.exports);
+    // Publish the record before user code, just as at final publication.
+    // #10178: esbuild replaces module.exports before requiring its peer.
+    // Holding the initial empty object loses that replacement at re-entry;
+    // the runtime's existing record unwrap reads the current exports instead.
+    // This needs no getter enumeration, copying, or extra publication calls.
+    __perry_register_path_module_partial({module_path_literal}, __cjs_module);
     var module = __cjs_module;
     var exports = __cjs_module.exports;
     const __perry_cjs_base_require = __perry_cjs_create_require({module_filename_literal});

crates/perry/tests/source_graph_export_regressions.rs

@@ -9,6 +9,9 @@ use std::sync::Once;
 #[path = "source_graph_export_regressions/issue_10153.rs"]
 mod issue_10153;
 
+#[path = "source_graph_export_regressions/issue_10178.rs"]
+mod issue_10178;
+
 const GC_ENV_OVERRIDES: &[&str] = &[
     "PERRY_GEN_GC",
     "PERRY_GC_SCAVENGE",
@@ -90,6 +93,10 @@ fn write(dir: &Path, name: &str, source: &str) {
 }
 
 fn compile_and_run(dir: &Path, entry: &str) -> String {
+    String::from_utf8_lossy(&compile_and_run_output(dir, entry).stdout).into_owned()
+}
+
+fn compile_and_run_output(dir: &Path, entry: &str) -> std::process::Output {
     let output = dir.join("main_bin");
     let compile = Command::new(perry_bin())
         .current_dir(dir)
@@ -119,7 +126,7 @@ fn compile_and_run(dir: &Path, entry: &str) -> String {
         String::from_utf8_lossy(&run.stdout),
         String::from_utf8_lossy(&run.stderr)
     );
-    String::from_utf8_lossy(&run.stdout).into_owned()
+    run
 }
 
 fn compile_and_run_with_llvm_trace(dir: &Path, entry: &str) -> (String, String) {

crates/perry/tests/source_graph_export_regressions/issue_10178.rs

@@ -0,0 +1,97 @@
+//! A CJS cycle must observe module.exports replacements before init finishes.
+
+use super::{compile_and_run_output, write};
+
+#[test]
+fn esbuild_getters_are_callable_and_live_through_a_require_cycle() {
+    let dir = tempfile::tempdir().unwrap();
+    write(
+        dir.path(),
+        "token.cjs",
+        include_str!("../../../../test-files/cjs_esbuild_cycle/token.cjs"),
+    );
+    write(
+        dir.path(),
+        "consumer.cjs",
+        include_str!("../../../../test-files/cjs_esbuild_cycle/consumer.cjs"),
+    );
+    write(
+        dir.path(),
+        "main.mjs",
+        &include_str!("../../../../test-files/test_cjs_esbuild_cycle.ts")
+            .replace("./cjs_esbuild_cycle/token.cjs", "./token.cjs"),
+    );
+    let run = compile_and_run_output(dir.path(), "main.mjs");
+    assert_eq!(
+        String::from_utf8_lossy(&run.stdout),
+        "function:function:true\n0\n42\n52\n2\n"
+    );
+    assert!(
+        run.stderr.is_empty(),
+        "cycle emitted a warning: {}",
+        String::from_utf8_lossy(&run.stderr)
+    );
+}
+
+#[test]
+fn class_static_getter_is_visible_in_a_comparator_first_cycle() {
+    let dir = tempfile::tempdir().unwrap();
+    write(
+        dir.path(),
+        "comparator.cjs",
+        "const ANY = Symbol('ANY');\n\
+         class Comparator { static get ANY() { return ANY; } }\n\
+         module.exports = Comparator;\n\
+         const Range = require('./range.cjs');\n\
+         Comparator.read = function () { return Range.read(); };\n",
+    );
+    write(
+        dir.path(),
+        "range.cjs",
+        "class Range { static read() { return Comparator.ANY; } }\n\
+         module.exports = Range;\n\
+         const Comparator = require('./comparator.cjs');\n",
+    );
+    write(
+        dir.path(),
+        "main.mjs",
+        "import Comparator from './comparator.cjs';\n\
+         console.log(typeof Comparator.read());\n\
+         console.log(Comparator.read() === Comparator.ANY);\n",
+    );
+    let run = compile_and_run_output(dir.path(), "main.mjs");
+    assert_eq!(String::from_utf8_lossy(&run.stdout), "symbol\ntrue\n");
+    assert!(
+        run.stderr.is_empty(),
+        "{}",
+        String::from_utf8_lossy(&run.stderr)
+    );
+}
+
+#[test]
+fn missing_property_in_a_cycle_still_warns() {
+    let dir = tempfile::tempdir().unwrap();
+    write(
+        dir.path(),
+        "a.cjs",
+        "exports.before = true;\n\
+         const b = require('./b.cjs');\n\
+         exports.seen = b.seen;\n\
+         exports.after = true;\n",
+    );
+    write(
+        dir.path(),
+        "b.cjs",
+        "const a = require('./a.cjs');\nexports.seen = a.after;\n",
+    );
+    write(
+        dir.path(),
+        "main.mjs",
+        "import a from './a.cjs';\nconsole.log(a.seen);\n",
+    );
+    let run = compile_and_run_output(dir.path(), "main.mjs");
+    assert_eq!(String::from_utf8_lossy(&run.stdout), "undefined\n");
+    assert!(String::from_utf8_lossy(&run.stderr).contains(
+        "Accessing non-existent property 'after' of module exports inside circular dependency"
+    ));
+}

test-files/cjs_esbuild_cycle/consumer.cjs

@@ -0,0 +1,24 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from) => {
+  for (let key of __getOwnPropNames(from))
+    __defProp(to, key, { get: () => from[key], enumerable: true });
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var consumer_exports = {};
+__export(consumer_exports, { read: () => read, cycleView: () => cycleView });
+module.exports = __toCommonJS(consumer_exports);
+var token = require("./token.cjs");
+// Inspect both views while token's wrapper is still running. Neither
+// descriptor inspection nor retaining the namespace should invoke a getter.
+var record = globalThis.__esbuildCycleRecord;
+var cachedGetter = record ? typeof Object.getOwnPropertyDescriptor(record.exports, "getValue")?.get : "missing-record";
+var requiredGetter = token ? typeof Object.getOwnPropertyDescriptor(token, "getValue")?.get : "missing-exports";
+var sameExports = record ? token === record.exports : false;
+function read() { return (0, token.getValue)(); }
+function cycleView() { return cachedGetter + ":" + requiredGetter + ":" + sameExports; }

test-files/cjs_esbuild_cycle/token.cjs

@@ -0,0 +1,30 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from) => {
+  for (let key of __getOwnPropNames(from))
+    __defProp(to, key, { get: () => from[key], enumerable: true });
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var token_exports = {};
+var getterReads = 0;
+__export(token_exports, {
+  getValue: () => (getterReads++, getValue),
+  callThroughCycle: () => callThroughCycle,
+  update: () => update,
+  cycleView: () => cycleView,
+  reads: () => reads
+});
+module.exports = __toCommonJS(token_exports);
+globalThis.__esbuildCycleRecord = module;
+var consumer = require("./consumer.cjs");
+var offset = 40;
+function getValue() { return offset + 2; }
+function callThroughCycle() { return (0, consumer.read)(); }
+function update() { getValue = function () { return 52; }; }
+function cycleView() { return consumer.cycleView(); }
+function reads() { return getterReads; }

test-files/test_cjs_esbuild_cycle.ts

@@ -0,0 +1,12 @@
+// #10178: an ESM import enters an esbuild CommonJS require cycle.
+import process from "node:process";
+import token from "./cjs_esbuild_cycle/token.cjs";
+
+if (typeof process.emitWarning !== "function") throw new Error("missing warning support");
+console.log(token.cycleView());
+console.log(token.reads());
+delete globalThis.__esbuildCycleRecord;
+console.log(token.callThroughCycle());
+token.update();
+console.log(token.callThroughCycle());
+console.log(token.reads());