From 4da88e0dc5e22872efc6618063d62673e23ed0cc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:02:48 +0000 Subject: [PATCH] Sorted property walk: list a module namespace by name, skip an export that is not set yet The sorted walk listed the enumerable keys of a value through its method table. For a module namespace that listing reads every export, as Object.keys() does, and throws a ReferenceError for an export in its temporal dead zone. Bun.inspect(ns, { sorted: true }), a Bun-format snapshot and the diff of a failing toEqual() rethrew that error. The walk now lists every key of a namespace, which reads no export, and drops the non-enumerable keys by their slot. The read of each key already skips one that throws, so the unset export is left out, as in the plain print. Both namespace classes are covered: the one of the module loader and the one of the ahead-of-time run time. The ErrorEvent test in inspect.test.js takes the change of #47123, so that it no longer depends on its line number. --- src/jsc/bindings/bindings.cpp | 16 ++- .../earlier.mjs | 5 + .../later.mjs | 3 + .../main.mjs | 4 + test/js/bun/console/bun-inspect-table.test.ts | 21 ++++ test/js/bun/test/expect-extend.test.js | 27 +++++ test/js/bun/test/expect.test.js | 44 ++++++++ .../test/snapshot-tests/bun-snapshots.test.ts | 24 +++++ .../snapshot-tests/snapshots/snapshot.test.ts | 62 +++++++++++ test/js/bun/util/inspect.test.js | 100 ++++++++++++++++-- 10 files changed, 297 insertions(+), 9 deletions(-) create mode 100644 test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/earlier.mjs create mode 100644 test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/later.mjs create mode 100644 test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/main.mjs diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 81597bb610cc..c85e6708da77 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/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,6 +6298,8 @@ 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); @@ -6295,7 +6307,7 @@ static void forEachPropertyOrderedImpl(JSC::EncodedJSValue JSValue0, JSC::JSGlob 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(); diff --git a/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/earlier.mjs b/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/earlier.mjs new file mode 100644 index 000000000000..b6b11e6964d6 --- /dev/null +++ b/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 })); diff --git a/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/later.mjs b/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/later.mjs new file mode 100644 index 000000000000..91647f429b39 --- /dev/null +++ b/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() {} diff --git a/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/main.mjs b/test/aot/parallel/modules/namespace-printed-sorted-while-export-in-tdz/main.mjs new file mode 100644 index 000000000000..d432ae1e8a36 --- /dev/null +++ b/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"); diff --git a/test/js/bun/console/bun-inspect-table.test.ts b/test/js/bun/console/bun-inspect-table.test.ts index 29a5d16c98e0..015615291604 100644 --- a/test/js/bun/console/bun-inspect-table.test.ts +++ b/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)", () => { diff --git a/test/js/bun/test/expect-extend.test.js b/test/js/bun/test/expect-extend.test.js index 1426a2c7aac5..5423f7b7de94 100644 --- a/test/js/bun/test/expect-extend.test.js +++ b/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: "], diff --git a/test/js/bun/test/expect.test.js b/test/js/bun/test/expect.test.js index db7fe78a3959..0dde3bfc1de2 100644 --- a/test/js/bun/test/expect.test.js +++ b/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; diff --git a/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts b/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts index 41bbca2fcd5a..d21c2f8e48bd 100644 --- a/test/js/bun/test/snapshot-tests/bun-snapshots.test.ts +++ b/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(() => { diff --git a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts index d7096456bf07..ec2b62e03143 100644 --- a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts +++ b/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": ` diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index 8e502eb2bab0..b19ad130a23a 100644 --- a/test/js/bun/util/inspect.test.js +++ b/test/js/bun/util/inspect.test.js @@ -832,6 +832,67 @@ describe.concurrent("Bun.inspect when a property lookup throws", () => { const result = await runChild(["a.mjs"], String(dir)); expect(result).toEqual({ stdout: "Module {\n hoisted: [Function: hoisted],\n}\n", stderr: "", exitCode: 0 }); }); + + it("skips a module namespace export that is in its temporal dead zone when the keys are sorted", async () => { + // Listing only the enumerable keys of a namespace reads every export, as Object.keys() does, and + // throws for one that is not set yet. The sorted walk listed the keys that way and rethrew. + using dir = tempDir("inspect-tdz-namespace-sorted", { + "a.mjs": ` + import { printSorted } from "./b.mjs"; + export const later = 1; + export let laterLet = 2; + export class LaterClass {} + export function hoisted() {} + printSorted(); + `, + "b.mjs": ` + import * as a from "./a.mjs"; + export const printSorted = () => console.log(Bun.inspect(a, { sorted: true })); + printSorted(); + for (const holder of [{ a }, [a], new Map([["k", a]]), new Proxy(a, {})]) + console.log(Bun.inspect(holder, { sorted: true, compact: true })); + let enumerableKeys; + try { + enumerableKeys = Object.keys(a); + } catch (error) { + enumerableKeys = error.name; + } + console.log(enumerableKeys, Reflect.ownKeys(a).filter(key => typeof key === "string").join()); + `, + }); + const result = await runChild(["a.mjs"], String(dir)); + expect(result).toEqual({ + stdout: [ + "Module {\n hoisted: [Function: hoisted],\n}", + "{ a: Module { hoisted: [Function: hoisted] } }", + "[ Module { hoisted: [Function: hoisted] } ]", + 'Map(1) { "k": Module { hoisted: [Function: hoisted] } }', + "Module { hoisted: [Function: hoisted] }", + "ReferenceError LaterClass,hoisted,later,laterLet", + "Module {\n LaterClass: [class LaterClass],\n hoisted: [Function: hoisted],\n later: 1,\n laterLet: 2,\n}", + "", + ].join("\n"), + stderr: "", + exitCode: 0, + }); + }); + + it("skips the unset exports of a node:vm module that is linked and not evaluated, sorted or not", async () => { + const module = new vm.SourceTextModule( + "export const later = 1; export let laterLet = 2; export function hoisted() {}", + ); + await module.link(() => {}); + const linked = [module.status, Bun.inspect(module.namespace), Bun.inspect(module.namespace, { sorted: true })]; + await module.evaluate(); + expect({ linked, evaluated: Bun.inspect(module.namespace, { sorted: true }) }).toEqual({ + linked: [ + "linked", + "Module {\n hoisted: [Function: hoisted],\n}", + "Module {\n hoisted: [Function: hoisted],\n}", + ], + evaluated: "Module {\n hoisted: [Function: hoisted],\n later: 1,\n laterLet: 2,\n}", + }); + }); }); describe("console.logging function displays async and generator names", async () => { @@ -986,16 +1047,24 @@ it("ErrorEvent", () => { colno: 10, error: new Error("Test error"), }); - expect(normalizeBunSnapshot(Bun.inspect(errorEvent)).replace(/\d+ \| /gim, "NNN |")).toMatchInlineSnapshot(` + const inspected = normalizeBunSnapshot(Bun.inspect(errorEvent)); + // The preview's gutter is as wide as its largest line number. The caret indent is that width plus the column. + const gutter = /(?<=^|error: ) *\d+ \| /gm; + const gutterWidth = inspected.match(gutter)[0].length; + expect( + inspected + .replace(gutter, "NNN | ") + .replace(/^ +\^$/m, caret => "^".padStart(caret.length - gutterWidth + "NNN | ".length)), + ).toMatchInlineSnapshot(` "ErrorEvent { type: "error", message: "Something went wrong", - error: NNN | const errorEvent = new ErrorEvent("error", { - NNN | message: "Something went wrong", - NNN | filename: "script.js", - NNN | lineno: 42, - NNN | colno: 10, - NNN | error: new Error("Test error"), + error: NNN | const errorEvent = new ErrorEvent("error", { + NNN | message: "Something went wrong", + NNN | filename: "script.js", + NNN | lineno: 42, + NNN | colno: 10, + NNN | error: new Error("Test error"), ^ error: Test error at (file:NN:NN) @@ -1470,6 +1539,23 @@ describe("own non-enumerable properties are not printed", () => { expect(Bun.inspect(hide({ b: 1, a: 2 }, "c"), { sorted: true })).toBe("{\n a: 2,\n b: 1,\n}"); }); + it("sorted, on a module namespace", () => { + // Script can define `__esModule` and symbol keys on the namespace that require() gives. + using dir = tempDir("inspect-namespace-hidden-keys", { + "esm.mjs": "export const b = 1;\nexport function a() {}\n", + }); + const namespace = require(join(String(dir), "esm.mjs")); + hide(hide(namespace, "__esModule", { value: true }), Symbol("hidden")); + expect(Reflect.ownKeys(namespace).map(String)).toEqual([ + "a", + "b", + "__esModule", + "Symbol(Symbol.toStringTag)", + "Symbol(hidden)", + ]); + expect(Bun.inspect(namespace, { sorted: true })).toBe("Module {\n a: [Function: a],\n b: 1,\n}"); + }); + it("hides the property of the prototype that it shadows", () => { expect(Bun.inspect(hide(Object.create({ a: "inherited", b: 2 }), "a"))).toBe("{\n b: 2,\n}"); class Foo {