diff --git a/src/jsc/bindings/BunObject.cpp b/src/jsc/bindings/BunObject.cpp index 062882b38a82..04c1c4ca808b 100644 --- a/src/jsc/bindings/BunObject.cpp +++ b/src/jsc/bindings/BunObject.cpp @@ -42,6 +42,7 @@ #include "GeneratedBunObject.h" #include "JavaScriptCore/BunV8HeapSnapshotBuilder.h" #include "BunObjectModule.h" +#include "_NativeModule.h" #include "JSCookie.h" #include "JSCookieMap.h" #include "Secrets.h" @@ -1151,9 +1152,6 @@ JSC::JSObject* createBunObject(VM& vm, JSObject* globalObject) } // namespace Bun namespace Zig { -// Every export except `default` is declared without a value: JSC reads `Bun[name]` the first time -// something binds to it (SyntheticModuleRecord::materializeLazyExport), so importing the module does -// not run the PropertyCallbacks in bunObjectTable, most of which construct a class or load a builtin. JSC::JSObject* generateNativeModule_BunObject(JSC::JSGlobalObject* lexicalGlobalObject, JSC::Identifier moduleKey, Vector& exportNames, @@ -1164,25 +1162,13 @@ JSC::JSObject* generateNativeModule_BunObject(JSC::JSGlobalObject* lexicalGlobal auto scope = DECLARE_THROW_SCOPE(vm); auto* object = globalObject->bunObject(); - // Static table entries are listed whether or not they have been reified. + // Static table entries are listed whether or not they have been reified, so this is the same export + // list that reifying them all used to produce, minus the cost of constructing every one of them. PropertyNameArrayBuilder propertyNames(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); object->getOwnNonIndexPropertyNames(globalObject, propertyNames, DontEnumPropertiesMode::Exclude); RETURN_IF_EXCEPTION(scope, nullptr); - exportNames.reserveCapacity(propertyNames.size() + 1); - exportValues.ensureCapacity(propertyNames.size() + 1); - - exportNames.append(vm.propertyNames->defaultKeyword); - exportValues.append(object); - - for (const auto& propertyName : propertyNames) { - if (propertyName == vm.propertyNames->defaultKeyword) [[unlikely]] - continue; - exportNames.append(propertyName); - exportValues.append(JSValue()); - } - - return object; + return exportObjectProperties(vm, object, propertyNames, exportNames, exportValues); } } // namespace Zig diff --git a/src/jsc/modules/NativeModuleList.h b/src/jsc/modules/NativeModuleList.h index 5dbe2fb7adda..c6e72ebd14b8 100644 --- a/src/jsc/modules/NativeModuleList.h +++ b/src/jsc/modules/NativeModuleList.h @@ -18,15 +18,15 @@ macro("abort-controller"_s, AbortControllerModule) #define BUN_FOREACH_ESM_NATIVE_MODULE(macro) \ - BUN_FOREACH_ESM_AND_CJS_NATIVE_MODULE(macro) \ - macro("node:module"_s, NodeModule) \ - macro("node:process"_s, NodeProcess) + BUN_FOREACH_ESM_AND_CJS_NATIVE_MODULE(macro) // Generators with JSC::SyntheticSourceProvider::LazySyntheticSourceGenerator's signature: they may -// declare exports without a value and return the object JSC reads those exports from on first binding. -// src/codegen/internal-module-registry-scanner.ts numbers native modules by their order in this file, -// so these stay after the lists above. +// declare exports without a value and return the object JSC reads those exports from on first binding +// (see exportObjectProperties in _NativeModule.h). src/codegen/internal-module-registry-scanner.ts +// numbers native modules by their order in this file, so these stay after the list above, in this order. #define BUN_FOREACH_LAZY_ESM_NATIVE_MODULE(macro) \ + macro("node:module"_s, NodeModule) \ + macro("node:process"_s, NodeProcess) \ macro("bun"_s, BunObject) #define BUN_FOREACH_CJS_NATIVE_MODULE(macro) \ diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index 520619e9abe9..b8732d7d8fee 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -1219,48 +1219,22 @@ JSC::JSValue createStreamIterEnabledFlag(Zig::GlobalObject*) } // namespace Bun namespace Zig { -void generateNativeModule_NodeModule(JSC::JSGlobalObject* lexicalGlobalObject, +JSC::JSObject* generateNativeModule_NodeModule(JSC::JSGlobalObject* lexicalGlobalObject, JSC::Identifier moduleKey, Vector& exportNames, JSC::MarkedArgumentBuffer& exportValues) { Zig::GlobalObject* globalObject = defaultGlobalObject(lexicalGlobalObject); auto& vm = JSC::getVM(globalObject); - auto topExceptionScope = DECLARE_TOP_EXCEPTION_SCOPE(vm); auto* constructor = globalObject->m_nodeModuleConstructor.getInitializedOnMainThread(globalObject); - // Don't bulk-reifyAllStaticProperties here. JSObject::reifyAllStaticProperties - // walks every PropertyCallbackAttribute back-to-back without an exception - // check between them, and several of our callbacks (getBuiltinModulesObject, - // getGlobalPathsObject, …) call constructArray/constructEmptyArray which - // open a ThrowScope at the same recursion depth as the next callback's - // ThrowScope — that trips the exception-check verifier on the synthetic - // ESM path (BUN_JSC_validateExceptionChecks=1). The loop below already - // does constructor->get(property) per-export, which lazy-reifies one entry - // at a time inside JSObject::get's own ThrowScope and is checked - // immediately after. - - exportNames.reserveCapacity(Bun::countof(Bun::nodeModuleObjectTableValues) + 1); - exportValues.ensureCapacity(Bun::countof(Bun::nodeModuleObjectTableValues) + 1); - - exportNames.append(vm.propertyNames->defaultKeyword); - exportValues.append(constructor); - - for (unsigned i = 0; i < Bun::countof(Bun::nodeModuleObjectTableValues); ++i) { - const auto& entry = Bun::nodeModuleObjectTableValues[i]; - const auto& property = Identifier::fromString(vm, entry.m_key); - JSValue value = constructor->get(globalObject, property); - - if (topExceptionScope.exception()) [[unlikely]] { - // A termination (worker terminate() mid-import) cannot be cleared: - // stop the walk and leave it pending for the loader. - if (!topExceptionScope.tryClearException()) - return; - value = jsUndefined(); - } - exportNames.append(property); - exportValues.append(value); - } + // The exports are the static table's entries, not the constructor's own properties (`length`, `name`, + // whatever user code assigned onto Module). + PropertyNameArrayBuilder properties(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); + for (const auto& entry : Bun::nodeModuleObjectTableValues) + properties.add(Identifier::fromString(vm, entry.m_key)); + + return exportObjectProperties(vm, constructor, properties, exportNames, exportValues); } } // namespace Zig diff --git a/src/jsc/modules/NodeModuleModule.h b/src/jsc/modules/NodeModuleModule.h index 64a261e66dd8..edb1c4429453 100644 --- a/src/jsc/modules/NodeModuleModule.h +++ b/src/jsc/modules/NodeModuleModule.h @@ -36,10 +36,10 @@ JSC::JSValue resolveLookupPaths(JSC::JSGlobalObject* globalObject, String reques namespace Zig { -void generateNativeModule_NodeModule( - JSC::JSGlobalObject *lexicalGlobalObject, JSC::Identifier moduleKey, +JSC::JSObject *generateNativeModule_NodeModule( + JSC::JSGlobalObject *lexicalGlobalObject, JSC::Identifier moduleKey, Vector &exportNames, - JSC::MarkedArgumentBuffer &exportValues); + JSC::MarkedArgumentBuffer &exportValues); } // namespace Zig diff --git a/src/jsc/modules/NodeProcessModule.h b/src/jsc/modules/NodeProcessModule.h index 0f4c87a6e189..da6657c29475 100644 --- a/src/jsc/modules/NodeProcessModule.h +++ b/src/jsc/modules/NodeProcessModule.h @@ -6,46 +6,19 @@ namespace Zig { -DEFINE_NATIVE_MODULE(NodeProcess) +DEFINE_LAZY_NATIVE_MODULE(NodeProcess) { auto& vm = lexicalGlobalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); auto* globalObject = defaultGlobalObject(lexicalGlobalObject); - Bun::Process* process = globalObject->processObject(); - // Don't bulk-reifyAllStaticProperties here (see generateNativeModule_NodeModule - // for the long version). It runs every PropertyCallback back-to-back without an - // exception check in between, which trips BUN_JSC_validateExceptionChecks=1. - // The per-export get() below lazy-reifies one property at a time inside - // JSObject::get's own checked ThrowScope. + // The whole prototype chain: the EventEmitter methods are exports of this module too. PropertyNameArrayBuilder properties(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); process->getPropertyNames(globalObject, properties, DontEnumPropertiesMode::Exclude); - RETURN_IF_EXCEPTION(scope, ); - - exportNames.append(vm.propertyNames->defaultKeyword); - exportValues.append(process); - - for (auto& entry : properties.releaseData()->propertyNameVector()) { - if (entry == vm.propertyNames->defaultKeyword) { - // skip because it's already on the default - // export (the Process object itself) - continue; - } - - auto topExceptionScope = DECLARE_TOP_EXCEPTION_SCOPE(vm); - JSValue result = process->get(globalObject, entry); - if (topExceptionScope.exception()) { - // A getter that throws exports undefined; a termination (worker - // terminate() mid-import) cannot be cleared: stop, leave it pending. - if (!topExceptionScope.tryClearException()) - return; - result = jsUndefined(); - } + RETURN_IF_EXCEPTION(scope, nullptr); - exportNames.append(entry); - exportValues.append(result); - } + return exportObjectProperties(vm, process, properties, exportNames, exportValues); } } // namespace Zig diff --git a/src/jsc/modules/_NativeModule.h b/src/jsc/modules/_NativeModule.h index 1031f303d642..29d63d3b18d8 100644 --- a/src/jsc/modules/_NativeModule.h +++ b/src/jsc/modules/_NativeModule.h @@ -3,6 +3,7 @@ #include "JSBuffer.h" #include #include +#include #include "ZigGlobalObject.h" #include "NativeModuleList.h" @@ -57,6 +58,12 @@ JSC::JSGlobalObject *lexicalGlobalObject, JSC::Identifier moduleKey, \ Vector &exportNames, \ JSC::MarkedArgumentBuffer &exportValues) +// For modules in BUN_FOREACH_LAZY_ESM_NATIVE_MODULE; the body usually ends in exportObjectProperties(). +#define DEFINE_LAZY_NATIVE_MODULE(name) \ + inline JSC::JSObject *generateNativeModule_##name( \ + JSC::JSGlobalObject *lexicalGlobalObject, JSC::Identifier moduleKey, \ + Vector &exportNames, \ + JSC::MarkedArgumentBuffer &exportValues) #define INIT_NATIVE_MODULE(slot, numberOfExportNames) \ Zig::GlobalObject *globalObject = \ @@ -134,4 +141,34 @@ JSC::JSObject* generateNativeModule_##enumName( \ Vector &exportNames, \ JSC::MarkedArgumentBuffer &exportValues); BUN_FOREACH_LAZY_ESM_NATIVE_MODULE(FORWARD_DECL_LAZY_GENERATOR) + +// The lazy modules each mirror an object that already exists (the Bun object, process, the Module +// constructor): `default` is the object and each of propertyNames is an export. A value that is +// already stored on the object is exported as is. Anything else, i.e. a static table entry nothing +// has read yet, an accessor, or an inherited property, is declared without a value, and JSC reads +// object[name] when something first binds to it, so loading the module does not construct the +// object's lazy properties. Returns the object, as the LazySyntheticSourceGenerator contract wants. +inline JSC::JSObject *exportObjectProperties( + JSC::VM &vm, JSC::JSObject *object, + const JSC::PropertyNameArrayBuilder &propertyNames, + Vector &exportNames, + JSC::MarkedArgumentBuffer &exportValues) { + exportNames.reserveCapacity(propertyNames.size() + 1); + exportValues.ensureCapacity(propertyNames.size() + 1); + + exportNames.append(vm.propertyNames->defaultKeyword); + exportValues.append(object); + + for (const auto &propertyName : propertyNames) { + if (propertyName == vm.propertyNames->defaultKeyword) [[unlikely]] + continue; + JSC::JSValue stored = object->getDirect(vm, propertyName); + if (stored && (stored.isGetterSetter() || stored.isCustomGetterSetter())) + stored = JSC::JSValue(); + exportNames.append(propertyName); + exportValues.append(stored); + } + + return object; +} } // namespace Zig \ No newline at end of file diff --git a/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts b/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts index b331e3ddb88f..d82c66e201f6 100644 --- a/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts +++ b/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts @@ -34,26 +34,43 @@ const helper = ` export { fs }; `; -// The "bun" module is generated natively from the Bun object (generateNativeModule_BunObject). Most Bun.* properties -// are PropertyCallback entries of its static table that construct their value (a class, the shell, the default -// SQL/S3/Redis clients, ...) the first time they are read, at which point they become own properties of the object. -// bun:jsc's describe() dumps the object's Structure, i.e. exactly the set of properties that have been constructed, -// which is the readout used here. WATCHED is a sample of them plus `write`, the plain function the entries below import. +// "bun", "node:process" and "node:module" are generated natively from an existing object (the Bun object, process, +// the Module constructor; see BUN_FOREACH_LAZY_ESM_NATIVE_MODULE). Most of those objects' properties are +// PropertyCallback entries of a static table that construct their value (a class, the shell, the default +// SQL/S3/Redis clients, the stdio streams, the builtinModules array, ...) the first time they are read, at which +// point they become own properties of the object. bun:jsc's describe() dumps the object's Structure, i.e. exactly the +// set of properties that have been constructed, which is the readout used here. (Object.keys and Reflect.ownKeys list +// static entries without constructing them; for...in constructs all of them, so the entries below avoid it.) The +// watched names are a sample of those entries plus the plain function (`write`, `createRequire`) an entry imports. // // Note that a literal `import ... from "bun"` (and `import("bun")` / `require("bun")`) is rewritten by the // transpiler into a read of globalThis.Bun and never loads the module; the module is what `export ... from "bun"` -// and a non-literal import() specifier go through. +// and a non-literal import() specifier go through. Imports of node:process and node:module always load the module. const WATCHED = ["$", "CryptoHasher", "Glob", "S3Client", "SQL", "TOML", "Transpiler", "secrets", "write"] as const; +const PROCESS_WATCHED = ["allowedNodeEnvironmentFlags", "config", "release", "stderr", "stdin", "stdout", "versions"]; +const MODULE_WATCHED = [ + "SourceMap", + "_cache", + "_extensions", + "builtinModules", + "constants", + "createRequire", + "globalPaths", +]; -const bunHelper = ` +const nativeHelper = ` import { describe } from "bun:jsc"; - const WATCHED = ${JSON.stringify(WATCHED)}; - /** The WATCHED names that are own properties of the Bun object by now, i.e. whose value has been constructed. */ - export function constructed() { - const properties = /\\{([^}]*)\\}/.exec(describe(Bun))[1]; + /** The names out of \`watched\` that are own properties of \`object\` by now, i.e. whose value has been constructed. */ + function constructedOn(object, watched) { + const properties = /\\{([^}]*)\\}/.exec(describe(object))[1]; const names = properties.split(",").map(entry => entry.trim().split(":")[0]); - return WATCHED.filter(name => names.includes(name)); + return watched.filter(name => names.includes(name)); } + export const constructed = () => constructedOn(Bun, ${JSON.stringify(WATCHED)}); + export const constructedOnProcess = () => constructedOn(process, ${JSON.stringify(PROCESS_WATCHED)}); + // getBuiltinModule hands out the constructor itself without going through the ES module. + export const constructedOnModule = () => + constructedOn(process.getBuiltinModule("node:module"), ${JSON.stringify(MODULE_WATCHED)}); export function print(result) { console.log(JSON.stringify(result)); } @@ -68,7 +85,11 @@ const bunReexport = ` `; async function run(files: Record, args: string[] = ["entry.mjs"], env: Record = {}) { - using dir = tempDir("builtin-esm-lazy-exports", { "helper.mjs": helper, "bun-helper.mjs": bunHelper, ...files }); + using dir = tempDir("builtin-esm-lazy-exports", { + "helper.mjs": helper, + "native-helper.mjs": nativeHelper, + ...files, + }); await using proc = Bun.spawn({ cmd: [bunExe(), ...args], cwd: String(dir), @@ -270,7 +291,7 @@ test.concurrent('"bun": re-exports construct the properties that get bound, not // Linking this import is what constructs Bun.write; nothing else is read. import { write } from "./reexport.mjs"; import * as reexported from "./reexport.mjs"; - import { constructed, print } from "./bun-helper.mjs"; + import { constructed, print } from "./native-helper.mjs"; const afterLink = constructed(); const present = "SQL" in reexported; const afterIn = constructed(); @@ -308,7 +329,7 @@ test.concurrent('"bun": re-exports construct the properties that get bound, not test.concurrent('"bun": import() namespace has the same export list, and every export is the Bun.* value', async () => { const result = await runEntry(` - import { constructed, print, specifier } from "./bun-helper.mjs"; + import { constructed, print, specifier } from "./native-helper.mjs"; const ns = await import(specifier); const afterImport = constructed(); // Neither [[OwnPropertyKeys]] of the namespace nor Object.keys of the Bun object reads any of the properties. @@ -348,7 +369,7 @@ test.concurrent('"bun": a property whose getter throws only fails the binding th ` import { write } from "./reexport.mjs"; import * as reexported from "./reexport.mjs"; - import { print } from "./bun-helper.mjs"; + import { print } from "./native-helper.mjs"; function message(read) { try { read(); @@ -383,7 +404,7 @@ test.concurrent('"bun": mock.module replaces a binding without constructing the "lazy.test.ts": ` import { expect, mock, test } from "bun:test"; import * as reexported from "./reexport.mjs"; - import { constructed } from "./bun-helper.mjs"; + import { constructed } from "./native-helper.mjs"; test("mock.module('bun')", () => { expect(constructed()).toEqual([]); @@ -402,3 +423,93 @@ test.concurrent('"bun": mock.module replaces a binding without constructing the expect(stderr).toContain(" 0 fail"); expect(exitCode).toBe(0); }); + +test.concurrent("node:process: linking constructs the linked bindings, not the stdio streams", async () => { + const result = await runEntry(` + import proc, { on, release } from "node:process"; + import * as ns from "node:process"; + import { constructedOnProcess, print } from "./native-helper.mjs"; + const afterLink = constructedOnProcess(); + // The exports are the enumerable names of process and its prototype chain, as listed when the module loaded. + // (Object.keys does not reify anything, unlike for...in, and nothing has changed the chain since loading: that + // happens further down, when reading stdout loads node:events.) + const enumerable = new Set(); + for (let object = process; object !== null; object = Object.getPrototypeOf(object)) { + for (const name of Object.keys(object)) enumerable.add(name); + } + const exportNames = Reflect.ownKeys(ns).filter(key => typeof key === "string"); + const exportListMatches = exportNames.sort().join() === [...enumerable, "default"].sort().join(); + const afterListing = constructedOnProcess(); + const stdout = ns.stdout; + print({ + defaultIsProcess: proc === process, + afterLink, + release: release === process.release, + // Inherited from the EventEmitter prototype, so it was never stored on process itself. + on: on === process.on && !Object.hasOwn(process, "on"), + exportListMatches, + afterListing, + stdout: stdout === process.stdout && typeof stdout.write === "function", + afterStdoutRead: constructedOnProcess(), + argv: ns.argv === process.argv, + }); + `); + expect(result).toEqual({ + defaultIsProcess: true, + afterLink: ["release"], + release: true, + on: true, + exportListMatches: true, + afterListing: ["release"], + stdout: true, + afterStdoutRead: ["release", "stdout"], + argv: true, + }); +}); + +test.concurrent("node:process: a value already stored on the object is exported as it was at load", async () => { + const result = await runEntry(` + import { print } from "./native-helper.mjs"; + process.addedBeforeLoad = "at load"; + const ns = await import("node:process"); + process.addedBeforeLoad = "after load"; + process.addedAfterLoad = true; + print({ addedBeforeLoad: ns.addedBeforeLoad, exportsAddedAfterLoad: "addedAfterLoad" in ns }); + `); + expect(result).toEqual({ addedBeforeLoad: "at load", exportsAddedAfterLoad: false }); +}); + +test.concurrent("node:module: linking constructs the linked bindings, not the rest of the table", async () => { + const result = await runEntry(` + import Module, { createRequire } from "node:module"; + import * as ns from "node:module"; + import { constructedOnModule, print } from "./native-helper.mjs"; + const afterLink = constructedOnModule(); + const exportNames = Reflect.ownKeys(ns).filter(key => typeof key === "string"); + // The export list is the static table, which is also exactly what Object.keys of the constructor lists. + const exportListMatches = exportNames.sort().join() === [...Object.keys(Module), "default"].sort().join(); + const afterListing = constructedOnModule(); + const builtinModules = ns.builtinModules; + print({ + defaultIsTheConstructor: Module === process.getBuiltinModule("node:module"), + afterLink, + createRequire: typeof createRequire(import.meta.url)("node:path").join === "function", + exportListMatches, + afterListing, + builtinModules: Array.isArray(builtinModules) && builtinModules === Module.builtinModules, + afterRead: constructedOnModule(), + // Backed by an accessor rather than a constructed value; the binding gets what the accessor returns. + resolveFilename: ns._resolveFilename === Module._resolveFilename && typeof ns._resolveFilename === "function", + }); + `); + expect(result).toEqual({ + defaultIsTheConstructor: true, + afterLink: ["createRequire"], + createRequire: true, + exportListMatches: true, + afterListing: ["createRequire"], + builtinModules: true, + afterRead: ["builtinModules", "createRequire"], + resolveFilename: true, + }); +});