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 {