Sorted property walk: list a module namespace by name and skip an export that is not set yet

src/jsc/bindings/bindings.cpp

@@ -6277,6 +6277,16 @@ extern "C" [[ZIG_EXPORT(nothrow)]] bool JSC__isBigIntInInt64Range(JSC::EncodedJS
     return high == JSBigInt::ComparisonResult::LessThan || high == JSBigInt::ComparisonResult::Equal;
 }
 
+// Listing only the enumerable keys of a module namespace reads every export, as Object.keys() does, and throws for one that is not initialized yet.
+static ALWAYS_INLINE bool listsEnumerableKeysByReadingThem(JSC::JSObject* object)
+{
+    JSC::JSType type = object->type();
+    if (type == JSC::ModuleNamespaceObjectType)
+        return true;
+    // The namespace of the ahead-of-time run time is an ObjectType cell. A compiled object has the same flag under another JSType, so it never makes the call.
+    return type == JSC::ObjectType && JSC::TypeInfo::overridesGetOwnPropertySlot(object->inlineTypeFlags()) && JSC::AOT::isModuleNamespace(object);
+}
+
 static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlobalObject* globalObject, void* arg2, void (*iter)(JSC::JSGlobalObject* arg0, void* ctx, EncodedSlice* arg2, JSC::EncodedJSValue JSValue3, bool isSymbol, bool isPrivateSymbol), bool callGetters, bool withNonEnumerable = false)
 {
     JSC::JSValue value = JSC::JSValue::decode(JSValue0);
@@ -6288,14 +6298,16 @@ static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlob
     auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
 
     JSC::PropertyNameArrayBuilder properties(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
+    // Asked for every key, a namespace reads no export (that of a mocked module still asks its mock). The loop below drops the non-enumerable keys and skips an export whose read throws.
+    const bool isNamespace = !withNonEnumerable && listsEnumerableKeysByReadingThem(object);
     {
 
         JSC::AOT::PropertyWalk::materializeLazyProperties(vm, object);
         // withNonEnumerable reproduces older Bun output. Skipping the method table prints a module namespace as `Module {}`.
         if (withNonEnumerable && !JSC::AOT::PropertyWalk::isCompiledObject(object))
             JSC::JSObject::getOwnPropertyNames(object, globalObject, properties, DontEnumPropertiesMode::Include);
         else
-            object->methodTable()->getOwnPropertyNames(object, globalObject, properties, withNonEnumerable ? DontEnumPropertiesMode::Include : DontEnumPropertiesMode::Exclude);
+            object->methodTable()->getOwnPropertyNames(object, globalObject, properties, withNonEnumerable || isNamespace ? DontEnumPropertiesMode::Include : DontEnumPropertiesMode::Exclude);
         RETURN_IF_EXCEPTION(scope, );
     }
 
@@ -6317,7 +6329,7 @@ static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlob
         if (!hasProperty) {
             continue;
         }
-        if ((slot.attributes() & PropertyAttribute::DontEnum) != 0 && (property == vm.propertyNames->underscoreProto || property == vm.propertyNames->toStringTagSymbol))
+        if ((slot.attributes() & PropertyAttribute::DontEnum) != 0 && (isNamespace || property == vm.propertyNames->underscoreProto || property == vm.propertyNames->toStringTagSymbol))
             continue;
 
         JSC::JSValue propertyValue = jsUndefined();

test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/earlier.mjs

@@ -0,0 +1,5 @@
+import * as later from "./later.mjs";
+
+console.log(Bun.inspect(later, { sorted: true, compact: true }));
+console.log(Bun.inspect({ later, list: [later] }, { sorted: true, compact: true }));
+console.log(Bun.inspect(later, { compact: true }));

test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/later.mjs

@@ -0,0 +1,3 @@
+import "./earlier.mjs";
+export let a = 1;
+export function f() {}

test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/main.mjs

@@ -0,0 +1,4 @@
+// Two modules import each other, and the one that runs first prints the namespace object of the other with sorted keys, while its `let` is still in its dead zone.
+// The print lists the names without a read of that export and leaves it out, as the print without sorted keys does.
+import "./later.mjs";
+console.log("end");

test/js/bun/console/bun-inspect-table.test.ts

@@ -1,5 +1,7 @@
 import { inspect } from "bun";
 import { describe, expect, it, test } from "bun:test";
+import { tempDir } from "harness";
+import { join } from "path";
 
 const inputs = [
   { a: 1, b: 2 },
@@ -74,6 +76,25 @@ describe("inspect.table", () => {
     expect(out).toContain("[ [Array ...] ]");
     expect(out).toContain("Map(1) { 1: [Map ...] }");
   });
+
+  it("with sorted, a cell prints a module namespace without an export that is not set yet", async () => {
+    // The module throws halfway through its body, so `later` stays in its temporal dead zone.
+    using dir = tempDir("inspect-table-namespace-unset-export", {
+      "stops.mjs": `
+        import * as self from "./stops.mjs";
+        export function hoisted() {}
+        (() => {
+          throw Object.assign(new Error("stops here"), { namespace: self });
+        })();
+        export const later = 1;
+      `,
+    });
+    const namespace = await import(join(String(dir), "stops.mjs")).catch(error => error.namespace);
+    const rows = [{ cell: namespace }];
+    const sorted = inspect.table(rows, { colors: false, sorted: true } as Bun.BunInspectOptions);
+    expect(sorted).toContain("│ 0 │ Module { hoisted: [Function: hoisted] } │");
+    expect(sorted).toBe(inspect.table(rows, { colors: false }));
+  });
 });
 
 describe("inspect.table (ansi)", () => {

test/js/bun/test/expect-extend.test.js

@@ -8,6 +8,7 @@
  */
 
 import { bunEnv, bunExe, tempDir, withoutAggressiveGC } from "harness";
+import { join } from "node:path";
 import test_interop from "./test-interop.js";
 var { isBun, expect, describe, test, it } = await test_interop();
 
@@ -745,6 +746,32 @@ describe("MatcherContext", () => {
       expect(() => utils.diff({}, value)).toThrow(thrown);
     });
 
+    test("diff prints a module namespace without an export that is not set yet", async () => {
+      // The module throws halfway through its body, so `later` stays in its temporal dead zone.
+      using dir = tempDir("matcher-utils-diff-namespace", {
+        "stops.mjs": `
+          import * as self from "./stops.mjs";
+          export function hoisted() {}
+          (() => {
+            throw Object.assign(new Error("stops here"), { namespace: self });
+          })();
+          export const later = 1;
+        `,
+      });
+      const namespace = await import(join(String(dir), "stops.mjs")).catch(error => error.namespace);
+      const marked = Bun.stripANSI(utils.diff({ a: 1 }, { a: namespace }))
+        .split("\n")
+        .filter(line => /^[-+]/.test(line));
+      expect(marked).toEqual([
+        '-   "a": 1,',
+        '+   "a": Module Module {',
+        '+     "hoisted": [Function: hoisted],',
+        "+   },",
+        "- Expected  - 1",
+        "+ Received  + 3",
+      ]);
+    });
+
     // Expected values measured on jest-matcher-utils 30.5.1.
     test.each([
       [undefined, "Received has value: <undefined>"],

test/js/bun/test/expect.test.js

@@ -2224,6 +2224,50 @@ describe("expect()", () => {
     expect([1, 2, 3, 4]).not.toEqual([1, 2, 3]);
   });
 
+  test_skipIf(!isBun)("toEqual() - the diff prints a namespace without an export that is not set yet", async () => {
+    const { tempDir } = require("harness");
+    const { join } = require("node:path");
+    // The module throws halfway through its body, so `later` stays in its temporal dead zone.
+    using dir = tempDir("expect-namespace-unset-export", {
+      "stops.mjs": `
+        import * as self from "./stops.mjs";
+        export function hoisted() {}
+        (() => {
+          throw Object.assign(new Error("stops here"), { namespace: self });
+        })();
+        export const later = 1;
+      `,
+    });
+    const namespace = await import(join(String(dir), "stops.mjs")).catch(error => error.namespace);
+    const firstLineAndDiff = (/** @type {() => void} */ assertion) => {
+      try {
+        assertion();
+      } catch (error) {
+        const lines = Bun.stripANSI(ANY(error).message).split("\n");
+        return [lines[0], ...lines.filter(line => /^[-+] /.test(line))];
+      }
+      return [];
+    };
+    expect(firstLineAndDiff(() => expect({ a: namespace }).toEqual({ a: 1 }))).toEqual([
+      "expect(received).toEqual(expected)",
+      '-   "a": 1,',
+      '+   "a": Module Module {',
+      '+     "hoisted": [Function: hoisted],',
+      "+   },",
+      "- Expected  - 1",
+      "+ Received  + 3",
+    ]);
+    expect(firstLineAndDiff(() => expect({ a: 1 }).toEqual({ a: expect.objectContaining(namespace) }))).toEqual([
+      "expect(received).toEqual(expected)",
+      '-   "a": ObjectContaining Module Module {',
+      '-     "hoisted": [Function: hoisted],',
+      "-   },",
+      '+   "a": 1,',
+      "- Expected  - 3",
+      "+ Received  + 1",
+    ]);
+  });
+
   test("toEqual() - private class fields", () => {
     class A {
       #three = 3;

test/js/bun/test/snapshot-tests/bun-snapshots.test.ts

@@ -1,4 +1,6 @@
 import { describe, expect, it, test } from "bun:test";
+import { tempDir } from "harness";
+import { join } from "path";
 
 test("it will create a snapshot file if it doesn't exist", () => {
   expect({ a: { b: { c: false } }, c: 2, jkfje: 99238 }).toMatchSnapshot({ a: { b: { c: expect.any(Boolean) } } });
@@ -33,6 +35,28 @@ test("ArrayBuffer values are serialized like typed arrays", () => {
   `);
 });
 
+test("a module namespace is serialized without an export that is not set yet", async () => {
+  // The module throws halfway through its body, so `later` stays in its temporal dead zone.
+  using dir = tempDir("snapshot-namespace-unset-export", {
+    "stops.mjs": `
+      import * as self from "./stops.mjs";
+      export function hoisted() {}
+      (() => {
+        throw Object.assign(new Error("stops here"), { namespace: self });
+      })();
+      export const later = 1;
+    `,
+  });
+  const namespace = await import(join(String(dir), "stops.mjs")).catch(error => error.namespace);
+  expect({ namespace }).toMatchInlineSnapshot(`
+    {
+      "namespace": Module Module {
+        "hoisted": [Function: hoisted],
+      },
+    }
+  `);
+});
+
 describe("toMatchSnapshot errors", () => {
   it("should throw if property matchers exist and received is not an object", () => {
     expect(() => {

test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts

@@ -2546,6 +2546,68 @@ describe.concurrent("a snapshot that does not match what the value prints as", (
     });
   });
 
+  test("of a module namespace leaves out an export that is not set yet and a hidden key, and an older Bun's passes", async () => {
+    const { log, stderr, exitCode } = await logOf({
+      // It throws halfway through its body, so `later` stays in its temporal dead zone.
+      "stops.mjs": `
+          import * as self from "./stops.mjs";
+          export function hoisted() {}
+          (() => {
+            throw Object.assign(new Error("stops here"), { namespace: self });
+          })();
+          export const later = 1;
+        `,
+      "required.mjs": `export const b = 1; export function a() {}`,
+      "a.test.js": `
+          import { test, expect } from "bun:test";
+          ${firstLine}
+          const unset = await import("./stops.mjs").catch(error => error.namespace);
+          // Script can define these two keys on the namespace that require() gives. An older Bun printed them and no export.
+          const required = require("./required.mjs");
+          Object.defineProperty(required, "__esModule", { value: true });
+          Object.defineProperty(required, Symbol("hidden"), { value: 1 });
+          test("now", () => {
+            expect({ unset, required }).toMatchInlineSnapshot(\`
+              {
+                "required": Module Module {
+                  "a": [Function: a],
+                  "b": 1,
+                },
+                "unset": Module Module {
+                  "hoisted": [Function: hoisted],
+                },
+              }
+            \`);
+          });
+          test("an older Bun", () => {
+            expect({ unset, required }).toMatchInlineSnapshot(\`
+              {
+                "required": Module Module {
+                  "__esModule": true,
+                  [Symbol(hidden)]: 1,
+                },
+                "unset": Module {},
+              }
+            \`);
+          });
+          test("another value", () => {
+            console.log(firstLine(() => expect({ unset }).toMatchInlineSnapshot(\`
+              {
+                "unset": Module Module {
+                  "hoisted": [Function: other],
+                },
+              }
+            \`)));
+          });
+        `,
+    });
+    expect({ log, stderr: exitCode ? stderr : "", exitCode }).toEqual({
+      log: ["expect(received).toMatchInlineSnapshot(expected)"],
+      stderr: "",
+      exitCode: 0,
+    });
+  });
+
   test("of import.meta.env fails when a variable has another value", async () => {
     using dir = tempDir("snapshot-import-meta-env", {
       "a.test.js": `