diff --git a/src/codegen/bundle-modules.ts b/src/codegen/bundle-modules.ts index 9d6fd1f9482f..969761b088e2 100644 --- a/src/codegen/bundle-modules.ts +++ b/src/codegen/bundle-modules.ts @@ -354,6 +354,8 @@ function idToEnumName(id: string) { function idToPublicSpecifierOrEnumName(id: string) { if (id === "internal-for-testing.ts") return "bun:internal-for-testing"; // not in the `bun/` folder because it's added conditionally + // The undici shim only stands in for the npm package (HardcodedModule.rs `Alias::fallback`), so its key is not the package name. + if (id === "thirdparty/undici.js") return "internal:undici"; id = id.replace(/\.[mc]?[tj]s$/, ""); if (id.startsWith("node/")) { return "node:" + id.slice(5).replaceAll(".", "/"); diff --git a/src/js/internal/repl/node-shims.js b/src/js/internal/repl/node-shims.js index 66e3db29eb4c..467cbdea9794 100644 --- a/src/js/internal/repl/node-shims.js +++ b/src/js/internal/repl/node-shims.js @@ -167,11 +167,11 @@ let builtinLibs; function getBuiltinLibs() { if (!builtinLibs) { - // Bun's builtinModules also lists `bun`, `bun:*`, `undici`, `ws`; none - // resolve under `node:`, so exclude them so completion and the REPL - // global scope match Node's. + // Bun's builtinModules also lists `bun`, `bun:*`, `ws`; none resolve + // under `node:`, so exclude them so completion and the REPL global + // scope match Node's. builtinLibs = Module.builtinModules.filter( - id => !id.startsWith("_") && !id.startsWith("node:") && !id.startsWith("bun") && id !== "undici" && id !== "ws", + id => !id.startsWith("_") && !id.startsWith("node:") && !id.startsWith("bun") && id !== "ws", ); } return builtinLibs; diff --git a/src/js/internal/repl/utils.js b/src/js/internal/repl/utils.js index 36a96a9c4ba5..34cdd6f77ed8 100644 --- a/src/js/internal/repl/utils.js +++ b/src/js/internal/repl/utils.js @@ -801,7 +801,7 @@ function getREPLResourceName() { const globalBuiltins = new SafeSet(vm.runInNewContext("Object.getOwnPropertyNames(globalThis)")); // node-shims' getBuiltinLibs() also excludes Bun-specific entries (`bun*`, -// `undici`, `ws`) so completion doesn't offer e.g. `node:undici`. +// `ws`) so completion doesn't offer e.g. `node:ws`. let _builtinLibs = getBuiltinLibs().slice(); // Note: the `getReplBuiltinLibs` and `setReplBuiltinLibs` are functions used to provide getters and diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index c3e5b5698f81..9bf40b720bd1 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -58,7 +58,8 @@ bun_core::declare_scope!(cache, visible); /// Version 28: the define table and `--drop` entries participate in the features hash. /// Version 29: `new Array(x, ...spread)` is no longer folded into an array literal. /// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member. -const EXPECTED_VERSION: u32 = 30; +/// Version 31: `next/dist/compiled/undici` is rewritten to `internal:undici`, no longer `undici`. +const EXPECTED_VERSION: u32 = 31; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/src/jsc/bindings/isBuiltinModule.cpp b/src/jsc/bindings/isBuiltinModule.cpp index 409219f91ced..b55d4407082f 100644 --- a/src/jsc/bindings/isBuiltinModule.cpp +++ b/src/jsc/bindings/isBuiltinModule.cpp @@ -30,7 +30,6 @@ static constexpr ASCIILiteral builtinModuleNamesSortedLength[] = { "module"_s, "stream"_s, "timers"_s, - "undici"_s, "bun:ffi"_s, "bun:jsc"_s, "cluster"_s, diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index f4ba6173af35..5b9fc95969a0 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -114,7 +114,6 @@ static constexpr ASCIILiteral builtinModuleNames[] = { "tls"_s, "trace_events"_s, "tty"_s, - "undici"_s, "url"_s, "util"_s, "util/types"_s, diff --git a/src/resolve_builtins/HardcodedModule.rs b/src/resolve_builtins/HardcodedModule.rs index 0034aee4c5be..d062069a1025 100644 --- a/src/resolve_builtins/HardcodedModule.rs +++ b/src/resolve_builtins/HardcodedModule.rs @@ -119,7 +119,7 @@ pub enum HardcodedModule { NodeWorkerThreads, #[strum(serialize = "node:punycode")] NodePunycode, - #[strum(serialize = "undici")] + #[strum(serialize = "internal:undici")] Undici, #[strum(serialize = "ws")] Ws, @@ -297,7 +297,7 @@ bun_core::comptime_string_map! { b"node-fetch" => HardcodedModule::NodeFetch, b"isomorphic-fetch" => HardcodedModule::IsomorphicFetch, - b"undici" => HardcodedModule::Undici, + b"internal:undici" => HardcodedModule::Undici, b"ws" => HardcodedModule::Ws, b"@vercel/fetch" => HardcodedModule::VercelFetch, b"utf-8-validate" => HardcodedModule::Utf8Validate, @@ -747,7 +747,8 @@ const BUN_EXTRA_ALIAS_KVS: &[AliasKv] = &[ entry!("@vercel/fetch"), entry!("isomorphic-fetch"), entry!("node-fetch"), - entry!("undici"), + // Fallback only (`Alias::fallback`); not the npm name, so the key never re-resolves into node_modules. + entry!("internal:undici"), entry!("utf-8-validate"), entry!("ws"), ( @@ -794,7 +795,7 @@ const BUN_EXTRA_ALIAS_KVS: &[AliasKv] = &[ ( b"next/dist/compiled/undici", Alias { - path: zstr!("undici"), + path: zstr!("internal:undici"), tag: import_record::Tag::Builtin, node_builtin: false, node_only_prefix: false, @@ -920,6 +921,17 @@ impl Alias { Self::get(name, target, cfg).is_some() } + /// The builtin that stands in for `name` when node_modules resolution fails (#36098). + pub fn fallback(name: &[u8], target: Target) -> Option { + if !target.is_bun() { + return None; + } + match name { + b"undici" => Self::get(b"internal:undici", target, Cfg::default()), + _ => None, + } + } + pub fn get(name: &[u8], target: Target, cfg: Cfg) -> Option { // Without `--experimental-stream-iter` the aliases stay invisible so // the bare specifier falls through to filesystem resolution diff --git a/src/resolver/resolver.rs b/src/resolver/resolver.rs index 31dd498e7972..ff58f2ea7ceb 100644 --- a/src/resolver/resolver.rs +++ b/src/resolver/resolver.rs @@ -1358,8 +1358,28 @@ impl<'a> Resolver<'a> { return ResultUnion::NotFound; } - let mut tmp = - self.resolve_without_symlinks(source_dir_normalized, import_path, kind, global_cache); + // Probe without auto-install; the builtin stands in when nothing is installed. + let fallback = HardcodedAlias::fallback(import_path, self.opts.target); + let mut tmp = self.resolve_without_symlinks( + source_dir_normalized, + import_path, + kind, + if fallback.is_some() { + GlobalCache::disable + } else { + global_cache + }, + ); + if let Some(fallback) = fallback { + if matches!(tmp, ResultUnion::NotFound) { + let _ = self.flush_debug_logs(FlushMode::Success); + self.extension_order = original_order; + return ResultUnion::Success(Self::external_builtin( + fallback.path.as_bytes(), + kind, + )); + } + } // Fragments in URLs in CSS imports are technically expected to work if matches!(tmp, ResultUnion::NotFound) && kind.is_from_css() { @@ -1451,6 +1471,21 @@ impl<'a> Resolver<'a> { ret } + /// An external result that rewrites the import to the builtin `path`. + fn external_builtin(path: &'static [u8], kind: ast::ImportKind) -> Result { + Result { + import_kind: kind, + path_pair: PathPair { + primary: Path::init(path), + secondary: None, + }, + module_type: options::ModuleType::Cjs, + primary_side_effects_data: SideEffects::NoSideEffectsPureData, + flags: ResultFlags::IS_EXTERNAL | ResultFlags::REWRITE_IMPORT_PATH, + ..Default::default() + } + } + pub fn resolve( &mut self, source_dir: &[u8], @@ -5022,14 +5057,34 @@ impl<'a> Resolver<'a> { } } - return self.load_node_modules( + // As in `resolve_and_auto_install`: probe without auto-install, then fall back. + let fallback = HardcodedAlias::fallback(&esm_resolution.path, self.opts.target); + let status = self.load_node_modules( &esm_resolution.path, kind, dir_info, - global_cache, + if fallback.is_some() { + GlobalCache::disable + } else { + global_cache + }, true, out, ); + if let Some(fallback) = fallback { + if matches!(status, MatchStatus::NotFound) { + *out = MatchResult { + path_pair: PathPair { + primary: Fs::Path::init(fallback.path.as_bytes()), + secondary: None, + }, + is_external: true, + ..Default::default() + }; + return MatchStatus::Success; + } + } + return status; } self.handle_esm_resolution( diff --git a/test/bundler/bundler_bytecode_portable.test.ts b/test/bundler/bundler_bytecode_portable.test.ts index 87de2667bdf1..77b10549f408 100644 --- a/test/bundler/bundler_bytecode_portable.test.ts +++ b/test/bundler/bundler_bytecode_portable.test.ts @@ -514,10 +514,10 @@ describe("bytecode cache portability", () => { }, }, "bun build --bytecode libraries.js": { - "js": "493bab674ff49b287f26be3f356a3ad6681afb0c7eeffaa590f10cdcd8b58724", + "js": "e143c4f3946997a36432a5208ccee1f81d27235b37dcfdf3d6c3b7ba24269d7b", "jsc": { - "bytes": 23772944, - "sha256": "c0bb1ca0664a9c7146fadb951e4f6f801811918489f5777fdbbda101a6ffd6c6", + "bytes": 24856912, + "sha256": "51854d56d6f4f107b5b1dc21d7704b585f06401e3dda6d9f976a762093cf1de0", }, }, "bun build --bytecode lodash/lodash.js": { diff --git a/test/js/bun/util/fuzzy-wuzzy.test.ts b/test/js/bun/util/fuzzy-wuzzy.test.ts index 665f919057b8..7c7e9bc89a6e 100644 --- a/test/js/bun/util/fuzzy-wuzzy.test.ts +++ b/test/js/bun/util/fuzzy-wuzzy.test.ts @@ -434,7 +434,9 @@ const modules = [ "_http_server", "http2", "process", - "undici", + // Bun's undici shim. Bare "undici" resolves to the installed package when + // present; this alias always maps to the shim. + "next/dist/compiled/undici", "timers", "punycode", "trace_events", diff --git a/test/js/first_party/undici/undici-prefer-installed.test.ts b/test/js/first_party/undici/undici-prefer-installed.test.ts new file mode 100644 index 000000000000..a5ced1610d80 --- /dev/null +++ b/test/js/first_party/undici/undici-prefer-installed.test.ts @@ -0,0 +1,255 @@ +// Bun ships a builtin undici shim, but it is only a fallback: when the real +// package is installed in node_modules, bare "undici" must resolve to the +// installed copy (https://github.com/oven-sh/bun/issues/36098). +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import Module, { createRequire } from "node:module"; +import { join } from "node:path"; +// The Next.js compat alias always maps to the shim. +import { Agent as ShimAgent } from "next/dist/compiled/undici"; +// test/node_modules has the real undici installed. +import { Agent, Dispatcher, MockAgent } from "undici"; + +describe.concurrent("undici prefer-installed resolution", () => { + test("installed undici wins over the builtin shim", () => { + // The shim's MockAgent is an empty class; the real package has the + // interception API. + const agent = new MockAgent(); + expect(typeof agent.disableNetConnect).toBe("function"); + expect(typeof agent.get).toBe("function"); + + const resolved = require.resolve("undici"); + expect(resolved).not.toBe("undici"); + expect(resolved).toContain("node_modules"); + }); + + test("installed undici has a dispatcher layer (#42084)", () => { + // The shim's Dispatcher and Agent are empty classes. + const proto = Object.getOwnPropertyNames(Dispatcher.prototype); + expect(proto).toEqual(expect.arrayContaining(["dispatch", "close", "destroy", "request", "stream", "pipeline"])); + expect(typeof new Agent().dispatch).toBe("function"); + expect(typeof new ShimAgent().dispatch).toBe("undefined"); + + const esmRequire = createRequire(import.meta.url); + expect(esmRequire.resolve("undici")).toContain("node_modules"); + }); + + test("dynamic import agrees with the static import", async () => { + const installed = await import("undici"); + expect(installed.Agent).toBe(Agent); + + // The module loader resolves the shim's key a second time for a dynamic + // import. That must not turn it into the installed package. + const shim = await import("next/dist/compiled/undici"); + expect(shim.Agent).toBe(ShimAgent); + }); + + test("next/dist/compiled/undici still maps to the shim", () => { + // The Next.js compat alias bypasses node_modules on purpose. + expect(require.resolve("next/dist/compiled/undici")).toBe("internal:undici"); + }); + + test("require.resolve.paths treats undici as a regular package", () => { + const paths = require.resolve.paths("undici"); + expect(Array.isArray(paths)).toBe(true); + expect(paths!.length).toBeGreaterThan(0); + // Real builtins still report no search paths. + expect(require.resolve.paths("node:fs")).toBeNull(); + }); + + test("undici is not reported as a builtin module", () => { + expect(Module.isBuiltin("undici")).toBe(false); + expect(Module.builtinModules).not.toContain("undici"); + expect(process.getBuiltinModule("undici")).toBeUndefined(); + expect(Module.isBuiltin("node:fs")).toBe(true); + expect(process.getBuiltinModule("fs")).toBeDefined(); + }); + + test("subpath imports mapping to undici prefer the installed package", async () => { + using dir = tempDir("undici-subpath", { + "package.json": `{ "name": "app", "imports": { "#undici": "undici" } }`, + "node_modules/undici/package.json": `{ "name": "undici", "version": "99.0.0", "main": "index.js" }`, + "node_modules/undici/index.js": `module.exports = { marker: "installed" };`, + "main.mjs": ` + import u from "#undici"; + console.log(u.marker); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("installed\n"); + expect(exitCode).toBe(0); + }); + + test("subpath imports mapping to undici fall back to the shim", async () => { + using dir = tempDir("undici-subpath-builtin", { + "package.json": `{ "name": "app", "imports": { "#undici": "undici" } }`, + "main.mjs": ` + import u from "#undici"; + console.log(typeof u.request, typeof u.MockAgent); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("function function\n"); + expect(exitCode).toBe(0); + }); + + test("require and import pick up a locally installed undici", async () => { + using dir = tempDir("undici-installed", { + "package.json": `{ "name": "app", "dependencies": { "undici": "*" } }`, + "node_modules/undici/package.json": `{ "name": "undici", "version": "99.0.0", "main": "index.js" }`, + "node_modules/undici/index.js": ` + class MockAgent { + disableNetConnect() {} + get() {} + } + module.exports = { MockAgent, marker: "installed" }; + `, + "main.cjs": ` + const undici = require("undici"); + const agent = new undici.MockAgent(); + agent.disableNetConnect(); + console.log(undici.marker, require.resolve("undici").includes("node_modules")); + `, + "main.mjs": ` + import { MockAgent, marker } from "undici"; + new MockAgent().disableNetConnect(); + console.log(marker, import.meta.resolve("undici").includes("node_modules")); + `, + }); + + for (const entry of ["main.cjs", "main.mjs"]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), entry], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("installed true\n"); + expect(exitCode).toBe(0); + } + }); + + test("without an installed undici, the builtin shim is used", async () => { + using dir = tempDir("undici-builtin", { + "package.json": `{ "name": "app" }`, + "main.cjs": ` + const undici = require("undici"); + console.log(require.resolve("undici"), typeof undici.request, typeof undici.MockAgent); + `, + "main.mjs": ` + import { request, MockAgent } from "undici"; + const dynamic = await import("undici"); + console.log(import.meta.resolve("undici"), typeof request, typeof MockAgent, dynamic.request === request); + `, + }); + + for (const [entry, expected] of [ + ["main.cjs", "internal:undici function function\n"], + ["main.mjs", "internal:undici function function true\n"], + ]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), entry], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe(expected); + expect(exitCode).toBe(0); + } + }); + + test("bun build --target=bun bundles an installed undici", async () => { + using dir = tempDir("undici-build-installed", { + "package.json": `{ "name": "app", "dependencies": { "undici": "*" }, "imports": { "#undici": "undici" } }`, + "node_modules/undici/package.json": `{ "name": "undici", "version": "99.0.0", "main": "index.js" }`, + "node_modules/undici/index.js": `module.exports = { marker: "installed" };`, + "entry.mjs": ` + import { marker } from "undici"; + import viaImports from "#undici"; + console.log(marker, viaImports.marker); + `, + }); + + await using build = Bun.spawn({ + cmd: [bunExe(), "build", "--target=bun", "entry.mjs", "--outfile=out.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [, buildStderr, buildExitCode] = await Promise.all([build.stdout.text(), build.stderr.text(), build.exited]); + expect(buildStderr).toBe(""); + expect(buildExitCode).toBe(0); + + // The installed package is an ordinary dependency, so it is inlined. + const bundled = await Bun.file(join(String(dir), "out.mjs")).text(); + expect(bundled).toContain("installed"); + expect(bundled).not.toContain('"undici"'); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "out.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("installed installed\n"); + expect(exitCode).toBe(0); + }); + + test("bun build --target=bun keeps the shim external when undici is not installed", async () => { + using dir = tempDir("undici-build-shim", { + "package.json": `{ "name": "app", "imports": { "#undici": "undici" } }`, + "entry.mjs": ` + import { request } from "undici"; + import viaImports from "#undici"; + console.log(typeof request, typeof viaImports.request); + `, + }); + + await using build = Bun.spawn({ + cmd: [bunExe(), "build", "--target=bun", "entry.mjs", "--outfile=out.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [, buildStderr, buildExitCode] = await Promise.all([build.stdout.text(), build.stderr.text(), build.exited]); + expect(buildStderr).toBe(""); + expect(buildExitCode).toBe(0); + + const bundled = await Bun.file(join(String(dir), "out.mjs")).text(); + expect(bundled).toContain('"internal:undici"'); + expect(bundled).not.toContain("#undici"); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "out.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("function function\n"); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/first_party/undici/undici-primordials.test.ts b/test/js/first_party/undici/undici-primordials.test.ts index 805df11e4c05..9267f4c18683 100644 --- a/test/js/first_party/undici/undici-primordials.test.ts +++ b/test/js/first_party/undici/undici-primordials.test.ts @@ -22,7 +22,9 @@ it("undici", () => { globalThis.URLSearchParams = 42; - const undici = require("undici"); + // Bare "undici" resolves to the installed package (test/node_modules has + // one); the "next/dist/compiled/undici" alias always maps to Bun's shim. + const undici = require("next/dist/compiled/undici"); expect(undici).toBeDefined(); expect(undici.Response).toBe(Response); expect(undici.Request).toBe(Request); diff --git a/test/js/first_party/undici/undici.test.ts b/test/js/first_party/undici/undici.test.ts index c966f50b52e7..31be328c1994 100644 --- a/test/js/first_party/undici/undici.test.ts +++ b/test/js/first_party/undici/undici.test.ts @@ -1,6 +1,9 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { Readable } from "node:stream"; -import { request, fetch as undiciFetch } from "undici"; +// These tests cover Bun's undici shim. A bare "undici" import resolves to the +// installed package (test/node_modules has one), so go through the +// "next/dist/compiled/undici" alias, which always maps to the shim. +import { request, fetch as undiciFetch } from "next/dist/compiled/undici"; import { createServer } from "../../../http-test-server"; diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index dc32ed69bec2..9958107ace4b 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -10,7 +10,7 @@ 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(75); }); test("isBuiltin() works", () => {