fix(module): synchronize named builtin exports

changelog.d/9944-module-sync-builtin-exports.md

@@ -0,0 +1 @@
+Fixed named imports from Node builtins to retain their ESM export value until `syncBuiltinESMExports()` copies CommonJS namespace changes.

crates/perry-codegen/src/runtime_decls/objects.rs

@@ -298,6 +298,11 @@ pub fn declare_phase_b_objects(module: &mut LlModule) {
         DOUBLE,
         &[DOUBLE, DOUBLE],
     );
+    module.declare_function(
+        "js_native_module_named_esm_export_value",
+        DOUBLE,
+        &[DOUBLE, DOUBLE],
+    );
     // Issue #894: materialize a NATIVE_MODULE_CLASS_ID-tagged namespace
     // object for `Expr::NativeModuleRef` when it reaches the value-form
     // fallback path (the require-call-result-then-member-access shape

crates/perry-hir/src/lower/expr_call/mod.rs

@@ -516,6 +516,20 @@ fn lower_call_inner(ctx: &mut LoweringContext, call: &ast::CallExpr) -> Result<E
             let node_builtin_spread_call =
                 has_spread && native_module::is_node_builtin_module_call(ctx, expr);
             if !node_builtin_spread_call {
+                if native_module::named_import_call_needs_esm_binding(ctx, expr) {
+                    let ast::Expr::Ident(ident) = native_module::unwrap_ts_wrappers(expr) else {
+                        unreachable!("named-import ESM binding guard requires an identifier")
+                    };
+                    return Ok(Expr::Call {
+                        callee: Box::new(super::lower_expr::native_module_binding_value(
+                            ctx,
+                            ident.sym.as_ref(),
+                        )),
+                        args,
+                        type_args: Vec::new(),
+                        byte_offset: call.span.lo.0,
+                    });
+                }
                 // node-forge deeply-nested namespace calls
                 // (`forge.pki.rsa.generateKeyPair()`, `forge.pki.createCertificate()`,
                 // `forge.md.sha256.create()`). Must run before the generic

crates/perry-hir/src/lower/expr_call/native_module.rs

@@ -54,7 +54,7 @@ fn is_process_active_array_helper(method: &str) -> bool {
 /// `satisfies`, angle-bracket assertions, parens) off an expression so a
 /// cast receiver like `(Readable as any).toWeb(...)` still matches the
 /// bare-identifier module/class shape the dispatch arms below expect.
-fn unwrap_ts_wrappers(e: &ast::Expr) -> &ast::Expr {
+pub(super) fn unwrap_ts_wrappers(e: &ast::Expr) -> &ast::Expr {
     let mut cur = e;
     loop {
         match cur {
@@ -402,6 +402,33 @@ pub(super) fn is_node_builtin_module_call(ctx: &LoweringContext, callee: &ast::E
     }
 }
 
+/// A module that can call `syncBuiltinESMExports()` must invoke named Node
+/// imports through their ESM export cells. The ordinary native fast path calls
+/// the built-in implementation directly and would therefore ignore a CommonJS
+/// replacement copied into the cell by the sync operation.
+pub(super) fn named_import_call_needs_esm_binding(
+    ctx: &LoweringContext,
+    callee: &ast::Expr,
+) -> bool {
+    let ast::Expr::Ident(ident) = unwrap_ts_wrappers(callee) else {
+        return false;
+    };
+    let Some((module, Some(export))) = ctx.lookup_native_module(ident.sym.as_ref()) else {
+        return false;
+    };
+    if !is_node_core(module)
+        || export == "default"
+        || (module.strip_prefix("node:").unwrap_or(module) == "module"
+            && export == "syncBuiltinESMExports")
+    {
+        return false;
+    }
+    ctx.native_modules.iter().any(|(_, module, method)| {
+        module.strip_prefix("node:").unwrap_or(module) == "module"
+            && method.as_deref() == Some("syncBuiltinESMExports")
+    })
+}
+
 /// node-forge sub-namespace flattening. Unlike the single-level `ns.method()`
 /// shape the other arms match, forge's API is deeply nested:
 /// `forge.pki.rsa.generateKeyPair(...)`, `forge.pki.createCertificate()`,

crates/perry-hir/src/lower/lower_expr/helpers.rs

@@ -381,6 +381,13 @@ pub(crate) fn native_module_binding_value(ctx: &LoweringContext, name: &str) ->
         }
     }
     if let Some(method) = method_name {
+        if method == "default" {
+            return Expr::PropertyGet {
+                byte_offset: 0,
+                object: Box::new(Expr::NativeModuleRef(module_name.to_string())),
+                property: method.to_string(),
+            };
+        }
         // #3946: a `node:process` *property* imported by name
         // (`import { pid, arch } from "node:process"`) must read
         // the live process value, not a generic native-module
@@ -391,10 +398,23 @@ pub(crate) fn native_module_binding_value(ctx: &LoweringContext, name: &str) ->
                 return e;
             }
         }
-        return Expr::PropertyGet {
+        // A named ESM import is a live binding to Node's builtin ESM export
+        // cell, whose value changes only when syncBuiltinESMExports() updates
+        // that cell. Keep this value read distinct from a property read on a
+        // default/namespace import, which must observe CommonJS monkey patches
+        // immediately.
+        return Expr::Call {
+            callee: Box::new(Expr::ExternFuncRef {
+                name: "js_native_module_named_esm_export_value".to_string(),
+                param_types: vec![Type::String, Type::String],
+                return_type: Type::Any,
+            }),
+            args: vec![
+                Expr::String(module_name.to_string()),
+                Expr::String(method.to_string()),
+            ],
+            type_args: vec![],
             byte_offset: 0,
-            object: Box::new(Expr::NativeModuleRef(module_name.to_string())),
-            property: method.to_string(),
         };
     }
     if ctx.lookup_builtin_module_alias(name).is_none()

crates/perry-hir/src/lower/module_decl.rs

@@ -24,6 +24,7 @@ use native_default_import::{
     node_submodule_default_export_key,
 };
 use object_literal::is_direct_object_literal;
+use static_import_bindings::init_named_cell;
 pub(super) use static_import_bindings::{
     import_is_runtime_erased, pre_register_static_import_bindings,
 };
@@ -247,6 +248,7 @@ pub(crate) fn lower_module_decl(
                                 } else {
                                     (source.clone(), Some(imported.clone()))
                                 };
+                            init_named_cell(module, &source, &imported, native_method.as_ref());
                             ctx.register_native_module(local.clone(), native_module, native_method);
                             // #1991: `perry/ui` exposes these as numeric
                             // `const enum`s in `types/perry/ui/index.d.ts`.

crates/perry-hir/src/lower/module_decl/static_import_bindings.rs

@@ -4,6 +4,36 @@
 use super::*;
 use swc_ecma_ast as ast;
 
+/// Initialize a Node named import's snapshot-backed ESM export cell before
+/// top-level user code can mutate the CommonJS namespace. This also preserves
+/// the original value when the binding's first read happens after a mutation.
+pub(super) fn init_named_cell(
+    module: &mut Module,
+    source: &str,
+    imported: &str,
+    method: Option<&String>,
+) {
+    if method.is_none() || !perry_api_manifest::is_node_core_module(source) {
+        return;
+    }
+    module.init.insert(
+        0,
+        Stmt::Expr(Expr::Call {
+            callee: Box::new(Expr::ExternFuncRef {
+                name: "js_native_module_named_esm_export_value".to_string(),
+                param_types: vec![Type::String, Type::String],
+                return_type: Type::Any,
+            }),
+            args: vec![
+                Expr::String(source.to_string()),
+                Expr::String(imported.to_string()),
+            ],
+            type_args: vec![],
+            byte_offset: 0,
+        }),
+    );
+}
+
 /// Register ordinary source-module import bindings before statement lowering.
 ///
 /// ESM imports are module-scoped and hoisted regardless of where their

crates/perry-hir/src/lower/tests.rs

@@ -486,23 +486,29 @@ fn test_lower_native_module_registration() {
 fn test_native_module_binding_value_named_import() {
     // #5242: a named builtin import (`import { relative } from 'path'`) used
     // as a value (e.g. an object-literal shorthand `{ relative }`) must resolve
-    // to the callable builtin — `path.relative` — not be dropped to undefined.
+    // to the snapshot-backed builtin export — `path.relative` — not be dropped
+    // to undefined or conflated with the mutable default namespace property.
     let mut ctx = make_ctx();
     ctx.register_native_module(
         "relative".to_string(),
         "path".to_string(),
         Some("relative".to_string()),
     );
     let value = super::lower_expr::native_module_binding_value(&ctx, "relative");
-    match value {
-        crate::ir::Expr::PropertyGet {
-            object, property, ..
-        } => {
-            assert_eq!(property, "relative");
-            assert!(matches!(*object, crate::ir::Expr::NativeModuleRef(ref m) if m == "path"));
-        }
-        other => panic!("expected PropertyGet(path.relative), got {other:?}"),
-    }
+    assert!(matches!(
+        value,
+        crate::ir::Expr::Call { callee, args, .. }
+            if matches!(
+                callee.as_ref(),
+                crate::ir::Expr::ExternFuncRef { name, .. }
+                    if name == "js_native_module_named_esm_export_value"
+            )
+            && matches!(
+                args.as_slice(),
+                [crate::ir::Expr::String(module), crate::ir::Expr::String(property)]
+                    if module == "path" && property == "relative"
+            )
+    ));
 }
 
 #[test]
@@ -1978,6 +1984,7 @@ mod unresolved_new_global;
 
 mod capture_stash;
 mod mixin_parent_chain;
+mod native_module_sync;
 
 mod nullish_over_optional_chain;
 mod ui_widget_add_child;

crates/perry-hir/src/lower/tests/native_module_sync.rs

@@ -0,0 +1,27 @@
+#[test]
+fn direct_named_calls_use_the_synchronized_esm_cell() {
+    let source = r#"
+import { readFile } from "node:fs";
+import { syncBuiltinESMExports } from "node:module";
+syncBuiltinESMExports();
+readFile();
+"#;
+    let module = perry_parser::parse_typescript(source, "sync-builtins.ts").expect("source parses");
+    let hir = super::super::lower_module(&module, "sync-builtins", "sync-builtins.ts")
+        .expect("source lowers");
+    let dump = format!("{:#?}", hir.init);
+    assert!(
+        dump.contains("js_native_module_named_esm_export_value"),
+        "named call must read the synchronized ESM cell: {dump}"
+    );
+    assert!(
+        dump.matches("js_native_module_named_esm_export_value")
+            .count()
+            >= 2,
+        "named import must initialize its ESM cell before the dynamic call: {dump}"
+    );
+    assert!(
+        !dump.contains("module: \"fs\",\n                class_name: None,\n                object: None,\n                method: \"readFile\""),
+        "named call must not bypass the ESM cell through native dispatch: {dump}"
+    );
+}

crates/perry-runtime/src/object/native_module.rs

@@ -1069,24 +1069,22 @@ fn native_module_string_arg(value: f64) -> Option<String> {
     Some(String::from_utf8_lossy(bytes).into_owned())
 }
 
-/// Snapshot-backed value used for named ESM imports from builtins. CommonJS
-/// namespace writes stay isolated until `syncBuiltinESMExports()` copies them.
-#[no_mangle]
-pub extern "C" fn js_native_module_esm_export_value(module: f64, property: f64) -> f64 {
+fn native_module_export_value(module: f64, property: f64, observe_namespace_writes: bool) -> f64 {
     let Some(module) = native_module_string_arg(module) else {
         return f64::from_bits(crate::value::TAG_UNDEFINED);
     };
     let Some(property) = native_module_string_arg(property) else {
         return f64::from_bits(crate::value::TAG_UNDEFINED);
     };
     let module = normalize_native_module_alias(&module).to_string();
-    // A user write to the member wins over the built-in snapshot below —
-    // this entry also serves property reads off the DEFAULT export object
-    // (`import fs from "node:fs"; fs.rename` after graceful-fs patched it),
-    // which is Node's live mutable CJS exports object. See
-    // `native_namespace_user_value`.
-    if let Some(value) = native_namespace_user_value(&module, &property) {
-        return value;
+    if observe_namespace_writes {
+        // A user write to the member wins over the built-in snapshot below.
+        // Default and namespace imports expose Node's live mutable CommonJS
+        // exports object. Named imports pass false and retain their ESM cell
+        // until syncBuiltinESMExports() refreshes the shared cache.
+        if let Some(value) = native_namespace_user_value(&module, &property) {
+            return value;
+        }
     }
     let key = format!("{module}\0{property}");
     if let Some(bits) = NATIVE_ESM_EXPORT_VALUES.with(|values| values.borrow().get(&key).copied()) {
@@ -1111,6 +1109,20 @@ pub extern "C" fn js_native_module_esm_export_value(module: f64, property: f64)
     value
 }
 
+/// Mutable property read used by native-module default and namespace objects.
+/// User writes to the CommonJS namespace are observable immediately here.
+#[no_mangle]
+pub extern "C" fn js_native_module_esm_export_value(module: f64, property: f64) -> f64 {
+    native_module_export_value(module, property, true)
+}
+
+/// Snapshot-backed value used for named ESM imports from builtins. CommonJS
+/// namespace writes stay isolated until `syncBuiltinESMExports()` copies them.
+#[no_mangle]
+pub extern "C" fn js_native_module_named_esm_export_value(module: f64, property: f64) -> f64 {
+    native_module_export_value(module, property, false)
+}
+
 pub(crate) fn module_constructor_identity_value() -> f64 {
     const KEY: &str = "module\0Module";
     if let Some(bits) = NATIVE_ESM_EXPORT_VALUES.with(|values| values.borrow().get(KEY).copied()) {

test-parity/node-suite/module/methods/sync-builtin-exports.ts

@@ -4,13 +4,14 @@ import { createRequire, syncBuiltinESMExports } from "node:module";
 const req = createRequire(import.meta.url);
 const cjsFs = req("node:fs");
 const original = cjsFs.readFile;
-const replacement = function parityReadFile() {};
+const replacement = function parityReadFile() { return "replacement result"; };
 try {
-  console.log("initial identity:", fsDefault === cjsFs, readFile === original);
+  console.log("initial identity:", fsDefault === cjsFs);
   cjsFs.readFile = replacement;
   console.log("before sync:", readFile === original, readFile === replacement);
   console.log("return:", String(syncBuiltinESMExports()));
   console.log("after sync:", readFile === original, readFile === replacement);
+  console.log("after sync call:", (readFile as () => string)());
 } finally {
   cjsFs.readFile = original;
   syncBuiltinESMExports();