diff --git a/scripts/build/deps/webkit.ts b/scripts/build/deps/webkit.ts index 6698656ba5d8..f3d33517b9f1 100644 --- a/scripts/build/deps/webkit.ts +++ b/scripts/build/deps/webkit.ts @@ -3,7 +3,7 @@ * for local mode. Override via `--webkit-version=` to test a branch. * From https://github.com/oven-sh/WebKit releases. */ -export const WEBKIT_VERSION = "09e477744721074f73a67eba197c6103afb9eab6"; +export const WEBKIT_VERSION = "7b763944f0ecd0e18cb9ac89a04ef994e3ccb4ae"; /** * WebKit (JavaScriptCore) — the JS engine. diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index 7c26804e8329..85a628805d90 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -86,18 +86,18 @@ static JSC::JSPromise* resolvedInternalPromise(JSC::JSGlobalObject* globalObject } // Converts an object from InternalModuleRegistry into { ...obj, default: obj } -static JSC::SyntheticSourceProvider::SyntheticSourceGenerator generateInternalModuleSourceCode(JSC::JSGlobalObject* globalObject, InternalModuleRegistry::Field moduleId) +static JSC::SyntheticSourceProvider::LazySyntheticSourceGenerator generateInternalModuleSourceCode(JSC::JSGlobalObject* globalObject, InternalModuleRegistry::Field moduleId) { return [moduleId](JSC::JSGlobalObject* lexicalGlobalObject, JSC::Identifier moduleKey, Vector& exportNames, - JSC::MarkedArgumentBuffer& exportValues) -> void { + JSC::MarkedArgumentBuffer& exportValues) -> JSC::JSObject* { auto& vm = JSC::getVM(lexicalGlobalObject); GlobalObject* globalObject = uncheckedDowncast(lexicalGlobalObject); auto throwScope = DECLARE_THROW_SCOPE(vm); JSValue requireResult = globalObject->internalModuleRegistry()->requireId(globalObject, vm, moduleId); - RETURN_IF_EXCEPTION(throwScope, void()); + RETURN_IF_EXCEPTION(throwScope, nullptr); auto* object = requireResult.getObject(); ASSERT_WITH_MESSAGE(object, "Expected object from requireId %s", moduleKey.string().string().utf8().data()); @@ -105,21 +105,33 @@ static JSC::SyntheticSourceProvider::SyntheticSourceGenerator generateInternalMo PropertyNameArrayBuilder properties(vm, PropertyNameMode::Strings, PrivateSymbolMode::Exclude); object->getOwnPropertyNames(object, globalObject, properties, DontEnumPropertiesMode::Exclude); - RETURN_IF_EXCEPTION(throwScope, void()); + RETURN_IF_EXCEPTION(throwScope, nullptr); auto len = properties.size() + 1; exportNames.reserveCapacity(len); exportValues.ensureCapacity(len); bool hasDefault = false; + bool hasLazyExports = false; for (auto& entry : properties) { if (entry == vm.propertyNames->defaultKeyword) [[unlikely]] { hasDefault = true; } exportNames.append(entry); - JSValue value = object->get(globalObject, entry); - RETURN_IF_EXCEPTION(throwScope, void()); + + PropertySlot slot(object, PropertySlot::InternalMethodType::GetOwnProperty); + bool hasOwn = object->methodTable()->getOwnPropertySlot(object, globalObject, entry, slot); + RETURN_IF_EXCEPTION(throwScope, nullptr); + if (hasOwn && !slot.isValue()) { + // Accessors are how builtins defer loading (fs.ReadStream pulls in node:stream); JSC reads them off `object` on first binding. + exportValues.append(JSValue()); + hasLazyExports = true; + continue; + } + + JSValue value = hasOwn ? slot.getValue(globalObject, entry) : object->get(globalObject, entry); + RETURN_IF_EXCEPTION(throwScope, nullptr); exportValues.append(value); } @@ -127,6 +139,8 @@ static JSC::SyntheticSourceProvider::SyntheticSourceGenerator generateInternalMo exportNames.append(vm.propertyNames->defaultKeyword); exportValues.append(object); } + + return hasLazyExports ? object : nullptr; }; } @@ -1036,7 +1050,7 @@ static JSValue fetchESMSourceCode( default: { if (tag & SyntheticModuleType::InternalModuleRegistryFlag) { constexpr auto mask = (SyntheticModuleType::InternalModuleRegistryFlag - 1); - auto source = JSC::SourceCode(JSC::SyntheticSourceProvider::create(generateInternalModuleSourceCode(globalObject, static_cast(tag & mask)), JSC::SourceOrigin(URL(makeString("builtins://"_s, moduleKey))), moduleKey)); + auto source = JSC::SourceCode(JSC::SyntheticSourceProvider::createWithLazyExports(generateInternalModuleSourceCode(globalObject, static_cast(tag & mask)), JSC::SourceOrigin(URL(makeString("builtins://"_s, moduleKey))), moduleKey)); RELEASE_AND_RETURN(scope, rejectOrResolve(JSSourceCode::create(vm, WTF::move(source)))); } else { auto&& provider = Zig::SourceProvider::create(globalObject, res->result.value, JSC::SourceProviderSourceType::Module, true); diff --git a/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts b/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts new file mode 100644 index 000000000000..513986fdff9d --- /dev/null +++ b/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts @@ -0,0 +1,232 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// Builtins implemented in src/js are exposed to ESM importers by turning their CommonJS exports object into a +// synthetic module. Some of those objects define accessors so that the expensive part of the module is only +// loaded by whoever touches it. node:fs is the clearest case: each of its stream classes is a getter that +// require()s internal/fs/streams (and with it the node:stream stack) and then replaces itself with a data +// property, so "is it still an accessor on the exports object" is a direct readout of whether importing the +// module as ESM ran the getter. +// +// Every scenario gets its own process because a binding can only be materialized once. + +const STREAMS = ["ReadStream", "WriteStream", "FileReadStream", "FileWriteStream", "Utf8Stream"] as const; +type Kind = "accessor" | "value"; + +/** Expected descriptor kinds: everything is still an accessor except the names listed as materialized. */ +function kinds(...materialized: (typeof STREAMS)[number][]): Record { + return Object.fromEntries(STREAMS.map(name => [name, materialized.includes(name) ? "value" : "accessor"])); +} + +// `import fs from "node:fs"` here is itself an import of the module under test; the default export is the +// exports object, which is also the thing whose property descriptors we inspect. +const helper = ` + import fs from "node:fs"; + export const STREAMS = ${JSON.stringify(STREAMS)}; + export function kinds() { + return Object.fromEntries( + STREAMS.map(name => [name, typeof Object.getOwnPropertyDescriptor(fs, name).get === "function" ? "accessor" : "value"]), + ); + } + export function print(result) { + console.log(JSON.stringify(result)); + } + export { fs }; +`; + +async function run(files: Record, args: string[] = ["entry.mjs"]) { + using dir = tempDir("builtin-esm-lazy-exports", { "helper.mjs": helper, ...files }); + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +async function runEntry(entry: string, extraFiles: Record = {}) { + const { stdout, stderr, exitCode } = await run({ ...extraFiles, "entry.mjs": entry }); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + return JSON.parse(stdout); +} + +test.concurrent("importing the module does not run its accessors", async () => { + const result = await runEntry(` + import fs, { readFileSync } from "node:fs"; + import { kinds, print } from "./helper.mjs"; + const ns = await import("node:fs"); + print({ defaultIsExportsObject: ns.default === fs, readFileSync: typeof readFileSync, kinds: kinds() }); + `); + expect(result).toEqual({ defaultIsExportsObject: true, readFileSync: "function", kinds: kinds() }); +}); + +test.concurrent("a named import materializes exactly the binding it links", async () => { + const result = await runEntry(` + import { ReadStream } from "node:fs"; + import { fs, kinds, print } from "./helper.mjs"; + print({ kinds: kinds(), isTheClass: typeof ReadStream === "function" && ReadStream === fs.ReadStream }); + `); + expect(result).toEqual({ kinds: kinds("ReadStream"), isTheClass: true }); +}); + +test.concurrent("a namespace export materializes when it is read, not on import or `in`", async () => { + const result = await runEntry(` + import * as ns from "node:fs"; + import { fs, kinds, print } from "./helper.mjs"; + const afterImport = kinds(); + // [[HasProperty]] does not read the binding. ([[GetOwnProperty]], e.g. hasOwnProperty or Object.keys, does.) + const present = "WriteStream" in ns; + const afterIn = kinds(); + const WriteStream = ns.WriteStream; + print({ + afterImport, + present, + afterIn, + afterRead: kinds(), + isTheClass: typeof WriteStream === "function" && WriteStream === fs.WriteStream, + secondReadIsStable: ns.WriteStream === WriteStream, + }); + `); + expect(result).toEqual({ + afterImport: kinds(), + present: true, + afterIn: kinds(), + afterRead: kinds("WriteStream"), + isTheClass: true, + secondReadIsStable: true, + }); +}); + +test.concurrent("a deferred namespace (import defer) materializes on read as well", async () => { + const result = await runEntry(` + import defer * as ns from "node:fs"; + import { fs, kinds, print } from "./helper.mjs"; + const afterImport = kinds(); + const FileReadStream = ns.FileReadStream; + print({ + afterImport, + afterRead: kinds(), + isTheClass: typeof FileReadStream === "function" && FileReadStream === fs.FileReadStream, + }); + `); + expect(result).toEqual({ afterImport: kinds(), afterRead: kinds("FileReadStream"), isTheClass: true }); +}); + +test.concurrent("import() namespace: same export list as before, enumerating it materializes everything", async () => { + const result = await runEntry(` + import { STREAMS, fs, kinds, print } from "./helper.mjs"; + const exportsKeys = Object.keys(fs); + const ns = await import("node:fs"); + const afterImport = kinds(); + const isTheClass = ns.Utf8Stream === fs.Utf8Stream && typeof ns.Utf8Stream === "function"; + const afterRead = kinds(); + const namespaceKeys = Object.keys(ns); + print({ + afterImport, + isTheClass, + afterRead, + afterKeys: kinds(), + keysMatch: namespaceKeys.sort().join() === [...exportsKeys, "default"].sort().join(), + allAreTheClasses: STREAMS.every(name => typeof ns[name] === "function" && ns[name] === fs[name]), + }); + `); + expect(result).toEqual({ + afterImport: kinds(), + isTheClass: true, + afterRead: kinds("Utf8Stream"), + afterKeys: kinds(...STREAMS), + keysMatch: true, + allAreTheClasses: true, + }); +}); + +test.concurrent("re-exports bind through to the builtin's own binding", async () => { + const result = await runEntry( + ` + // Linking this import is what materializes FileWriteStream, through the export *. + import { FileWriteStream } from "./reexport.mjs"; + import * as reexported from "./reexport.mjs"; + import { fs, kinds, print } from "./helper.mjs"; + const afterLink = kinds(); + const Renamed = reexported.Renamed; + const afterRenamedRead = kinds(); + const viaStar = reexported.ReadStream; + print({ + afterLink, + afterRenamedRead, + afterStarRead: kinds(), + linked: typeof FileWriteStream === "function" && FileWriteStream === fs.FileWriteStream, + renamed: typeof Renamed === "function" && Renamed === fs.FileReadStream, + viaStar: typeof viaStar === "function" && viaStar === fs.ReadStream, + }); + `, + { + "reexport.mjs": ` + export * from "node:fs"; + export { FileReadStream as Renamed } from "node:fs"; + `, + }, + ); + expect(result).toEqual({ + afterLink: kinds("FileWriteStream"), + afterRenamedRead: kinds("FileWriteStream", "FileReadStream"), + afterStarRead: kinds("FileWriteStream", "FileReadStream", "ReadStream"), + linked: true, + renamed: true, + viaStar: true, + }); +}); + +test.concurrent("accessors on other builtins bind to what require() returns", async () => { + const result = await runEntry(` + import assert, { AssertionError } from "node:assert"; + import * as timers from "node:timers"; + import * as stream from "node:stream"; + import { createRequire } from "node:module"; + import { print } from "./helper.mjs"; + const require = createRequire(import.meta.url); + print({ + // node:assert's exports object is a function; AssertionError is an accessor defined on it. + assertIsFunction: typeof assert === "function", + assertionError: typeof AssertionError === "function" && AssertionError === require("node:assert").AssertionError, + timersPromises: timers.promises === require("node:timers/promises"), + streamPromises: stream.promises === require("node:stream/promises"), + }); + `); + expect(result).toEqual({ assertIsFunction: true, assertionError: true, timersPromises: true, streamPromises: true }); +}); + +test.concurrent("spyOn and mock.module on an imported builtin", async () => { + const { stderr, exitCode } = await run( + { + "lazy.test.ts": ` + import { expect, mock, spyOn, test } from "bun:test"; + import * as ns from "node:fs"; + import { fs, kinds } from "./helper.mjs"; + + test("spyOn reads (and so materializes) the binding it wraps", () => { + expect(kinds()).toEqual(${JSON.stringify(kinds())}); + spyOn(ns, "WriteStream"); + expect(kinds()).toEqual(${JSON.stringify(kinds("WriteStream"))}); + expect(ns.WriteStream).not.toBe(fs.WriteStream); + mock.restore(); + expect(ns.WriteStream).toBe(fs.WriteStream); + }); + + test("mock.module writes into a binding nothing has read, without running its getter", () => { + mock.module("node:fs", () => ({ ReadStream: "mocked" })); + expect(ns.ReadStream).toBe("mocked"); + expect(kinds()).toEqual(${JSON.stringify(kinds("WriteStream"))}); + }); + `, + }, + ["test", "lazy.test.ts"], + ); + expect(stderr).toContain(" 2 pass"); + expect(stderr).toContain(" 0 fail"); + expect(exitCode).toBe(0); +});