From ff8392a6918177e484c690b32fd029219724421f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:04:16 +0000 Subject: [PATCH] internal-for-testing: make exposedInternals lazy Every entry of exposedInternals was a top-level require(), so loading bun:internal-for-testing evaluated all of those node internals and their dependency graphs. test/harness.ts requires the module from every test file (via test/preload.ts) just to read isASANEnabled, so on a debug build each `bun bd test ` paid about 0.8s of preload for modules the test never asked for; internal/assert/myers_diff alone costs most of that, because it requires internal/util/colors, which materializes process.stderr on load. Turn every entry into a getter, as the internal/child_process entry already was. Entries that assemble an object are memoized so repeated requires keep returning the same object. The consumers (the --expose-internals require interceptor in test/js/node/test/common/index.js, nodeinternals.js, dgram.test.ts, node-stream.test.js) all read entries by property access, so they are unaffected. Also drop the unused `fs` export, which eagerly loaded node:fs/promises for no consumer. --- src/js/internal-for-testing.ts | 110 ++++++++++++++------- test/internal/internal-for-testing.test.ts | 57 +++++++++++ 2 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 test/internal/internal-for-testing.test.ts diff --git a/src/js/internal-for-testing.ts b/src/js/internal-for-testing.ts index 9b1fed3e91d9..9b44165a1ca3 100644 --- a/src/js/internal-for-testing.ts +++ b/src/js/internal-for-testing.ts @@ -453,68 +453,108 @@ function nodeGetValidStdio(stdio, sync?) { return { stdio, ipc, ipcFd }; } +// Entries below that assemble an object rather than return a module are built +// once, so repeated requires get the same object, as they would in node. +let cachedInternalUtil; +let cachedInternalEventTarget; let cachedInternalChildProcess; +let cachedInternalFsUtils; // Userland access to node-internal modules for vendored node tests that // declare `// Flags: --expose-internals` (served via the require interceptor // in test/js/node/test/common/index.js). Static requires only — the builtin // bundler cannot rewrite variable-path requires. Extend the map as more // vendored tests need more internals. +// +// Every entry must be a getter (test/internal/internal-for-testing.test.ts +// enforces it): test/harness.ts loads this module in every test file just to +// read isASANEnabled, so anything required here is evaluated on every test +// run. Small-looking entries are not cheap either; internal/assert/myers_diff +// pulls in internal/util/colors, which materializes process.stderr on load. export const exposedInternals = { - "internal/streams/add-abort-signal": require("internal/streams/add-abort-signal"), - "internal/util/debuglog": require("internal/util/debuglog"), - "internal/async_context_frame": require("internal/async_context_frame"), - "internal/async_hooks": require("internal/async_hooks"), - "internal/webstreams/adapters": require("internal/webstreams_adapters"), - "internal/dgram": require("internal/dgram"), + get "internal/streams/add-abort-signal"() { + return require("internal/streams/add-abort-signal"); + }, + get "internal/util/debuglog"() { + return require("internal/util/debuglog"); + }, + get "internal/async_context_frame"() { + return require("internal/async_context_frame"); + }, + get "internal/async_hooks"() { + return require("internal/async_hooks"); + }, + get "internal/webstreams/adapters"() { + return require("internal/webstreams_adapters"); + }, + get "internal/dgram"() { + return require("internal/dgram"); + }, // Bun's real implementations, under the names node's tests import them by. - "internal/validators": require("internal/validators"), - "internal/util/inspect": require("internal/util/inspect"), - "internal/freelist": require("internal/freelist"), + get "internal/validators"() { + return require("internal/validators"); + }, + get "internal/util/inspect"() { + return require("internal/util/inspect"); + }, + get "internal/freelist"() { + return require("internal/freelist"); + }, // Node's internal/fixed_queue module IS the FixedQueue class. - "internal/fixed_queue": require("internal/fixed_queue").FixedQueue, - "internal/assert/myers_diff": require("internal/assert/myers_diff"), + get "internal/fixed_queue"() { + return require("internal/fixed_queue").FixedQueue; + }, + get "internal/assert/myers_diff"() { + return require("internal/assert/myers_diff"); + }, // Bun's internal/errors only carries aggregateTwoErrors; the ERR_* hierarchy // is native, not a JS `codes` table, so nothing else is exposed here. - "internal/errors": require("internal/errors"), + get "internal/errors"() { + return require("internal/errors"); + }, // normalizeEncoding wraps the same Rust binding node:crypto and the // webstream adapters call; the rest are node's own JS helpers, ported // verbatim from lib/internal/util.js where Bun has no native equivalent. - "internal/util": { - normalizeEncoding: nodeNormalizeEncoding, - // Bun always has crypto support compiled in. - assertCrypto() {}, - getCIDR, - isError: nodeIsError, - assignFunctionName, - kEnumerableProperty: Object.freeze({ __proto__: null, enumerable: true }), - kEmptyObject: nodeKEmptyObject, - WeakReference, + get "internal/util"() { + return (cachedInternalUtil ??= { + normalizeEncoding: nodeNormalizeEncoding, + // Bun always has crypto support compiled in. + assertCrypto() {}, + getCIDR, + isError: nodeIsError, + assignFunctionName, + kEnumerableProperty: Object.freeze({ __proto__: null, enumerable: true }), + kEmptyObject: nodeKEmptyObject, + WeakReference, + }); }, // Bun's EventTarget/Event/CustomEvent are the native (global) ones; node // keeps them in internal/event_target. kWeakHandler is Bun's real weak // listener symbol from internal/shared. Bun has no NodeEventTarget. - "internal/event_target": { - Event: globalThis.Event, - CustomEvent: globalThis.CustomEvent, - EventTarget: globalThis.EventTarget, - kWeakHandler: require("internal/shared").kWeakHandler, + get "internal/event_target"() { + return (cachedInternalEventTarget ??= { + Event: globalThis.Event, + CustomEvent: globalThis.CustomEvent, + EventTarget: globalThis.EventTarget, + kWeakHandler: require("internal/shared").kWeakHandler, + }); }, // ChildProcess is the real class exec()/spawn() instantiate, so vendored // tests can monkeypatch its prototype; getValidStdio is ported from node. - // A getter so loading this module does not eagerly pull in child_process. get "internal/child_process"() { return (cachedInternalChildProcess ??= { ChildProcess: require("node:child_process").ChildProcess, getValidStdio: nodeGetValidStdio, }); }, - "internal/fs/utils": { - // Both are the REAL parsers the fs entry points use (FileSystemFlags::from_js - // and args::Rm::from_js), not JS reimplementations -- vendored tests assert - // the production behavior through these. - stringToFlags: $newRustFunction("node_fs_binding.rs", "string_to_flags_for_testing", 1), - validateRmOptionsSync: $newRustFunction("node_fs_binding.rs", "rm_options_for_testing", 2), + get "internal/fs/utils"() { + return (cachedInternalFsUtils ??= { + // Both are the REAL parsers the fs entry points use (FileSystemFlags::from_js + // and args::Rm::from_js), not JS reimplementations -- vendored tests assert + // the production behavior through these. + stringToFlags: $newRustFunction("node_fs_binding.rs", "string_to_flags_for_testing", 1), + validateRmOptionsSync: $newRustFunction("node_fs_binding.rs", "rm_options_for_testing", 2), + }); }, // internalBinding() is served by the registered "internal/test/binding" // module (src/js/internal/test/binding.ts), not from here. @@ -541,8 +581,6 @@ export function getWebStreamState(stream: ReadableStream | WritableStream): { } } -export const fs = require("node:fs/promises").$data; - export const fsStreamInternals = { writeStreamFastPath(str) { return str[require("internal/fs/streams").kWriteStreamFastPath]; diff --git a/test/internal/internal-for-testing.test.ts b/test/internal/internal-for-testing.test.ts new file mode 100644 index 000000000000..654dca7de6a2 --- /dev/null +++ b/test/internal/internal-for-testing.test.ts @@ -0,0 +1,57 @@ +// test/harness.ts requires bun:internal-for-testing from every test file (via +// test/preload.ts), so loading the module itself has to stay cheap: the node +// internals it exposes for `--expose-internals` tests must only be evaluated +// when a test asks for them. +import { exposedInternals } from "bun:internal-for-testing"; +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +const names = Object.keys(exposedInternals); + +test("every exposedInternals entry is a getter", () => { + expect(names).not.toBeEmpty(); + const eager = names.filter( + name => typeof Object.getOwnPropertyDescriptor(exposedInternals, name)?.get !== "function", + ); + expect(eager).toEqual([]); +}); + +test("every exposedInternals entry loads and is the same object on each access", () => { + const broken = names.filter(name => { + const first = exposedInternals[name]; + return first === undefined || exposedInternals[name] !== first; + }); + expect(broken).toEqual([]); +}); + +// harness.ts loads the module with require(); test files usually import it. +test.concurrent.each([ + ["require", `require("bun:internal-for-testing")`], + ["import", `await import("bun:internal-for-testing")`], +])("loading bun:internal-for-testing via %s does not evaluate the exposed internals", async (_, load) => { + // internal/util/colors, a dependency of the exposed internal/assert/myers_diff, + // reads FORCE_COLOR once, when it is evaluated. Setting the variable after + // loading bun:internal-for-testing and only then requiring colors therefore + // shows whether loading bun:internal-for-testing had already evaluated it: + // hasColors is false if it did, true if colors is first evaluated here. + const env = { ...bunEnv }; + // With either of these set, FORCE_COLOR also prints an "is ignored" warning to stderr. + delete env.NO_COLOR; + delete env.NODE_DISABLE_COLORS; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "--expose-internals", + "-e", + ` + ${load}; + process.env.FORCE_COLOR = "1"; + console.log(require("internal/util/colors").hasColors); + `, + ], + env, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "true", stderr: "", exitCode: 0 }); +});