From 8baf99b07d4ba2969000c248da4ed2d40b512d4d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 19 Aug 2026 23:19:52 +0000 Subject: [PATCH 1/4] node:module: list node:test in builtinModules Node lists the modules that only resolve with the node: prefix under that prefixed name in module.builtinModules. Bun already does this for node:sqlite. node:test was missing from the list although isBuiltin("node:test") is true and require("node:test") works. --- src/jsc/modules/NodeModuleModule.cpp | 4 ++++ test/js/node/module/node-module-module.test.js | 16 +++++++++++++++- test/js/node/test_runner/node-test.test.ts | 9 +++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index 6b94b9af3f97..8cb19937f888 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -47,6 +47,9 @@ JSC_DECLARE_CUSTOM_SETTER(setterRequireFunction); // The reason for overstuffing this list is so that uses that use these as the // 'external' option to a bundler will properly exclude things like 'ws' which // only work with Bun's native 'ws' implementation and not the JS one on NPM. +// +// Modules that only resolve with the "node:" prefix ("node:sqlite", +// "node:test") are listed with the prefix, as in Node's module.builtinModules. static constexpr ASCIILiteral builtinModuleNames[] = { "_http_agent"_s, "_http_client"_s, @@ -92,6 +95,7 @@ static constexpr ASCIILiteral builtinModuleNames[] = { "module"_s, "net"_s, "node:sqlite"_s, + "node:test"_s, "os"_s, "path"_s, "path/posix"_s, diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index f812826ccfdf..f2ecfb955854 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -9,7 +9,21 @@ describe.concurrent("node-module-module", () => { expect(Array.isArray(builtinModules)).toBe(true); // "bun:wrap" is no longer listed: it is internal transpiler plumbing, // not a requireable public module. - expect(builtinModules).toHaveLength(76); + expect(builtinModules).toHaveLength(77); + }); + + test("builtinModules lists prefix-only node modules with their prefix, as Node does", () => { + const prefixed = builtinModules.filter(name => name.startsWith("node:")); + // Node lists these with the prefix because the bare names are not builtins. "node:quic" is left out on + // purpose: Node 26 does not list it unless QUIC is compiled in and enabled with --experimental-quic. + expect(prefixed).toEqual(["node:sqlite", "node:test"]); + expect(builtinModules).not.toContain("test"); + // Every entry resolves as a builtin, and a prefixed entry is requireable only with its prefix. + expect(builtinModules.filter(name => !isBuiltin(name))).toEqual([]); + for (const name of prefixed) { + expect(isBuiltin(name.slice("node:".length))).toBe(false); + expect(process.getBuiltinModule(name)).toBe(require(name)); + } }); test("isBuiltin() works", () => { diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 2a27963203c0..2725100a73a9 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -1,9 +1,18 @@ import { spawn } from "bun"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; +import { builtinModules, isBuiltin } from "node:module"; import { join } from "node:path"; describe("node:test", () => { + test("node:test is a built-in module", () => { + expect(isBuiltin("node:test")).toBe(true); + // Like node:sqlite, node:test is only available with the node: prefix. + expect(isBuiltin("test")).toBe(false); + expect(builtinModules).toContain("node:test"); + expect(builtinModules).not.toContain("test"); + }); + // These three drive the largest fixtures (01-harness has 32 node:test cases); // a debug+ASAN `bun test` child takes several seconds to start, so give them // headroom and let them spawn in parallel instead of serially. From 08b77b702d069812b75555ba89d2b172996f116c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 20 Aug 2026 04:37:15 +0000 Subject: [PATCH 2/4] Lint the builtin module tables against each other module.builtinModules and module.isBuiltin() read two hand-written tables, and the module loader has a third marker for the builtins that resolve with the node: prefix only. A new module tends to reach the last two and not the first. The new source lint requires every name isBuiltin() accepts to be listed in builtinModules, with an allowlist that names the reasons for the deliberate exceptions. The runtime test now checks the array's properties instead of its length, and the copy of those assertions in node-test.test.ts is gone. --- .github/workflows/source-lints.yml | 2 + src/jsc/modules/NodeModuleModule.cpp | 16 ++-- .../builtin-module-tables.test.ts | 73 +++++++++++++++++++ .../js/node/module/node-module-module.test.js | 21 ++---- test/js/node/test_runner/node-test.test.ts | 9 --- 5 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 test/internal/source-lints/builtin-module-tables.test.ts diff --git a/.github/workflows/source-lints.yml b/.github/workflows/source-lints.yml index 98775deb80d2..a9e14c51d22a 100644 --- a/.github/workflows/source-lints.yml +++ b/.github/workflows/source-lints.yml @@ -17,6 +17,7 @@ on: - "src/codegen/class-definitions.ts" - "src/js/builtins.d.ts" - "src/jsc/bindings/**" + - "src/jsc/modules/NodeModuleModule.cpp" - "packages/bun-types/redis.d.ts" - "scripts/build/**" - "scripts/glob-sources.ts" @@ -38,6 +39,7 @@ on: - "src/codegen/class-definitions.ts" - "src/js/builtins.d.ts" - "src/jsc/bindings/**" + - "src/jsc/modules/NodeModuleModule.cpp" - "packages/bun-types/redis.d.ts" - "scripts/build/**" - "scripts/glob-sources.ts" diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index 8cb19937f888..54e6e1de9d10 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -42,14 +42,16 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionWrap); JSC_DECLARE_CUSTOM_GETTER(getterRequireFunction); JSC_DECLARE_CUSTOM_SETTER(setterRequireFunction); -// This is a list of builtin module names that do not have the node prefix. It -// also includes Bun's builtin modules, as well as Bun's thirdparty overrides. -// The reason for overstuffing this list is so that uses that use these as the -// 'external' option to a bundler will properly exclude things like 'ws' which -// only work with Bun's native 'ws' implementation and not the JS one on NPM. +// The contents of module.builtinModules. As in Node, a builtin is listed +// without the "node:" prefix unless it only resolves with the prefix, like +// "node:sqlite" and "node:test". The list also includes Bun's builtin modules +// and Bun's thirdparty overrides: users pass this list as the 'external' +// option of a bundler, and that has to exclude things like 'ws', which only +// works with Bun's native 'ws' implementation and not the JS one on NPM. // -// Modules that only resolve with the "node:" prefix ("node:sqlite", -// "node:test") are listed with the prefix, as in Node's module.builtinModules. +// Bun::isBuiltinModule() (isBuiltinModule.cpp) accepts this list plus a few +// internal names. test/internal/source-lints/builtin-module-tables.test.ts +// keeps the two tables in sync. static constexpr ASCIILiteral builtinModuleNames[] = { "_http_agent"_s, "_http_client"_s, diff --git a/test/internal/source-lints/builtin-module-tables.test.ts b/test/internal/source-lints/builtin-module-tables.test.ts new file mode 100644 index 000000000000..b62053f18fa5 --- /dev/null +++ b/test/internal/source-lints/builtin-module-tables.test.ts @@ -0,0 +1,73 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import path from "node:path"; + +// The names of the builtin modules live in three hand-written tables: +// +// - builtinModuleNames in src/jsc/modules/NodeModuleModule.cpp is module.builtinModules. +// - builtinModuleNamesSortedLength in src/jsc/bindings/isBuiltinModule.cpp is what module.isBuiltin() +// accepts (and what Bun.plugin refuses to override). +// - node_entry_only_prefix!() in src/resolve_builtins/HardcodedModule.rs marks the builtins that the module +// loader resolves with the "node:" prefix only. Node lists those in builtinModules with the prefix. +// +// Nothing else compares them. A new module tends to land in the last two and not in the first: "node:test" +// was missing from builtinModules for over a year while isBuiltin("node:test") was true. This lint requires +// every name isBuiltin() accepts to be listed in builtinModules, unless notListed says why it is not. + +// isBuiltin() names that builtinModules leaves out on purpose, with the reason. The lint fails while an entry +// here is listed after all, or is no longer accepted by isBuiltin(). +const internalPlumbing = + "internal plumbing, not a public module (#31831). isBuiltin() accepts it so that Bun.plugin refuses to override it."; +const notListed: Record = { + "bun:main": internalPlumbing, + "bun:wrap": internalPlumbing, + "node:quic": "experimental. Stock Node 26 builds do not list it either: QUIC is compiled out of them.", +}; + +const builtinModulesFile = "src/jsc/modules/NodeModuleModule.cpp"; +const isBuiltinFile = "src/jsc/bindings/isBuiltinModule.cpp"; +const resolverFile = "src/resolve_builtins/HardcodedModule.rs"; +const root = path.resolve(import.meta.dir, "..", "..", ".."); + +function asciiLiteralTable(file: string, name: string): Set { + const source = readFileSync(path.join(root, file), "utf8"); + const table = new RegExp(String.raw`\b${name}\[\] = \{([^}]*)\};`).exec(source); + if (table === null) throw new Error(`${file} no longer defines the table ${name}[]`); + const names = new Set(Array.from(table[1].matchAll(/"([^"]+)"_s/g), m => m[1])); + if (names.size === 0) throw new Error(`${file}: the table ${name}[] has no "..."_s entries`); + return names; +} + +const builtinModules = asciiLiteralTable(builtinModulesFile, "builtinModuleNames"); +const isBuiltin = asciiLiteralTable(isBuiltinFile, "builtinModuleNamesSortedLength"); +const onlyPrefix = Array.from( + readFileSync(path.join(root, resolverFile), "utf8").matchAll(/\bnode_entry_only_prefix!\("([^"]+)"\)/g), + m => m[1], +); +if (onlyPrefix.length === 0) throw new Error(`${resolverFile} has no node_entry_only_prefix!() entries`); + +test(`${builtinModulesFile} lists every name ${isBuiltinFile} accepts, unless notListed says why not`, () => { + const missing = [...isBuiltin].filter(name => !builtinModules.has(name) && !Object.hasOwn(notListed, name)).sort(); + expect(missing).toEqual([]); +}); + +test(`${isBuiltinFile} accepts every name ${builtinModulesFile} lists`, () => { + const unknown = [...builtinModules].filter(name => !isBuiltin.has(name)).sort(); + expect(unknown).toEqual([]); +}); + +test(`${isBuiltinFile} accepts every prefix-only builtin of ${resolverFile} under its prefixed name`, () => { + const unknown = onlyPrefix.filter(name => !isBuiltin.has(name)).sort(); + expect(unknown).toEqual([]); +}); + +test("notListed names only entries that isBuiltin() still accepts and builtinModules still leaves out", () => { + const stale = Object.keys(notListed) + .flatMap(name => { + if (!isBuiltin.has(name)) return [`${name} is no longer in ${isBuiltinFile}; delete its entry`]; + if (builtinModules.has(name)) return [`${name} is listed in ${builtinModulesFile} now; delete its entry`]; + return []; + }) + .sort(); + expect(stale).toEqual([]); +}); diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index f2ecfb955854..ad0a3a4a6974 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -5,22 +5,17 @@ import Module, { _nodeModulePaths, builtinModules, createRequire, isBuiltin, wra import path from "path"; describe.concurrent("node-module-module", () => { - test("builtinModules exists", () => { + // test/internal/source-lints/builtin-module-tables.test.ts checks the table behind builtinModules against the + // table behind isBuiltin(), so this only checks what the array looks like at runtime. + test("builtinModules holds requireable builtins and lists prefix-only modules with their prefix, as Node does", () => { expect(Array.isArray(builtinModules)).toBe(true); - // "bun:wrap" is no longer listed: it is internal transpiler plumbing, - // not a requireable public module. - expect(builtinModules).toHaveLength(77); - }); - - test("builtinModules lists prefix-only node modules with their prefix, as Node does", () => { - const prefixed = builtinModules.filter(name => name.startsWith("node:")); - // Node lists these with the prefix because the bare names are not builtins. "node:quic" is left out on - // purpose: Node 26 does not list it unless QUIC is compiled in and enabled with --experimental-quic. - expect(prefixed).toEqual(["node:sqlite", "node:test"]); + expect(builtinModules).toContain("node:test"); expect(builtinModules).not.toContain("test"); - // Every entry resolves as a builtin, and a prefixed entry is requireable only with its prefix. + // "bun:wrap" is internal transpiler plumbing, not a requireable public module. + expect(builtinModules).not.toContain("bun:wrap"); expect(builtinModules.filter(name => !isBuiltin(name))).toEqual([]); - for (const name of prefixed) { + // A prefixed entry is listed that way because it is requireable only with its prefix. + for (const name of builtinModules.filter(name => name.startsWith("node:"))) { expect(isBuiltin(name.slice("node:".length))).toBe(false); expect(process.getBuiltinModule(name)).toBe(require(name)); } diff --git a/test/js/node/test_runner/node-test.test.ts b/test/js/node/test_runner/node-test.test.ts index 2725100a73a9..2a27963203c0 100644 --- a/test/js/node/test_runner/node-test.test.ts +++ b/test/js/node/test_runner/node-test.test.ts @@ -1,18 +1,9 @@ import { spawn } from "bun"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; -import { builtinModules, isBuiltin } from "node:module"; import { join } from "node:path"; describe("node:test", () => { - test("node:test is a built-in module", () => { - expect(isBuiltin("node:test")).toBe(true); - // Like node:sqlite, node:test is only available with the node: prefix. - expect(isBuiltin("test")).toBe(false); - expect(builtinModules).toContain("node:test"); - expect(builtinModules).not.toContain("test"); - }); - // These three drive the largest fixtures (01-harness has 32 node:test cases); // a debug+ASAN `bun test` child takes several seconds to start, so give them // headroom and let them spawn in parallel instead of serially. From f815c682eee0959dad240cc898ba064414a2902b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 20 Aug 2026 04:39:39 +0000 Subject: [PATCH 3/4] Shorten the builtinModuleNames comment --- src/jsc/modules/NodeModuleModule.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index 54e6e1de9d10..c88c3a45de3f 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -42,16 +42,12 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionWrap); JSC_DECLARE_CUSTOM_GETTER(getterRequireFunction); JSC_DECLARE_CUSTOM_SETTER(setterRequireFunction); -// The contents of module.builtinModules. As in Node, a builtin is listed -// without the "node:" prefix unless it only resolves with the prefix, like -// "node:sqlite" and "node:test". The list also includes Bun's builtin modules -// and Bun's thirdparty overrides: users pass this list as the 'external' -// option of a bundler, and that has to exclude things like 'ws', which only -// works with Bun's native 'ws' implementation and not the JS one on NPM. -// -// Bun::isBuiltinModule() (isBuiltinModule.cpp) accepts this list plus a few -// internal names. test/internal/source-lints/builtin-module-tables.test.ts -// keeps the two tables in sync. +// module.builtinModules. As in Node, a builtin is listed without the "node:" +// prefix unless it only resolves with the prefix ("node:sqlite", "node:test"). +// Bun's own modules and Bun's thirdparty overrides are listed too, so that +// users who pass this list as a bundler's 'external' option exclude things +// like 'ws', which only works with Bun's native implementation and not the JS +// one on NPM. builtin-module-tables.test.ts checks it against isBuiltinModule.cpp. static constexpr ASCIILiteral builtinModuleNames[] = { "_http_agent"_s, "_http_client"_s, From f06a92c3f44de5ce9ed8afb4433407c6dc331071 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 20 Aug 2026 04:42:04 +0000 Subject: [PATCH 4/4] Only correct the first sentence of the builtinModuleNames comment --- src/jsc/modules/NodeModuleModule.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index c88c3a45de3f..5ae91f914d63 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -42,12 +42,12 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionWrap); JSC_DECLARE_CUSTOM_GETTER(getterRequireFunction); JSC_DECLARE_CUSTOM_SETTER(setterRequireFunction); -// module.builtinModules. As in Node, a builtin is listed without the "node:" -// prefix unless it only resolves with the prefix ("node:sqlite", "node:test"). -// Bun's own modules and Bun's thirdparty overrides are listed too, so that -// users who pass this list as a bundler's 'external' option exclude things -// like 'ws', which only works with Bun's native implementation and not the JS -// one on NPM. builtin-module-tables.test.ts checks it against isBuiltinModule.cpp. +// module.builtinModules. Node builtins are listed without the "node:" prefix, +// except those that only resolve with it ("node:sqlite", "node:test"). It +// also includes Bun's builtin modules, as well as Bun's thirdparty overrides. +// The reason for overstuffing this list is so that uses that use these as the +// 'external' option to a bundler will properly exclude things like 'ws' which +// only work with Bun's native 'ws' implementation and not the JS one on NPM. static constexpr ASCIILiteral builtinModuleNames[] = { "_http_agent"_s, "_http_client"_s,