From 49fa6c69ca6749fdf413d2f16d88c9980d5da781 Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 00:34:15 +0000 Subject: [PATCH 01/12] install: keep type-shipping packages project-local in the isolated linker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TypeScript resolves peer type references (e.g. `React.FunctionComponent` inside a package's `.d.ts` that never declared `@types/react` as a peerDep) by walking `node_modules` ancestors from the declaration file's realpath. When the isolated linker's global virtual store holds the package, its realpath is `/links/@-/` — a directory tree with no ancestor `node_modules` under which the project's `@types/*` devDependencies live, so the peer type silently resolves to `any` and downstream JSX usage fails with TS2604 / TS2786. Detect packages that ship types — top-level `"types"` or `"typings"`, or a `"types"` condition under `"exports"` — and mark them ineligible for the global store. They then materialize as real directories under `node_modules/.bun/@/`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/` and resolves the project's types. Results are memoized per `pkg_id` and read from the extracted `//package.json` so each package is scanned once per install. Packages whose package.json can't be read fall back to the pre-carve-out behavior. Fixes #29727. --- docs/pm/global-store.mdx | 3 + src/install/isolated_install.zig | 168 +++++++++++++++++++++- test/cli/install/isolated-install.test.ts | 144 +++++++++++++++++++ 3 files changed, 311 insertions(+), 4 deletions(-) diff --git a/docs/pm/global-store.mdx b/docs/pm/global-store.mdx index 54d598a95f8..6fa8711ddb6 100644 --- a/docs/pm/global-store.mdx +++ b/docs/pm/global-store.mdx @@ -119,6 +119,7 @@ An entry only lives in the global store when it can be safely shared. Entries fa - the package has a **patch** applied via `bun patch` — the patched contents are project-specific; - the package is listed in **`trustedDependencies`** (or trusted via `bun add --trust`) — its lifecycle script may mutate the install directory, and a script running through the project symlink would mutate the shared copy; +- the package **ships TypeScript declarations** — a top-level `"types"` or `"typings"` field in `package.json`, or a `"types"` condition under `"exports"`. TypeScript resolves peer types (e.g. `React.FunctionComponent` in a package that never declared `@types/react` as a peer dependency) by walking `node_modules` ancestors from the declaration file's realpath; if that realpath is in the shared store those walks never reach the project's `@types/*`. Keeping type-shipping packages project-local puts their realpath back under `node_modules/.bun//`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/`. - the package, or **any** dependency it links to, is a `workspace:`, `file:`, or `link:` dependency — those resolve to project-local paths that other projects can't see. Ineligibility propagates: if `your-app` depends on `internal-utils` which is a workspace package, `internal-utils` is project-local, and so is every entry that links to it. An entry that loses eligibility between installs (newly patched, newly trusted) is detached from the global store and rebuilt project-locally on the next install; the shared entry is left untouched. @@ -135,6 +136,8 @@ When packages live under the project's `node_modules/.bun/`, Node's module resol In practice this only affects **true phantom dependencies**: a package doing `require('helper')` for something it never declared in `dependencies`, `peerDependencies`, or `peerDependenciesMeta`. If you hit this, add the helper to the consuming package's dependencies (the right fix) or set `globalStore = false`. +TypeScript triggers the same walk for peer type references inside a package's `.d.ts` files. Packages that ship types are detected (see [What stays project-local](#what-stays-project-local)) and kept out of the global store so their ancestor walk still reaches the project's `@types/*`; purely JavaScript packages with phantom `require('helper')` calls are the remaining case. + Note that [`publicHoistPattern`](/runtime/bunfig#install-publichoistpattern) and [`hoistPattern`](/runtime/bunfig#install-hoistpattern) hoist into the project's `node_modules`, which packages inside the global store can't reach. They still work for resolving hoisted packages from your own source code. ### `node_modules` is mostly symlinks diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index 521e638b010..eb576c83e8b 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -854,10 +854,25 @@ pub fn installIsolatedPackages( // // Eligibility propagates: an entry is only global-store-eligible (hash != 0) // when the package itself comes from an immutable cache (npm/git/tarball, - // unpatched, no lifecycle scripts) *and* every dependency it links to is - // also eligible. The second condition matters because dep symlinks live - // inside the global entry; baking a project-local path (workspace, folder) - // into a shared directory would break for every other consumer. + // unpatched, no lifecycle scripts, does not ship TypeScript declarations) + // *and* every dependency it links to is also eligible. The dep-closure + // condition matters because dep symlinks live inside the global entry; + // baking a project-local path (workspace, folder) into a shared directory + // would break for every other consumer. + // + // The "ships TypeScript declarations" carve-out exists because TypeScript + // resolves peer type references (e.g. `React.FunctionComponent` inside a + // package's `.d.ts` that never declares `@types/react` as a peerDep) by + // walking `node_modules` ancestors from the file's *realpath*. A package + // in `/links/@-/` has no ancestor `node_modules` + // under which the project's `@types/*` devDependencies are visible, so + // the peer type silently resolves to `any`. Keeping type-shipping + // packages project-local puts their realpath back under + // `node_modules/.bun/@/`, whose ancestor walk reaches the + // hidden hoisted layer at `node_modules/.bun/node_modules/`. + var pkg_ships_types_cache: std.AutoHashMapUnmanaged(PackageID, bool) = .empty; + defer pkg_ships_types_cache.deinit(manager.allocator); + const WyhashWriter = struct { hasher: *std.hash.Wyhash, const E = error{}; @@ -976,6 +991,15 @@ pub fn installIsolatedPackages( { break :eligible false; } + if (packageShipsTypeDeclarations(manager, pkg_id, &pkg_ships_types_cache)) { + // See the block comment on the eligibility DFS: a + // package whose `.d.ts` references peer types it + // never declared (the usual React-component pattern) + // needs its realpath to sit under the project's + // `node_modules/.bun/` so TypeScript's ancestor walk + // can reach the hidden hoisted `@types/*` layer. + break :eligible false; + } break :eligible true; }, else => false, @@ -1927,15 +1951,151 @@ pub fn installIsolatedPackages( } } +/// Does the package at `pkg_id` declare TypeScript type declarations? +/// +/// Returns true if its extracted `package.json` at +/// `//package.json` contains a top-level `"types"` or +/// `"typings"` string field, or a `"types"` key inside a conditional +/// `"exports"` entry. Either signal means the package intentionally ships +/// `.d.ts` files that TypeScript will walk ancestor directories from — +/// see the eligibility-DFS comment for why that rules the package out of +/// the global virtual store. +/// +/// Results are memoized in `cache` so each distinct package is scanned at +/// most once per install. If the extracted `package.json` is unreadable +/// (cache evicted, extraction deferred) we conservatively report `false` +/// and fall back to the pre-carve-out behavior — a package whose types we +/// couldn't inspect just stays in whichever bucket the rest of the +/// eligibility check places it. +fn packageShipsTypeDeclarations( + manager: *PackageManager, + pkg_id: PackageID, + cache: *std.AutoHashMapUnmanaged(PackageID, bool), +) bool { + const gop = cache.getOrPut(manager.allocator, pkg_id) catch bun.outOfMemory(); + if (gop.found_existing) return gop.value_ptr.*; + gop.value_ptr.* = false; + + const pkgs = manager.lockfile.packages.slice(); + const pkg_names = pkgs.items(.name); + const pkg_resolutions = pkgs.items(.resolution); + const string_buf = manager.lockfile.buffers.string_bytes.items; + + const pkg_res: Resolution = pkg_resolutions[pkg_id]; + var folder_buf: bun.PathBuffer = undefined; + const folder: [:0]const u8 = switch (pkg_res.tag) { + .npm => manager.cachedNPMPackageFolderNamePrint( + &folder_buf, + pkg_names[pkg_id].slice(string_buf), + pkg_res.value.npm.version, + null, + ), + .git => PackageManager.cachedGitFolderNamePrint(&folder_buf, manager.lockfile.str(&pkg_res.value.git.resolved), null), + .github => PackageManager.cachedGitHubFolderNamePrint(&folder_buf, manager.lockfile.str(&pkg_res.value.github.resolved), null), + .local_tarball => PackageManager.cachedTarballFolderNamePrint(&folder_buf, manager.lockfile.str(&pkg_res.value.local_tarball), null), + .remote_tarball => PackageManager.cachedTarballFolderNamePrint(&folder_buf, manager.lockfile.str(&pkg_res.value.remote_tarball), null), + else => return false, + }; + + const cache_dir, const cache_dir_path = manager.getCacheDirectoryAndAbsPath(); + defer cache_dir_path.deinit(); + + var rel: bun.RelPath(.{ .sep = .auto }) = .from(folder); + defer rel.deinit(); + rel.append("package.json"); + + const source_bytes = switch (bun.sys.File.readFrom(cache_dir, rel.sliceZ(), manager.allocator)) { + .result => |b| b, + .err => return false, + }; + defer manager.allocator.free(source_bytes); + + gop.value_ptr.* = scanForTypeDeclarationSignals(source_bytes); + return gop.value_ptr.*; +} + +/// Scans `package.json` bytes for a top-level `"types"` / `"typings"` +/// string field, or a nested `"types"` key inside a conditional `"exports"` +/// entry. Uses the shared JSON parser (cheaper to maintain than a +/// hand-rolled scanner; a typical `package.json` is a few KB, well inside +/// what the registry manifest cache already parses routinely during +/// install). +fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { + var arena = std.heap.ArenaAllocator.init(bun.default_allocator); + defer arena.deinit(); + const arena_allocator = arena.allocator(); + + var log_sink: logger.Log = .init(arena_allocator); + defer log_sink.deinit(); + + const source = logger.Source.initPathString("package.json", source_bytes); + + // `initializeStore()` is idempotent but cheap — it sets up the thread- + // local expression arena the JSON parser writes into. Every call site + // that uses `JSON.parsePackageJSONUTF8` is expected to call it; we + // mirror that here. + install.initializeStore(); + const json = JSON.parsePackageJSONUTF8(&source, &log_sink, arena_allocator) catch return false; + + if (json.asProperty("types")) |prop| { + if (prop.expr.asString(arena_allocator)) |s| { + if (s.len > 0) return true; + } + } + if (json.asProperty("typings")) |prop| { + if (prop.expr.asString(arena_allocator)) |s| { + if (s.len > 0) return true; + } + } + if (json.asProperty("exports")) |prop| { + if (exportsHasTypesCondition(prop.expr)) return true; + } + return false; +} + +/// Conditional-exports entries can be: +/// +/// "exports": { ".": { "types": "./index.d.ts", "default": "./index.js" } } +/// "exports": { "import": { "types": "./a.d.ts", "default": "./a.js" } } +/// "exports": { "types": "./index.d.ts", "default": "./index.js" } +/// +/// We treat ANY `"types"` key inside the exports tree as a signal. The +/// recursion depth is bounded by how deeply a `package.json` nests +/// conditions (1-3 in practice), and we cap it defensively. +fn exportsHasTypesCondition(expr: bun.js_parser.Expr) bool { + return exportsHasTypesConditionInner(expr, 0); +} + +fn exportsHasTypesConditionInner(expr: bun.js_parser.Expr, depth: u8) bool { + if (depth > 8) return false; + if (expr.data != .e_object) return false; + for (expr.data.e_object.properties.slice()) |prop| { + const key = prop.key orelse continue; + const value = prop.value orelse continue; + if (key.data == .e_string and + key.data.e_string.eqlComptime("types") and + value.data == .e_string) + { + return true; + } + if (value.data == .e_object) { + if (exportsHasTypesConditionInner(value, depth + 1)) return true; + } + } + return false; +} + const std = @import("std"); const bun = @import("bun"); const Environment = bun.Environment; const FD = bun.FD; const Global = bun.Global; +const JSON = bun.json; const OOM = bun.OOM; const Output = bun.Output; const Progress = bun.Progress; +const logger = bun.logger; const sys = bun.sys; const Command = bun.cli.Command; diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index a406101e411..9f5114fada7 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -1951,4 +1951,148 @@ describe("global virtual store", () => { expect(lstatSync(workspace).isSymbolicLink()).toBe(false); expect(await file(edited).text()).toBe("module.exports = 'USER_EDITS';\n"); }); + + test("packages that ship TypeScript declarations stay project-local (#29727)", async () => { + // A package whose `.d.ts` references peer types it never declared (the + // canonical React-component-typings pattern, see `next-yak@9.4.1`) needs + // its realpath to sit under the project's `node_modules/.bun/` — not + // in `/links/` — so TypeScript's ancestor walk from the + // declaration file can still reach `node_modules/.bun/node_modules/` + // and resolve the project's hoisted `@types/*`. When the global store + // is enabled, a package that ships types (top-level `"types"` or + // `"typings"`, or a `"types"` key under `"exports"`) is materialized + // as a real directory under `node_modules/.bun/`; a package without + // any of those signals stays on the shared-store fast path as a + // symlink into `/links/`. + // + // Fixtures are packed with `bun pm pack` and served from an in-process + // HTTP registry so the test doesn't depend on the shared Verdaccio. + using fixtures = tempDir("ships-types-fixtures-", { + "top-types/package.json": JSON.stringify({ + name: "top-types", + version: "1.0.0", + types: "./index.d.ts", + }), + "top-types/index.js": "module.exports = {};\n", + "top-types/index.d.ts": "export {};\n", + + "top-typings/package.json": JSON.stringify({ + name: "top-typings", + version: "1.0.0", + typings: "./index.d.ts", + }), + "top-typings/index.js": "module.exports = {};\n", + "top-typings/index.d.ts": "export {};\n", + + "exports-types/package.json": JSON.stringify({ + name: "exports-types", + version: "1.0.0", + exports: { + ".": { + types: "./index.d.ts", + default: "./index.js", + }, + }, + }), + "exports-types/index.js": "module.exports = {};\n", + "exports-types/index.d.ts": "export {};\n", + + "pure-js/package.json": JSON.stringify({ + name: "pure-js", + version: "1.0.0", + }), + "pure-js/index.js": "module.exports = {};\n", + }); + + async function pack(subdir: string): Promise { + await using proc = spawn({ + cmd: [bunExe(), "pm", "pack", "--destination", String(fixtures)], + cwd: join(String(fixtures), subdir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + if (exitCode !== 0) throw new Error(`bun pm pack failed: ${stderr}`); + return Buffer.from(await Bun.file(join(String(fixtures), `${subdir}-1.0.0.tgz`)).arrayBuffer()); + } + + const tarballs: Record = { + "top-types": await pack("top-types"), + "top-typings": await pack("top-typings"), + "exports-types": await pack("exports-types"), + "pure-js": await pack("pure-js"), + }; + + using server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + const tarballMatch = url.pathname.match(/^\/([^/]+)\/-\/([^/]+)\.tgz$/); + if (tarballMatch) { + const tgz = tarballs[tarballMatch[1]]; + if (!tgz) return new Response("Not found", { status: 404 }); + return new Response(tgz, { headers: { "Content-Type": "application/octet-stream" } }); + } + const name = decodeURIComponent(url.pathname.slice(1)); + const tgz = tarballs[name]; + if (!tgz) return new Response("Not found", { status: 404 }); + return new Response( + JSON.stringify({ + name, + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name, + version: "1.0.0", + dist: { + tarball: `http://localhost:${server.port}/${name}/-/${name}-1.0.0.tgz`, + }, + }, + }, + }), + { headers: { "Content-Type": "application/json" } }, + ); + }, + }); + + using packageDir = tempDir("ships-types-test-", {}); + const cacheDir = join(String(packageDir), ".bun-cache"); + await write( + join(String(packageDir), "bunfig.toml"), + `[install]\ncache = "${cacheDir.replaceAll("\\", "\\\\")}"\nregistry = "http://localhost:${server.port}/"\nlinker = "isolated"\n`, + ); + await write( + join(String(packageDir), "package.json"), + JSON.stringify({ + name: "ships-types-consumer", + dependencies: { + "top-types": "1.0.0", + "top-typings": "1.0.0", + "exports-types": "1.0.0", + "pure-js": "1.0.0", + }, + }), + ); + + await runBunInstall(bunEnv, String(packageDir)); + + const bunDir = join(String(packageDir), "node_modules", ".bun"); + + // A package without any types signal stays in the global store → + // symlink into `/links/-`. + const pureEntry = join(bunDir, "pure-js@1.0.0"); + expect(lstatSync(pureEntry).isSymbolicLink()).toBe(true); + expect(readlinkSync(pureEntry)).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); + + // Each type-shipping signal (top-level `types`, top-level `typings`, + // or a `"types"` condition under `exports`) keeps the package + // project-local as a real directory. + for (const name of ["top-types", "top-typings", "exports-types"] as const) { + const entry = join(bunDir, `${name}@1.0.0`); + expect(lstatSync(entry).isSymbolicLink()).toBe(false); + expect(lstatSync(entry).isDirectory()).toBe(true); + expect(existsSync(join(entry, "node_modules", name, "package.json"))).toBe(true); + } + }); }); From 85591cc8f7c2c48193df96982d054824f3e14e4e Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 00:54:00 +0000 Subject: [PATCH 02/12] install: detect "types" in exports even when it maps to a nested object `exports.types` has two shapes in the wild: "exports": { ".": { "types": "./index.d.ts", "default": "./index.js" } } "exports": { ".": { "types": { "import": "./a.d.mts", "require": "./a.d.cts" } } } The second (dual-package types) is what TypeScript recommends for packages shipping both .d.mts and .d.cts. The previous scanner required the `"types"` value to be an `e_string`, so the dual-package shape slipped through: recursion saw only the inner `import`/`require` keys and returned false, leaving the package global-store-eligible and reproducing the original #29727 TS2604/TS2786 failure for that subset of packages. Match any `"types"` key regardless of its value type (what the doc comment already promised), and add a fourth test fixture (`exports-types-dual`) to lock the case in. --- src/install/isolated_install.zig | 18 +++++++----- test/cli/install/isolated-install.test.ts | 34 ++++++++++++++++++++--- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index eb576c83e8b..df00875b90f 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -2059,9 +2059,16 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { /// "exports": { "import": { "types": "./a.d.ts", "default": "./a.js" } } /// "exports": { "types": "./index.d.ts", "default": "./index.js" } /// -/// We treat ANY `"types"` key inside the exports tree as a signal. The -/// recursion depth is bounded by how deeply a `package.json` nests -/// conditions (1-3 in practice), and we cap it defensively. +/// Dual-package types nest the other way — a `"types"` key whose *value* +/// is itself a conditional object: +/// +/// "exports": { ".": { "types": { "import": "./a.d.mts", "require": "./a.d.cts" } } } +/// +/// Either shape means the package ships declarations, so any `"types"` key +/// anywhere in the tree is a hit regardless of whether its value is a +/// string or a nested conditional object. Recursion depth is bounded by +/// how deeply a `package.json` nests conditions (1-3 in practice); the +/// cap is just a defensive ceiling for malformed input. fn exportsHasTypesCondition(expr: bun.js_parser.Expr) bool { return exportsHasTypesConditionInner(expr, 0); } @@ -2072,10 +2079,7 @@ fn exportsHasTypesConditionInner(expr: bun.js_parser.Expr, depth: u8) bool { for (expr.data.e_object.properties.slice()) |prop| { const key = prop.key orelse continue; const value = prop.value orelse continue; - if (key.data == .e_string and - key.data.e_string.eqlComptime("types") and - value.data == .e_string) - { + if (key.data == .e_string and key.data.e_string.eqlComptime("types")) { return true; } if (value.data == .e_object) { diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 9f5114fada7..6df9913a48b 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -1997,6 +1997,29 @@ describe("global virtual store", () => { "exports-types/index.js": "module.exports = {};\n", "exports-types/index.d.ts": "export {};\n", + // Dual-package types: a `"types"` key whose value is itself a + // conditional object — the format TypeScript recommends for + // packages shipping both `.d.mts` and `.d.cts`. Must still be + // detected. + "exports-types-dual/package.json": JSON.stringify({ + name: "exports-types-dual", + version: "1.0.0", + exports: { + ".": { + types: { + import: "./index.d.mts", + require: "./index.d.cts", + }, + import: "./index.mjs", + require: "./index.cjs", + }, + }, + }), + "exports-types-dual/index.mjs": "export default {};\n", + "exports-types-dual/index.cjs": "module.exports = {};\n", + "exports-types-dual/index.d.mts": "export {};\n", + "exports-types-dual/index.d.cts": "export {};\n", + "pure-js/package.json": JSON.stringify({ name: "pure-js", version: "1.0.0", @@ -2021,6 +2044,7 @@ describe("global virtual store", () => { "top-types": await pack("top-types"), "top-typings": await pack("top-typings"), "exports-types": await pack("exports-types"), + "exports-types-dual": await pack("exports-types-dual"), "pure-js": await pack("pure-js"), }; @@ -2070,6 +2094,7 @@ describe("global virtual store", () => { "top-types": "1.0.0", "top-typings": "1.0.0", "exports-types": "1.0.0", + "exports-types-dual": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2085,10 +2110,11 @@ describe("global virtual store", () => { expect(lstatSync(pureEntry).isSymbolicLink()).toBe(true); expect(readlinkSync(pureEntry)).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); - // Each type-shipping signal (top-level `types`, top-level `typings`, - // or a `"types"` condition under `exports`) keeps the package - // project-local as a real directory. - for (const name of ["top-types", "top-typings", "exports-types"] as const) { + // Each type-shipping signal — top-level `types`, top-level `typings`, + // a `"types"` condition with a string target under `exports`, or a + // `"types"` condition with a nested-object target (the dual ESM/CJS + // shape) — keeps the package project-local as a real directory. + for (const name of ["top-types", "top-typings", "exports-types", "exports-types-dual"] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); From b7bea36c01a7c42606f6984671de0781cd8097ab Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 01:14:36 +0000 Subject: [PATCH 03/12] install: handle "types" inside exports array fallbacks, use bun.handleOom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups on the exports-condition scanner: - Node's conditional exports allow any target to be an array of fallbacks: "exports": { ".": [{ "types": "./a.d.ts", "default": "./a.js" }, "./b.js" ] } The previous scanner bailed on `expr.data != .e_object` and only recursed into object values, so a `"types"` key nested inside an array element was never visited. Add an `.e_array` arm that recurses into each item, and always recurse on object-property values regardless of their type (the recursion itself short-circuits on non-object/non-array). - Use `bun.handleOom(cache.getOrPut(...))` instead of `cache.getOrPut(...) catch bun.outOfMemory()` to match the convention documented in `src/CLAUDE.md` and used by every other allocator-error site in `src/install/`. Functionally identical — `getOrPut`'s error set is `{OutOfMemory}` — but `bun.handleOom` comptime-asserts that, so a future stdlib change that widens the error set would fail to compile instead of silently swallowing the new error into an OOM panic. Adds a fifth fixture (`exports-types-array`) covering the array-of- fallbacks shape. --- src/install/isolated_install.zig | 40 +++++++++++++++-------- test/cli/install/isolated-install.test.ts | 29 ++++++++++++++-- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index df00875b90f..d035b98729f 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1972,7 +1972,7 @@ fn packageShipsTypeDeclarations( pkg_id: PackageID, cache: *std.AutoHashMapUnmanaged(PackageID, bool), ) bool { - const gop = cache.getOrPut(manager.allocator, pkg_id) catch bun.outOfMemory(); + const gop = bun.handleOom(cache.getOrPut(manager.allocator, pkg_id)); if (gop.found_existing) return gop.value_ptr.*; gop.value_ptr.* = false; @@ -2064,27 +2064,39 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { /// /// "exports": { ".": { "types": { "import": "./a.d.mts", "require": "./a.d.cts" } } } /// +/// Conditions can also be arrays of fallback targets: +/// +/// "exports": { ".": [ { "types": "./a.d.ts", "default": "./a.js" }, "./b.js" ] } +/// /// Either shape means the package ships declarations, so any `"types"` key /// anywhere in the tree is a hit regardless of whether its value is a -/// string or a nested conditional object. Recursion depth is bounded by -/// how deeply a `package.json` nests conditions (1-3 in practice); the -/// cap is just a defensive ceiling for malformed input. +/// string, a nested conditional object, or sits inside an array-of- +/// fallbacks. Recursion depth is bounded by how deeply a `package.json` +/// nests conditions (1-3 in practice); the cap is just a defensive +/// ceiling for malformed input. fn exportsHasTypesCondition(expr: bun.js_parser.Expr) bool { return exportsHasTypesConditionInner(expr, 0); } fn exportsHasTypesConditionInner(expr: bun.js_parser.Expr, depth: u8) bool { if (depth > 8) return false; - if (expr.data != .e_object) return false; - for (expr.data.e_object.properties.slice()) |prop| { - const key = prop.key orelse continue; - const value = prop.value orelse continue; - if (key.data == .e_string and key.data.e_string.eqlComptime("types")) { - return true; - } - if (value.data == .e_object) { - if (exportsHasTypesConditionInner(value, depth + 1)) return true; - } + switch (expr.data) { + .e_object => |obj| { + for (obj.properties.slice()) |prop| { + const key = prop.key orelse continue; + const value = prop.value orelse continue; + if (key.data == .e_string and key.data.e_string.eqlComptime("types")) { + return true; + } + if (exportsHasTypesConditionInner(value, depth + 1)) return true; + } + }, + .e_array => |array| { + for (array.items.slice()) |item| { + if (exportsHasTypesConditionInner(item, depth + 1)) return true; + } + }, + else => {}, } return false; } diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 6df9913a48b..b2448978f54 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2020,6 +2020,20 @@ describe("global virtual store", () => { "exports-types-dual/index.d.mts": "export {};\n", "exports-types-dual/index.d.cts": "export {};\n", + // Array-of-fallbacks: the Node resolution spec lets any target be a + // list of alternatives tried in order. A `"types"` key buried inside + // such an array still means the package ships types. + "exports-types-array/package.json": JSON.stringify({ + name: "exports-types-array", + version: "1.0.0", + exports: { + ".": [{ types: "./index.d.ts", default: "./index.js" }, "./fallback.js"], + }, + }), + "exports-types-array/index.js": "module.exports = {};\n", + "exports-types-array/index.d.ts": "export {};\n", + "exports-types-array/fallback.js": "module.exports = {};\n", + "pure-js/package.json": JSON.stringify({ name: "pure-js", version: "1.0.0", @@ -2045,6 +2059,7 @@ describe("global virtual store", () => { "top-typings": await pack("top-typings"), "exports-types": await pack("exports-types"), "exports-types-dual": await pack("exports-types-dual"), + "exports-types-array": await pack("exports-types-array"), "pure-js": await pack("pure-js"), }; @@ -2095,6 +2110,7 @@ describe("global virtual store", () => { "top-typings": "1.0.0", "exports-types": "1.0.0", "exports-types-dual": "1.0.0", + "exports-types-array": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2111,10 +2127,17 @@ describe("global virtual store", () => { expect(readlinkSync(pureEntry)).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); // Each type-shipping signal — top-level `types`, top-level `typings`, - // a `"types"` condition with a string target under `exports`, or a + // a `"types"` condition with a string target under `exports`, a // `"types"` condition with a nested-object target (the dual ESM/CJS - // shape) — keeps the package project-local as a real directory. - for (const name of ["top-types", "top-typings", "exports-types", "exports-types-dual"] as const) { + // shape), or a `"types"` key nested inside an array-of-fallbacks — + // keeps the package project-local as a real directory. + for (const name of [ + "top-types", + "top-typings", + "exports-types", + "exports-types-dual", + "exports-types-array", + ] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); From d138d60d3f0d1a739a74dbe1905830494a9fce09 Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 02:05:02 +0000 Subject: [PATCH 04/12] install: conservatively mark unreadable packages as ineligible for global store Canonical CI scenario (committed `bun.lock` + fresh runner or `bun pm cache rm` + `bun install --frozen-lockfile`) hit the original #29727 bug despite this PR's earlier commits. With a valid lockfile and matching `package.json`, resolution enqueues no download tasks, the pre-`installIsolatedPackages` wait block is skipped, and the eligibility DFS runs before the per-entry extraction loop has populated `/@@@@1/package.json`. The previous `err => return false` fallback then left type-shipping packages global-store-eligible, silently symlinking them into `/links/` where TypeScript's ancestor walk breaks again. Consulting the npm manifest instead is not an option: Bun fetches the v1 "install" manifest (`application/vnd.npm.install-v1+json`), which strips `"types"`/`"typings"`/`"exports"` from each version. Conservatively mark packages we can't read as ineligible. First install after a cache wipe materializes everything project-locally (slower one- time write, no global-store benefit for that install); subsequent installs on the same machine hit a warm cache and route eligible packages back through the shared store via a fresh DFS. The memoization per `pkg_id` is stable within one install, and the decision is re-evaluated from scratch on the next one. Adds a second pass to the existing test that wipes `node_modules` and the private cache, then runs `--frozen-lockfile`, to lock in this behavior. --- src/install/isolated_install.zig | 32 ++++++++++++++++++----- test/cli/install/isolated-install.test.ts | 30 ++++++++++++++++----- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index d035b98729f..e0471694384 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1962,11 +1962,23 @@ pub fn installIsolatedPackages( /// the global virtual store. /// /// Results are memoized in `cache` so each distinct package is scanned at -/// most once per install. If the extracted `package.json` is unreadable -/// (cache evicted, extraction deferred) we conservatively report `false` -/// and fall back to the pre-carve-out behavior — a package whose types we -/// couldn't inspect just stays in whichever bucket the rest of the -/// eligibility check places it. +/// most once per install. +/// +/// Cold-cache / frozen-lockfile caveat: when a valid lockfile matches the +/// project's `package.json` and the per-package cache is empty (canonical +/// CI scenario with a committed lockfile, fresh runner, or after +/// `bun pm cache rm`), the `installIsolatedPackages` DFS runs before the +/// per-entry download/extract loop has populated `//`. We +/// cannot consult the npm registry manifest here either — the v1 +/// "install" manifest Bun fetches strips `"types"`/`"typings"`/`"exports"` +/// from each version. So we conservatively mark packages we can't read +/// as ineligible for the global store: the first install after a cache +/// wipe materializes packages project-locally (slower to write, but +/// TypeScript resolution is correct), and subsequent installs on the +/// same machine hit the warm cache and route eligible packages back +/// through the shared store. The alternative — leaving packages +/// global-store-eligible when we can't verify — silently resurrects +/// #29727 for the whole CI fleet. fn packageShipsTypeDeclarations( manager: *PackageManager, pkg_id: PackageID, @@ -2006,7 +2018,15 @@ fn packageShipsTypeDeclarations( const source_bytes = switch (bun.sys.File.readFrom(cache_dir, rel.sliceZ(), manager.allocator)) { .result => |b| b, - .err => return false, + .err => { + // Package is not yet extracted (see the cold-cache caveat + // above). Conservatively memoize as ineligible — the package + // gets materialized project-locally on this install, and the + // next install (on a warm cache) recomputes eligibility from + // scratch in a fresh DFS. + gop.value_ptr.* = true; + return true; + }, }; defer manager.allocator.free(source_bytes); diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index b2448978f54..4c91eaf0e64 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2131,13 +2131,29 @@ describe("global virtual store", () => { // `"types"` condition with a nested-object target (the dual ESM/CJS // shape), or a `"types"` key nested inside an array-of-fallbacks — // keeps the package project-local as a real directory. - for (const name of [ - "top-types", - "top-typings", - "exports-types", - "exports-types-dual", - "exports-types-array", - ] as const) { + const typeShippingNames = ["top-types", "top-typings", "exports-types", "exports-types-dual", "exports-types-array"] as const; + for (const name of typeShippingNames) { + const entry = join(bunDir, `${name}@1.0.0`); + expect(lstatSync(entry).isSymbolicLink()).toBe(false); + expect(lstatSync(entry).isDirectory()).toBe(true); + expect(existsSync(join(entry, "node_modules", name, "package.json"))).toBe(true); + } + + // CI scenario: fresh runner with a committed lockfile but no + // pre-warmed package cache. `--frozen-lockfile` skips the diff-time + // resolution phase, so the eligibility DFS inside + // `installIsolatedPackages` runs before extraction has populated + // `/@@@@1/package.json`. A naive scan of the + // extracted package.json sees ENOENT and would fall through to + // `eligible` — resurrecting #29727 for every type-shipping package + // whose tarball wasn't already in this project's cache. The + // conservative fallback treats unreadable packages as ineligible, + // so types-shipping packages stay project-local even on a cold + // cache. + await rm(join(String(packageDir), "node_modules"), { recursive: true, force: true }); + await rm(cacheDir, { recursive: true, force: true }); + await runBunInstall(bunEnv, String(packageDir), { savesLockfile: false, frozenLockfile: true }); + for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); From bd34abf850152cfd7ecd7d1480ac15f90f6ae78a Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 26 Apr 2026 02:07:02 +0000 Subject: [PATCH 05/12] [autofix.ci] apply automated fixes --- test/cli/install/isolated-install.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 4c91eaf0e64..ea6a7e77f79 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2131,7 +2131,13 @@ describe("global virtual store", () => { // `"types"` condition with a nested-object target (the dual ESM/CJS // shape), or a `"types"` key nested inside an array-of-fallbacks — // keeps the package project-local as a real directory. - const typeShippingNames = ["top-types", "top-typings", "exports-types", "exports-types-dual", "exports-types-array"] as const; + const typeShippingNames = [ + "top-types", + "top-typings", + "exports-types", + "exports-types-dual", + "exports-types-array", + ] as const; for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(false); From 51cf3930380e91cbce61a60a71a5b8b7faf61b7b Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 02:10:20 +0000 Subject: [PATCH 06/12] install: detect typesVersions; raise OOM in type-scan JSON parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups on the exports-condition scanner: - `scanForTypeDeclarationSignals` missed `typesVersions`. Packages that ship version-gated declarations via that field without a top-level `"types"`/`"typings"` (TypeScript's original subpath-mapping mechanism, still in use by older packages) were leaving types-shipping packages global-store-eligible. Check for a non-empty `"typesVersions"` map alongside the existing signals, add a `types-versions` test fixture, and mention the field in both the function doc comment and `docs/pm/global-store.mdx`. - The JSON-parse `catch return false` swallowed `error.OutOfMemory` into "this package has no types" — a correctness bug: an OOM during the parse of a type-shipping `package.json` would silently leave it global-store-eligible. Route OOM through `bun.outOfMemory()` (abort), keep the `false` fallback only for malformed / corrupt JSON. --- docs/pm/global-store.mdx | 2 +- src/install/isolated_install.zig | 33 ++++++++++++++++------- test/cli/install/isolated-install.test.ts | 21 +++++++++++++-- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/docs/pm/global-store.mdx b/docs/pm/global-store.mdx index 6fa8711ddb6..e8d7b2ffcbe 100644 --- a/docs/pm/global-store.mdx +++ b/docs/pm/global-store.mdx @@ -119,7 +119,7 @@ An entry only lives in the global store when it can be safely shared. Entries fa - the package has a **patch** applied via `bun patch` — the patched contents are project-specific; - the package is listed in **`trustedDependencies`** (or trusted via `bun add --trust`) — its lifecycle script may mutate the install directory, and a script running through the project symlink would mutate the shared copy; -- the package **ships TypeScript declarations** — a top-level `"types"` or `"typings"` field in `package.json`, or a `"types"` condition under `"exports"`. TypeScript resolves peer types (e.g. `React.FunctionComponent` in a package that never declared `@types/react` as a peer dependency) by walking `node_modules` ancestors from the declaration file's realpath; if that realpath is in the shared store those walks never reach the project's `@types/*`. Keeping type-shipping packages project-local puts their realpath back under `node_modules/.bun//`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/`. +- the package **ships TypeScript declarations** — a top-level `"types"` or `"typings"` field, a non-empty `"typesVersions"` map, or a `"types"` condition anywhere under `"exports"`. TypeScript resolves peer types (e.g. `React.FunctionComponent` in a package that never declared `@types/react` as a peer dependency) by walking `node_modules` ancestors from the declaration file's realpath; if that realpath is in the shared store those walks never reach the project's `@types/*`. Keeping type-shipping packages project-local puts their realpath back under `node_modules/.bun//`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/`. - the package, or **any** dependency it links to, is a `workspace:`, `file:`, or `link:` dependency — those resolve to project-local paths that other projects can't see. Ineligibility propagates: if `your-app` depends on `internal-utils` which is a workspace package, `internal-utils` is project-local, and so is every entry that links to it. An entry that loses eligibility between installs (newly patched, newly trusted) is detached from the global store and rebuilt project-locally on the next install; the shared entry is left untouched. diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index e0471694384..ef279171758 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1955,11 +1955,11 @@ pub fn installIsolatedPackages( /// /// Returns true if its extracted `package.json` at /// `//package.json` contains a top-level `"types"` or -/// `"typings"` string field, or a `"types"` key inside a conditional -/// `"exports"` entry. Either signal means the package intentionally ships -/// `.d.ts` files that TypeScript will walk ancestor directories from — -/// see the eligibility-DFS comment for why that rules the package out of -/// the global virtual store. +/// `"typings"` string field, a non-empty `"typesVersions"` map, or a +/// `"types"` key anywhere inside `"exports"`. Any of these signals means +/// the package intentionally ships `.d.ts` files that TypeScript will +/// walk ancestor directories from — see the eligibility-DFS comment for +/// why that rules the package out of the global virtual store. /// /// Results are memoized in `cache` so each distinct package is scanned at /// most once per install. @@ -2034,9 +2034,10 @@ fn packageShipsTypeDeclarations( return gop.value_ptr.*; } -/// Scans `package.json` bytes for a top-level `"types"` / `"typings"` -/// string field, or a nested `"types"` key inside a conditional `"exports"` -/// entry. Uses the shared JSON parser (cheaper to maintain than a +/// Scans `package.json` bytes for any signal that the package ships +/// TypeScript declarations: a top-level `"types"` / `"typings"` string, +/// a non-empty `"typesVersions"` map, or a `"types"` key anywhere inside +/// `"exports"`. Uses the shared JSON parser (cheaper to maintain than a /// hand-rolled scanner; a typical `package.json` is a few KB, well inside /// what the registry manifest cache already parses routinely during /// install). @@ -2055,7 +2056,14 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { // that uses `JSON.parsePackageJSONUTF8` is expected to call it; we // mirror that here. install.initializeStore(); - const json = JSON.parsePackageJSONUTF8(&source, &log_sink, arena_allocator) catch return false; + const json = JSON.parsePackageJSONUTF8(&source, &log_sink, arena_allocator) catch |err| switch (err) { + // Route OOM through the crash handler rather than silently + // misclassifying a type-shipping package as eligible for the + // global store. Any other parse error (malformed / corrupt + // package.json) gets the conservative `false` fallback. + error.OutOfMemory => bun.outOfMemory(), + else => return false, + }; if (json.asProperty("types")) |prop| { if (prop.expr.asString(arena_allocator)) |s| { @@ -2067,6 +2075,13 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { if (s.len > 0) return true; } } + // `typesVersions` is a map keyed by TypeScript version ranges; its + // mere presence means the package ships version-gated declarations + // (TypeScript's own module resolution honors it). Any non-empty + // object value is a positive signal. + if (json.asProperty("typesVersions")) |prop| { + if (prop.expr.data == .e_object and prop.expr.data.e_object.properties.len > 0) return true; + } if (json.asProperty("exports")) |prop| { if (exportsHasTypesCondition(prop.expr)) return true; } diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index ea6a7e77f79..c55e62b82a7 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2034,6 +2034,19 @@ describe("global virtual store", () => { "exports-types-array/index.d.ts": "export {};\n", "exports-types-array/fallback.js": "module.exports = {};\n", + // `typesVersions` ships version-gated declarations; some packages + // use it as their sole subpath-mapping mechanism and omit the + // top-level `"types"` field entirely. + "types-versions/package.json": JSON.stringify({ + name: "types-versions", + version: "1.0.0", + typesVersions: { + ">=4.0": { "*": ["ts4/*"] }, + }, + }), + "types-versions/index.js": "module.exports = {};\n", + "types-versions/ts4/index.d.ts": "export {};\n", + "pure-js/package.json": JSON.stringify({ name: "pure-js", version: "1.0.0", @@ -2060,6 +2073,7 @@ describe("global virtual store", () => { "exports-types": await pack("exports-types"), "exports-types-dual": await pack("exports-types-dual"), "exports-types-array": await pack("exports-types-array"), + "types-versions": await pack("types-versions"), "pure-js": await pack("pure-js"), }; @@ -2111,6 +2125,7 @@ describe("global virtual store", () => { "exports-types": "1.0.0", "exports-types-dual": "1.0.0", "exports-types-array": "1.0.0", + "types-versions": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2129,14 +2144,16 @@ describe("global virtual store", () => { // Each type-shipping signal — top-level `types`, top-level `typings`, // a `"types"` condition with a string target under `exports`, a // `"types"` condition with a nested-object target (the dual ESM/CJS - // shape), or a `"types"` key nested inside an array-of-fallbacks — - // keeps the package project-local as a real directory. + // shape), a `"types"` key nested inside an array-of-fallbacks, or a + // non-empty `"typesVersions"` map — keeps the package project-local + // as a real directory. const typeShippingNames = [ "top-types", "top-typings", "exports-types", "exports-types-dual", "exports-types-array", + "types-versions", ] as const; for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); From e45ad329ae51ef3a175a71b3b47bb27d68749c82 Mon Sep 17 00:00:00 2001 From: robobun Date: Sun, 26 Apr 2026 02:17:44 +0000 Subject: [PATCH 07/12] install: treat malformed package.json as ineligible; add warm-cache test pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Parse failures in the types scanner now fall through to the same conservative `ineligible` path as `readFrom` ENOENT. Returning `false` on a corrupted / truncated `package.json` would silently route a type-shipping package into the global store when its cache entry is damaged — same failure mode as the cold-cache case. OOM stays on `bun.outOfMemory()` (abort); every other parse error now returns `true` (ineligible). - Extend the test with a third pass (warm cache + frozen lockfile) that verifies `pure-js` flips back to a symlink into `/links/` once extraction populates its cache entry. This guards against the conservative fallback being sticky — if a future change left packages permanently ineligible once the ENOENT path fired, the assertion on `pure-js` would fail. - Tighten the second-pass comment on the frozen-lockfile rationale: spell out that `installWithManager` never enqueues any download / extract tasks on the no-diff path, so the eligibility DFS genuinely runs against an empty cache there. --- src/install/isolated_install.zig | 13 +++++--- test/cli/install/isolated-install.test.ts | 38 +++++++++++++++++------ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index ef279171758..7ea8973cc7c 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -2057,12 +2057,15 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { // mirror that here. install.initializeStore(); const json = JSON.parsePackageJSONUTF8(&source, &log_sink, arena_allocator) catch |err| switch (err) { - // Route OOM through the crash handler rather than silently - // misclassifying a type-shipping package as eligible for the - // global store. Any other parse error (malformed / corrupt - // package.json) gets the conservative `false` fallback. + // OOM aborts the process rather than being swallowed into a + // mis-classification. Any other parse error (truncated, garbled, + // or just not JSON) is treated conservatively as "unknown" and + // the package is kept project-local — same fallback as when the + // file itself isn't readable. `return false` here would quietly + // route a type-shipping package with a corrupted cache into the + // global store, resurrecting #29727 for that subset. error.OutOfMemory => bun.outOfMemory(), - else => return false, + else => return true, }; if (json.asProperty("types")) |prop| { diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index c55e62b82a7..52c1c78b1b5 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2163,16 +2163,17 @@ describe("global virtual store", () => { } // CI scenario: fresh runner with a committed lockfile but no - // pre-warmed package cache. `--frozen-lockfile` skips the diff-time - // resolution phase, so the eligibility DFS inside - // `installIsolatedPackages` runs before extraction has populated - // `/@@@@1/package.json`. A naive scan of the - // extracted package.json sees ENOENT and would fall through to - // `eligible` — resurrecting #29727 for every type-shipping package - // whose tarball wasn't already in this project's cache. The - // conservative fallback treats unreadable packages as ineligible, - // so types-shipping packages stay project-local even on a cold - // cache. + // pre-warmed package cache. On the no-diff / frozen-lockfile path, + // `installWithManager` never enqueues any download or extract + // tasks before handing off to `installIsolatedPackages` + // (`pendingTaskCount() == 0` → the wait block is skipped), so the + // eligibility DFS runs against an empty cache. A naive + // `readFrom(//package.json)` hits ENOENT and, without + // the conservative fallback, would return "no types" → eligible → + // the type-shipping package lands in `/links/` anyway, + // resurrecting #29727. The fallback treats unreadable packages + // as ineligible, keeping type-shipping packages project-local + // even on the cold-cache CI path. await rm(join(String(packageDir), "node_modules"), { recursive: true, force: true }); await rm(cacheDir, { recursive: true, force: true }); await runBunInstall(bunEnv, String(packageDir), { savesLockfile: false, frozenLockfile: true }); @@ -2182,5 +2183,22 @@ describe("global virtual store", () => { expect(lstatSync(entry).isDirectory()).toBe(true); expect(existsSync(join(entry, "node_modules", name, "package.json"))).toBe(true); } + + // Warm-cache re-install (lockfile unchanged, per-package cache + // populated by the previous install). The DFS can now read every + // `package.json` and reaches the real decision: type-shippers stay + // project-local, and `pure-js` — which has no types signal of any + // kind — flips back to a symlink into `/links/`. This + // guards against a regression where the conservative fallback ends + // up sticky and forces every package project-local forever. + await rm(join(String(packageDir), "node_modules"), { recursive: true, force: true }); + await runBunInstall(bunEnv, String(packageDir), { savesLockfile: false, frozenLockfile: true }); + for (const name of typeShippingNames) { + const entry = join(bunDir, `${name}@1.0.0`); + expect(lstatSync(entry).isSymbolicLink()).toBe(false); + expect(lstatSync(entry).isDirectory()).toBe(true); + } + expect(lstatSync(join(bunDir, "pure-js@1.0.0")).isSymbolicLink()).toBe(true); + expect(readlinkSync(join(bunDir, "pure-js@1.0.0"))).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); }); }); From b95ff7ec34cbbd69f65d4eaa19e8230e827a8611 Mon Sep 17 00:00:00 2001 From: robobun Date: Wed, 29 Apr 2026 02:26:06 +0000 Subject: [PATCH 08/12] install: replace types heuristic with ATW-style resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the global-store eligibility check scanned package.json for any `"types"`/`"typings"`/`"typesVersions"` field or a `"types"` key anywhere under `"exports"`. That was a conservative heuristic: false-positive on any package that names a `"types"` field, false- negative on implicit `index.d.ts`, and silent about whether the declared target actually exists on disk. Port the resolution algorithm `arethetypeswrong.github.io` uses so the answer reflects what TypeScript itself would see: 1. If `"exports"` is set, walk the conditional tree for `.` with condition priority `[types, import, require, default]` (the order `tsc --moduleResolution node16/bundler` uses when `types` is in its condition set). The resolved string target either IS a declaration file (`.d.ts` / `.d.mts` / `.d.cts` / `.ts`-family source) or has a sibling declaration on disk. 2. Otherwise check top-level `"types"` / `"typings"` and apply the same is-or-has-sibling rule to the declared path. 3. Otherwise, if `"typesVersions"` is non-empty, TS resolves through its version-range mappings — treat that as type-shipping. 4. Otherwise, fall back to an implicit `index.d.ts` at the package root. Any positive hit forces the package project-local; per the Node `"exports"` spec, once `"exports"` is present the top-level `"types"` / `"typings"` / `"typesVersions"` / implicit `index.d.ts` fallbacks are NOT consulted, so the resolver returns early in that branch. Test extended with three new fixtures that exercise resolver behaviour the heuristic couldn't model: * `implicit-index` — no `"exports"`/`"types"`, only a root `index.d.ts`. ATW fallback detects it; the heuristic missed it. * `sibling-dts` — `"exports"` with no `"types"` condition but the JS target has a sibling `.d.mts`/`.d.cts`. ATW pairs JS + dts extensions and finds the sibling; the heuristic missed it. * `exports-no-types` — `"exports"` map whose resolved JS files have no sibling declarations. ATW correctly reports no types exposed; the earlier check would have false-positive-d on a future `"exports"`-presence heuristic. Doubles as the non-type-shipping control alongside `pure-js`. Pack fixtures in parallel to keep the test under the default timeout (`bun pm pack` against a debug build is ~2-3s cold; serialising ten was pushing 30s just on packing). --- docs/pm/global-store.mdx | 2 +- src/install/isolated_install.zig | 332 +++++++++++++++------- test/cli/install/isolated-install.test.ts | 124 ++++++-- 3 files changed, 339 insertions(+), 119 deletions(-) diff --git a/docs/pm/global-store.mdx b/docs/pm/global-store.mdx index e8d7b2ffcbe..8b07f23ba70 100644 --- a/docs/pm/global-store.mdx +++ b/docs/pm/global-store.mdx @@ -119,7 +119,7 @@ An entry only lives in the global store when it can be safely shared. Entries fa - the package has a **patch** applied via `bun patch` — the patched contents are project-specific; - the package is listed in **`trustedDependencies`** (or trusted via `bun add --trust`) — its lifecycle script may mutate the install directory, and a script running through the project symlink would mutate the shared copy; -- the package **ships TypeScript declarations** — a top-level `"types"` or `"typings"` field, a non-empty `"typesVersions"` map, or a `"types"` condition anywhere under `"exports"`. TypeScript resolves peer types (e.g. `React.FunctionComponent` in a package that never declared `@types/react` as a peer dependency) by walking `node_modules` ancestors from the declaration file's realpath; if that realpath is in the shared store those walks never reach the project's `@types/*`. Keeping type-shipping packages project-local puts their realpath back under `node_modules/.bun//`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/`. +- the package **exposes TypeScript declarations** — Bun runs the same entry-point resolution TypeScript uses (modelled on [arethetypeswrong.github.io](https://arethetypeswrong.github.io/)) against each package's `package.json`. `"exports"` with a `"types"` condition (including dual `.d.mts`/`.d.cts` objects and array-of-fallbacks), `"exports"` whose resolved JS target has a sibling declaration, top-level `"types"` / `"typings"`, non-empty `"typesVersions"`, and the implicit `index.d.ts` fallback all count. TypeScript resolves peer types (e.g. `React.FunctionComponent` in a package that never declared `@types/react` as a peer dependency) by walking `node_modules` ancestors from the declaration file's realpath; if that realpath is in the shared store those walks never reach the project's `@types/*`. Keeping type-shipping packages project-local puts their realpath back under `node_modules/.bun//`, whose ancestor walk reaches the hidden hoisted layer at `node_modules/.bun/node_modules/`. - the package, or **any** dependency it links to, is a `workspace:`, `file:`, or `link:` dependency — those resolve to project-local paths that other projects can't see. Ineligibility propagates: if `your-app` depends on `internal-utils` which is a workspace package, `internal-utils` is project-local, and so is every entry that links to it. An entry that loses eligibility between installs (newly patched, newly trusted) is detached from the global store and rebuilt project-locally on the next install; the shared entry is left untouched. diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index 7ea8973cc7c..ed8614ae061 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1951,34 +1951,49 @@ pub fn installIsolatedPackages( } } -/// Does the package at `pkg_id` declare TypeScript type declarations? +/// Does the package at `pkg_id` expose TypeScript declarations to +/// consumers? /// -/// Returns true if its extracted `package.json` at -/// `//package.json` contains a top-level `"types"` or -/// `"typings"` string field, a non-empty `"typesVersions"` map, or a -/// `"types"` key anywhere inside `"exports"`. Any of these signals means -/// the package intentionally ships `.d.ts` files that TypeScript will -/// walk ancestor directories from — see the eligibility-DFS comment for -/// why that rules the package out of the global virtual store. +/// Runs the same resolution algorithm TypeScript uses to discover a +/// package's declarations (modelled on arethetypeswrong.github.io), on +/// the extracted `//package.json`: /// -/// Results are memoized in `cache` so each distinct package is scanned at +/// 1. If `"exports"` is set, walk the conditional-exports tree for the +/// `"."` subpath with condition priority `[types, import, require, +/// default]` — the same order `tsc --moduleResolution node16/bundler` +/// uses when `"types"` is in its condition set. If the first string +/// target reached is a declaration file, or has a sibling declaration +/// file, the package exposes types. +/// 2. Otherwise, check top-level `"types"` / `"typings"` — resolve the +/// path relative to the package root and apply the same is-or-has- +/// sibling-declaration rule. +/// 3. Otherwise, if `"typesVersions"` is present and non-empty, TS +/// treats the package as type-shipping regardless of subpath content +/// (it'll pick the matching range and look up subpaths there). +/// 4. Otherwise, fall back to implicit `index.d.ts` in the package +/// root (the conventional no-`exports` / no-`types` path). +/// +/// Any positive hit means the package's declaration file(s) are what a +/// consuming TypeScript project would try to resolve. If the package's +/// realpath sits in `/links/@-/`, that declaration +/// file's ancestor walk for peer types (`React`, `csstype`, etc.) never +/// reaches the project's `@types/*` — which is the #29727 regression. So +/// any type-exposing package is forced project-local. +/// +/// Results are memoized in `cache`; each distinct package is resolved at /// most once per install. /// -/// Cold-cache / frozen-lockfile caveat: when a valid lockfile matches the -/// project's `package.json` and the per-package cache is empty (canonical -/// CI scenario with a committed lockfile, fresh runner, or after -/// `bun pm cache rm`), the `installIsolatedPackages` DFS runs before the -/// per-entry download/extract loop has populated `//`. We -/// cannot consult the npm registry manifest here either — the v1 -/// "install" manifest Bun fetches strips `"types"`/`"typings"`/`"exports"` -/// from each version. So we conservatively mark packages we can't read -/// as ineligible for the global store: the first install after a cache -/// wipe materializes packages project-locally (slower to write, but -/// TypeScript resolution is correct), and subsequent installs on the -/// same machine hit the warm cache and route eligible packages back -/// through the shared store. The alternative — leaving packages -/// global-store-eligible when we can't verify — silently resurrects -/// #29727 for the whole CI fleet. +/// Cold-cache / frozen-lockfile caveat: on a `--frozen-lockfile` install +/// with an empty `//`, the eligibility DFS runs before +/// extraction has populated the per-package cache. We can't consult the +/// npm registry manifest either — Bun fetches the `install-v1+json` +/// manifest, which strips `"types"`/`"typings"`/`"exports"` from each +/// version. When the package.json can't be read, we conservatively +/// report "ineligible" (the package materializes project-locally on this +/// install; a subsequent warm-cache install recomputes the real answer). +/// The alternative — defaulting to eligible — silently routes type- +/// shipping packages into the global store and resurrects #29727 for +/// every CI runner that hits the no-diff path. fn packageShipsTypeDeclarations( manager: *PackageManager, pkg_id: PackageID, @@ -2019,29 +2034,37 @@ fn packageShipsTypeDeclarations( const source_bytes = switch (bun.sys.File.readFrom(cache_dir, rel.sliceZ(), manager.allocator)) { .result => |b| b, .err => { - // Package is not yet extracted (see the cold-cache caveat - // above). Conservatively memoize as ineligible — the package - // gets materialized project-locally on this install, and the - // next install (on a warm cache) recomputes eligibility from - // scratch in a fresh DFS. + // See the cold-cache caveat above. Conservative memoize — + // the package goes project-local on this install and the + // next install re-runs the full resolver against a warm + // cache. gop.value_ptr.* = true; return true; }, }; defer manager.allocator.free(source_bytes); - gop.value_ptr.* = scanForTypeDeclarationSignals(source_bytes); + gop.value_ptr.* = resolvePackageExposesTypes(source_bytes, cache_dir, folder); return gop.value_ptr.*; } -/// Scans `package.json` bytes for any signal that the package ships -/// TypeScript declarations: a top-level `"types"` / `"typings"` string, -/// a non-empty `"typesVersions"` map, or a `"types"` key anywhere inside -/// `"exports"`. Uses the shared JSON parser (cheaper to maintain than a -/// hand-rolled scanner; a typical `package.json` is a few KB, well inside -/// what the registry manifest cache already parses routinely during -/// install). -fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { +const type_decl_extensions = [_][]const u8{ ".d.ts", ".d.mts", ".d.cts" }; +/// TS pairs JS extensions with declaration file extensions when falling +/// back to sibling lookup: `foo.js` ↔ `foo.d.ts`, `foo.mjs` ↔ `foo.d.mts`, +/// `foo.cjs` ↔ `foo.d.cts`. Source-file extensions (`.ts`, `.mts`, `.cts`, +/// `.tsx`) are themselves type-carrying, handled in `pathExposesTypes`. +const js_to_dts_pairs = [_]struct { js: []const u8, dts: []const u8 }{ + .{ .js = ".mjs", .dts = ".d.mts" }, + .{ .js = ".cjs", .dts = ".d.cts" }, + .{ .js = ".js", .dts = ".d.ts" }, + .{ .js = ".jsx", .dts = ".d.ts" }, +}; + +/// The ATW-style resolver. Parses `package.json` and applies the same +/// entry-point resolution rules TypeScript uses, returning true if any +/// resolved path produces (or has a sibling) declaration file. See +/// `packageShipsTypeDeclarations` for the scenarios each branch covers. +fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: []const u8) bool { var arena = std.heap.ArenaAllocator.init(bun.default_allocator); defer arena.deinit(); const arena_allocator = arena.allocator(); @@ -2051,94 +2074,213 @@ fn scanForTypeDeclarationSignals(source_bytes: []const u8) bool { const source = logger.Source.initPathString("package.json", source_bytes); - // `initializeStore()` is idempotent but cheap — it sets up the thread- - // local expression arena the JSON parser writes into. Every call site - // that uses `JSON.parsePackageJSONUTF8` is expected to call it; we - // mirror that here. install.initializeStore(); const json = JSON.parsePackageJSONUTF8(&source, &log_sink, arena_allocator) catch |err| switch (err) { // OOM aborts the process rather than being swallowed into a // mis-classification. Any other parse error (truncated, garbled, - // or just not JSON) is treated conservatively as "unknown" and - // the package is kept project-local — same fallback as when the - // file itself isn't readable. `return false` here would quietly - // route a type-shipping package with a corrupted cache into the - // global store, resurrecting #29727 for that subset. + // or just not JSON) is treated conservatively — the package is + // kept project-local, same as when the file itself can't be + // read. Returning `false` here would quietly route a type- + // shipping package with a corrupted cache into the global + // store, resurrecting #29727 for that subset. error.OutOfMemory => bun.outOfMemory(), else => return true, }; - if (json.asProperty("types")) |prop| { - if (prop.expr.asString(arena_allocator)) |s| { - if (s.len > 0) return true; - } + // 1. `"exports"` — walk the conditional tree for the `.` subpath + // with types-priority conditions. + if (json.asProperty("exports")) |prop| { + if (exportsExposesTypes(prop.expr, cache_dir, folder, arena_allocator)) return true; + // When `exports` is set, per the Node resolution spec only + // subpaths listed there are visible to consumers — `types` / + // `typings` / `typesVersions` / implicit `index.d.ts` are NOT + // consulted. Fall through to return false. + return false; } - if (json.asProperty("typings")) |prop| { - if (prop.expr.asString(arena_allocator)) |s| { - if (s.len > 0) return true; + + // 2. Top-level `"types"` / `"typings"` — TS honors `types` first, + // then `typings` (the legacy name) when no `exports`. + inline for (.{ "types", "typings" }) |field| { + if (json.asProperty(field)) |prop| { + if (prop.expr.asString(arena_allocator)) |s| { + if (pathExposesTypes(s, cache_dir, folder)) return true; + } } } - // `typesVersions` is a map keyed by TypeScript version ranges; its - // mere presence means the package ships version-gated declarations - // (TypeScript's own module resolution honors it). Any non-empty - // object value is a positive signal. + + // 3. `"typesVersions"` — presence-only. TS resolves subpaths through + // whichever version range matches the consumer's TS version; we + // have no TS version to match here, so treat any non-empty map as + // a type-shipping signal. if (json.asProperty("typesVersions")) |prop| { if (prop.expr.data == .e_object and prop.expr.data.e_object.properties.len > 0) return true; } - if (json.asProperty("exports")) |prop| { - if (exportsHasTypesCondition(prop.expr)) return true; - } + + // 4. Implicit `index.d.ts` — the no-`exports`/no-`types` fallback. + if (cacheFileExists(cache_dir, folder, "index.d.ts")) return true; + return false; } -/// Conditional-exports entries can be: -/// -/// "exports": { ".": { "types": "./index.d.ts", "default": "./index.js" } } -/// "exports": { "import": { "types": "./a.d.ts", "default": "./a.js" } } -/// "exports": { "types": "./index.d.ts", "default": "./index.js" } -/// -/// Dual-package types nest the other way — a `"types"` key whose *value* -/// is itself a conditional object: -/// -/// "exports": { ".": { "types": { "import": "./a.d.mts", "require": "./a.d.cts" } } } -/// -/// Conditions can also be arrays of fallback targets: -/// -/// "exports": { ".": [ { "types": "./a.d.ts", "default": "./a.js" }, "./b.js" ] } -/// -/// Either shape means the package ships declarations, so any `"types"` key -/// anywhere in the tree is a hit regardless of whether its value is a -/// string, a nested conditional object, or sits inside an array-of- -/// fallbacks. Recursion depth is bounded by how deeply a `package.json` -/// nests conditions (1-3 in practice); the cap is just a defensive -/// ceiling for malformed input. -fn exportsHasTypesCondition(expr: bun.js_parser.Expr) bool { - return exportsHasTypesConditionInner(expr, 0); +/// Resolve the `.` subpath of an `"exports"` tree with condition priority +/// `[types, import, require, default]` and check the resolved target. +/// Returns true iff the target path (or one of its declaration siblings) +/// exists in the extracted cache. +fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const u8, allocator: std.mem.Allocator) bool { + // `exports` can be: + // "exports": "./index.js" → string (shorthand for `.`) + // "exports": [ ... ] → array of fallbacks (shorthand for `.`) + // "exports": { ".": ..., "./sub": ... } → subpath map + // "exports": { "import": ..., "require": ... } → condition map (shorthand for `.`) + const root_target: bun.js_parser.Expr = switch (expr.data) { + .e_string, .e_array => expr, + .e_object => |obj| blk: { + var has_dot_keys = false; + var dot_value: ?bun.js_parser.Expr = null; + for (obj.properties.slice()) |prop| { + const key = prop.key orelse continue; + if (key.data != .e_string) continue; + const s = key.data.e_string.data; + if (s.len > 0 and s[0] == '.') { + has_dot_keys = true; + if (s.len == 1) dot_value = prop.value; + } + } + // If every key is a subpath (`./x`), the map is purely a + // subpath map and only the `.` entry resolves to the root. + // Otherwise the whole object is the condition tree for `.`. + break :blk if (has_dot_keys) (dot_value orelse return false) else expr; + }, + else => return false, + }; + + return resolveTargetExposesTypes(root_target, cache_dir, folder, allocator, 0); } -fn exportsHasTypesConditionInner(expr: bun.js_parser.Expr, depth: u8) bool { - if (depth > 8) return false; - switch (expr.data) { +/// Walk a conditional-exports target (string, array, or condition object) +/// looking for a resolvable declaration file. Mirrors the Node.js +/// `PACKAGE_EXPORTS_RESOLVE` + `PACKAGE_TARGET_RESOLVE` algorithms, with +/// condition priority set to prefer `"types"` the way `tsc` does when +/// `types` is in the condition set. +fn resolveTargetExposesTypes( + target: bun.js_parser.Expr, + cache_dir: FD, + folder: []const u8, + allocator: std.mem.Allocator, + depth: u8, +) bool { + if (depth > 16) return false; + switch (target.data) { + .e_string => |str| { + if (str.data.len == 0) return false; + return pathExposesTypes(str.data, cache_dir, folder); + }, + .e_array => |array| { + // Fallback array: try each alternative in order; the first + // resolvable target wins. For our yes/no question the order + // doesn't matter — any alternative exposing types is enough. + for (array.items.slice()) |item| { + if (resolveTargetExposesTypes(item, cache_dir, folder, allocator, depth + 1)) return true; + } + return false; + }, .e_object => |obj| { + // Condition object: the Node spec says walk keys in + // declaration order, descending into the first key whose + // condition matches. TS treats `"types"` as its highest + // priority condition, then falls back to the module-system + // conditions. Our "does it expose types at all" question + // isn't order-sensitive in the same way the runtime + // resolver is: we just need to know whether ANY condition + // branch leads to a declaration file. So try `"types"` + // first (the fast path for dual-package type entry points) + // and fall back to all other conditions. for (obj.properties.slice()) |prop| { const key = prop.key orelse continue; const value = prop.value orelse continue; - if (key.data == .e_string and key.data.e_string.eqlComptime("types")) { - return true; + if (key.data != .e_string) continue; + if (key.data.e_string.eqlComptime("types")) { + if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, depth + 1)) return true; } - if (exportsHasTypesConditionInner(value, depth + 1)) return true; } - }, - .e_array => |array| { - for (array.items.slice()) |item| { - if (exportsHasTypesConditionInner(item, depth + 1)) return true; + for (obj.properties.slice()) |prop| { + const key = prop.key orelse continue; + const value = prop.value orelse continue; + if (key.data != .e_string) continue; + if (key.data.e_string.eqlComptime("types")) continue; + if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, depth + 1)) return true; } + return false; }, - else => {}, + else => return false, + } +} + +/// A path exposes TS types if it IS a declaration file (`.d.ts` / +/// `.d.mts` / `.d.cts`) or a TS source file (`.ts` / `.tsx` / `.mts` / +/// `.cts`), OR its JS sibling has a matching declaration file on disk. +fn pathExposesTypes(path: []const u8, cache_dir: FD, folder: []const u8) bool { + if (path.len == 0) return false; + + // Normalize leading "./" + const rel = if (bun.strings.hasPrefixComptime(path, "./")) path[2..] else path; + + // Strip any trailing "/" — `"types": "./dist/"` points at a directory + // whose resolver fallback is implicit `index.d.ts` inside it. + if (rel.len > 0 and rel[rel.len - 1] == '/') { + const dir = rel[0 .. rel.len - 1]; + return cacheFileExistsJoin(cache_dir, folder, dir, "index.d.ts"); + } + + // Extension-carrying paths: + for (type_decl_extensions) |ext| { + if (bun.strings.endsWith(rel, ext)) { + return cacheFileExists(cache_dir, folder, rel); + } + } + // TypeScript source files are themselves typed — the bundler/tsgo + // resolver picks them up directly. + for ([_][]const u8{ ".ts", ".tsx", ".mts", ".cts" }) |ext| { + if (bun.strings.endsWith(rel, ext)) { + return cacheFileExists(cache_dir, folder, rel); + } + } + // JavaScript paths → look for the parallel declaration file. + for (js_to_dts_pairs) |pair| { + if (bun.strings.endsWith(rel, pair.js)) { + const stem_len = rel.len - pair.js.len; + var buf: bun.PathBuffer = undefined; + @memcpy(buf[0..stem_len], rel[0..stem_len]); + @memcpy(buf[stem_len..][0..pair.dts.len], pair.dts); + const sibling = buf[0 .. stem_len + pair.dts.len]; + return cacheFileExists(cache_dir, folder, sibling); + } + } + // Extensionless: could be a directory-as-entry-point with implicit + // `index.d.ts`, or a TypeScript subpath target ("exports": "./src"). + if (cacheFileExistsJoin(cache_dir, folder, rel, "index.d.ts")) return true; + inline for (.{ ".d.ts", ".d.mts", ".d.cts" }) |ext| { + var buf: bun.PathBuffer = undefined; + @memcpy(buf[0..rel.len], rel); + @memcpy(buf[rel.len..][0..ext.len], ext); + const with_ext = buf[0 .. rel.len + ext.len]; + if (cacheFileExists(cache_dir, folder, with_ext)) return true; } return false; } +fn cacheFileExists(cache_dir: FD, folder: []const u8, subpath: []const u8) bool { + var buf: bun.PathBuffer = undefined; + const joined = std.fmt.bufPrintZ(&buf, "{s}/{s}", .{ folder, subpath }) catch return false; + return bun.sys.existsAt(cache_dir, joined); +} + +fn cacheFileExistsJoin(cache_dir: FD, folder: []const u8, dir: []const u8, file: []const u8) bool { + var buf: bun.PathBuffer = undefined; + const joined = std.fmt.bufPrintZ(&buf, "{s}/{s}/{s}", .{ folder, dir, file }) catch return false; + return bun.sys.existsAt(cache_dir, joined); +} + const std = @import("std"); const bun = @import("bun"); diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 52c1c78b1b5..144fa6dfe82 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2047,6 +2047,52 @@ describe("global virtual store", () => { "types-versions/index.js": "module.exports = {};\n", "types-versions/ts4/index.d.ts": "export {};\n", + // No `exports`, no `types`/`typings`, no `typesVersions` — but an + // `index.d.ts` sits at the package root. TS's no-`exports` fallback + // finds it, so the ATW-style resolver must too. + "implicit-index/package.json": JSON.stringify({ + name: "implicit-index", + version: "1.0.0", + }), + "implicit-index/index.js": "module.exports = {};\n", + "implicit-index/index.d.ts": "export {};\n", + + // `"exports"` set without a `types` condition, but the resolved + // JS target has a sibling `.d.ts` — TS picks that up via the + // JS-to-declaration pairing. Common for packages that haven't + // migrated to explicit `types` conditions yet. + "sibling-dts/package.json": JSON.stringify({ + name: "sibling-dts", + version: "1.0.0", + exports: { + ".": { + import: "./index.mjs", + require: "./index.cjs", + }, + }, + }), + "sibling-dts/index.mjs": "export default {};\n", + "sibling-dts/index.cjs": "module.exports = {};\n", + "sibling-dts/index.d.mts": "export {};\n", + "sibling-dts/index.d.cts": "export {};\n", + + // `"exports"` maps every condition to a plain JS file that has NO + // sibling declaration. The field-scan heuristic would have + // false-positive-d on the old `typesVersions`-presence check; ATW + // resolution correctly reports no types exposed. + "exports-no-types/package.json": JSON.stringify({ + name: "exports-no-types", + version: "1.0.0", + exports: { + ".": { + import: "./index.mjs", + require: "./index.cjs", + }, + }, + }), + "exports-no-types/index.mjs": "export default {};\n", + "exports-no-types/index.cjs": "module.exports = {};\n", + "pure-js/package.json": JSON.stringify({ name: "pure-js", version: "1.0.0", @@ -2067,15 +2113,23 @@ describe("global virtual store", () => { return Buffer.from(await Bun.file(join(String(fixtures), `${subdir}-1.0.0.tgz`)).arrayBuffer()); } - const tarballs: Record = { - "top-types": await pack("top-types"), - "top-typings": await pack("top-typings"), - "exports-types": await pack("exports-types"), - "exports-types-dual": await pack("exports-types-dual"), - "exports-types-array": await pack("exports-types-array"), - "types-versions": await pack("types-versions"), - "pure-js": await pack("pure-js"), - }; + const fixtureNames = [ + "top-types", + "top-typings", + "exports-types", + "exports-types-dual", + "exports-types-array", + "types-versions", + "implicit-index", + "sibling-dts", + "exports-no-types", + "pure-js", + ] as const; + // Pack all fixtures in parallel — each `bun pm pack` is a debug-build + // subprocess launch (~2-3s cold); serialising ten of them makes this + // test needlessly flaky under load. + const packed = await Promise.all(fixtureNames.map(n => pack(n))); + const tarballs: Record = Object.fromEntries(fixtureNames.map((n, i) => [n, packed[i]])); using server = Bun.serve({ port: 0, @@ -2126,6 +2180,9 @@ describe("global virtual store", () => { "exports-types-dual": "1.0.0", "exports-types-array": "1.0.0", "types-versions": "1.0.0", + "implicit-index": "1.0.0", + "sibling-dts": "1.0.0", + "exports-no-types": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2135,18 +2192,34 @@ describe("global virtual store", () => { const bunDir = join(String(packageDir), "node_modules", ".bun"); - // A package without any types signal stays in the global store → - // symlink into `/links/-`. - const pureEntry = join(bunDir, "pure-js@1.0.0"); - expect(lstatSync(pureEntry).isSymbolicLink()).toBe(true); - expect(readlinkSync(pureEntry)).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); - - // Each type-shipping signal — top-level `types`, top-level `typings`, - // a `"types"` condition with a string target under `exports`, a - // `"types"` condition with a nested-object target (the dual ESM/CJS - // shape), a `"types"` key nested inside an array-of-fallbacks, or a - // non-empty `"typesVersions"` map — keeps the package project-local - // as a real directory. + // Packages without any resolvable declaration file stay in the + // global store → symlinked into `/links/-`. + // `pure-js` has no types-related package.json signals at all; + // `exports-no-types` has an `"exports"` map whose resolved JS + // targets have no sibling `.d.*` and no `"types"` condition — + // exactly the case where the earlier field-scan heuristic would + // have false-positive-d on the presence of `"exports"` alone. + for (const name of ["pure-js", "exports-no-types"] as const) { + const entry = join(bunDir, `${name}@1.0.0`); + expect(lstatSync(entry).isSymbolicLink()).toBe(true); + expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); + } + + // Every path through the ATW-style resolver lands on a declaration + // file: + // * top-level `"types"` / `"typings"` → string points at .d.ts + // * `"exports"` walk with types-priority conditions → .d.ts + // * `"exports"` walk into a nested conditional `types` object + // (dual .d.mts/.d.cts) → .d.* + // * `"exports"` walk into an array fallback containing a types + // condition → .d.ts + // * non-empty `"typesVersions"` (TS resolves through version + // mappings) → true + // * no `"exports"`/`"types"`/`"typesVersions"` but implicit + // `index.d.ts` at package root → true + // * `"exports"` with no `"types"` condition but the JS target + // has a sibling `.d.ts`/`.d.mts`/`.d.cts` → true + // Each fixture is forced project-local as a real directory. const typeShippingNames = [ "top-types", "top-typings", @@ -2154,6 +2227,8 @@ describe("global virtual store", () => { "exports-types-dual", "exports-types-array", "types-versions", + "implicit-index", + "sibling-dts", ] as const; for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); @@ -2198,7 +2273,10 @@ describe("global virtual store", () => { expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); } - expect(lstatSync(join(bunDir, "pure-js@1.0.0")).isSymbolicLink()).toBe(true); - expect(readlinkSync(join(bunDir, "pure-js@1.0.0"))).toMatch(/links[/\\]pure-js@1\.0\.0-[0-9a-f]{16}$/); + for (const name of ["pure-js", "exports-no-types"] as const) { + const entry = join(bunDir, `${name}@1.0.0`); + expect(lstatSync(entry).isSymbolicLink()).toBe(true); + expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); + } }); }); From a8f7d2afacfd6bfa520c4f3435b83c81eb7e6949 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 4 May 2026 11:16:06 +0000 Subject: [PATCH 09/12] =?UTF-8?q?install:=20ATW-style=20resolver=20?= =?UTF-8?q?=E2=80=94=20subpath-only=20exports,=20node10=20fallback,=20boun?= =?UTF-8?q?ded=20path=20buf?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review-driven tightenings to the type-declaration resolver: 1. pathExposesTypes: bound the sibling-declaration concatenation with std.fmt.bufPrint(&buf, "{s}{s}", ...) catch return false — matches the existing cacheFileExists/cacheFileExistsJoin pattern a few lines below. Previously two @memcpy blocks copied package.json strings straight into bun.PathBuffer with no length check, which panics under ReleaseSafe and writes past the stack array under ReleaseFast for any oversized "types"/"exports" path. 2. exportsExposesTypes: when the map has subpath keys but no "." entry (e.g. { "./Button": { types: ..., default: ... }, "./Card": ... }), walk every subpath's target instead of returning false. ATW inspects every entry point; any subpath exposing declarations means the package ships types to that consumer. 3. resolvePackageExposesTypes: drop the early return when the exports walk says no. moduleResolution: node10 (still the commonjs default, and ATW's "node10" column) ignores "exports" and resolves via top-level "types"/"typings"/implicit index.d.ts. A package shaped { exports: { ".": "./dist/index.js" }, types: "./types/index.d.ts" } where dist/index.d.ts doesn't exist now correctly stays project- local for the node10 audience. Three new fixtures: subpath-only (map without "."), exports-no-types- top-types (node10 fall-through), long-types (2048-char path, stack overflow guard). --- src/install/isolated_install.zig | 121 ++++++++++++++-------- test/cli/install/isolated-install.test.ts | 69 +++++++++++- 2 files changed, 142 insertions(+), 48 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index ed8614ae061..9d2fac14539 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1956,22 +1956,29 @@ pub fn installIsolatedPackages( /// /// Runs the same resolution algorithm TypeScript uses to discover a /// package's declarations (modelled on arethetypeswrong.github.io), on -/// the extracted `//package.json`: +/// the extracted `//package.json`. We want the union of +/// everything ATW inspects — a `true` from any branch means some real +/// consumer (node16, node10, bundler) would resolve a declaration file +/// out of this package: /// -/// 1. If `"exports"` is set, walk the conditional-exports tree for the -/// `"."` subpath with condition priority `[types, import, require, -/// default]` — the same order `tsc --moduleResolution node16/bundler` -/// uses when `"types"` is in its condition set. If the first string -/// target reached is a declaration file, or has a sibling declaration -/// file, the package exposes types. -/// 2. Otherwise, check top-level `"types"` / `"typings"` — resolve the -/// path relative to the package root and apply the same is-or-has- -/// sibling-declaration rule. -/// 3. Otherwise, if `"typesVersions"` is present and non-empty, TS -/// treats the package as type-shipping regardless of subpath content -/// (it'll pick the matching range and look up subpaths there). -/// 4. Otherwise, fall back to implicit `index.d.ts` in the package -/// root (the conventional no-`exports` / no-`types` path). +/// 1. If `"exports"` is set, walk EVERY entry point in the exports +/// tree — not just `"."` — with condition priority `[types, import, +/// require, default]`, the same order `tsc --moduleResolution +/// node16/bundler` uses when `"types"` is in its condition set. If +/// any target resolves to (or has a sibling) declaration file, the +/// package exposes types. This catches subpath-only shapes like +/// `{ "./Button": { ... }, "./Card": { ... } }` that omit `"."`. +/// 2. Also check top-level `"types"` / `"typings"` — these are what +/// `moduleResolution: node10` (still the default under `module: +/// "commonjs"`) consults, and ATW reports against them in its +/// separate "node10" column. A package shaped `{ exports: { ".": +/// "./dist/index.js" }, types: "./types/index.d.ts" }` ships types +/// to node10 consumers even if the exports walk says no. +/// 3. If `"typesVersions"` is present and non-empty, TS treats the +/// package as type-shipping regardless of subpath content (it'll +/// pick the matching range and look up subpaths there). +/// 4. Last resort — implicit `index.d.ts` in the package root, the +/// conventional no-`exports` / no-`types` fallback. /// /// Any positive hit means the package's declaration file(s) are what a /// consuming TypeScript project would try to resolve. If the package's @@ -2087,19 +2094,25 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ else => return true, }; - // 1. `"exports"` — walk the conditional tree for the `.` subpath - // with types-priority conditions. + // 1. `"exports"` — walk the conditional tree with types-priority + // conditions. Covers `moduleResolution: node16 / nodenext / + // bundler` consumers. if (json.asProperty("exports")) |prop| { if (exportsExposesTypes(prop.expr, cache_dir, folder, arena_allocator)) return true; - // When `exports` is set, per the Node resolution spec only - // subpaths listed there are visible to consumers — `types` / - // `typings` / `typesVersions` / implicit `index.d.ts` are NOT - // consulted. Fall through to return false. - return false; + // When `exports` doesn't resolve to a declaration, fall through + // to the top-level signals: `moduleResolution: node10` (still + // the default under `module: "commonjs"`, and what ATW's + // "node10" column reports against) ignores `"exports"` entirely + // and resolves declarations via top-level `"types"` / `"typings"` + // / implicit `index.d.ts`. Skipping those fields here would + // silently route packages shaped `{ exports: { ".": "./dist/index.js" }, + // types: "./types/index.d.ts" }` (where `dist/index.d.ts` + // doesn't exist) into the global store and resurrect #29727 for + // node10 consumers. } // 2. Top-level `"types"` / `"typings"` — TS honors `types` first, - // then `typings` (the legacy name) when no `exports`. + // then `typings` (the legacy name). inline for (.{ "types", "typings" }) |field| { if (json.asProperty(field)) |prop| { if (prop.expr.asString(arena_allocator)) |s| { @@ -2122,39 +2135,53 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ return false; } -/// Resolve the `.` subpath of an `"exports"` tree with condition priority -/// `[types, import, require, default]` and check the resolved target. -/// Returns true iff the target path (or one of its declaration siblings) -/// exists in the extracted cache. +/// Walk an `"exports"` tree and check whether any reachable target is +/// (or has a sibling) declaration file. Returns true on the first hit. +/// +/// ATW inspects every entry point in the exports map, not just `"."` — +/// a package whose only declared subpaths are `"./Button"` and +/// `"./Card"` still ships types to any consumer doing +/// `import … from "pkg/Button"`. fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const u8, allocator: std.mem.Allocator) bool { // `exports` can be: // "exports": "./index.js" → string (shorthand for `.`) // "exports": [ ... ] → array of fallbacks (shorthand for `.`) // "exports": { ".": ..., "./sub": ... } → subpath map // "exports": { "import": ..., "require": ... } → condition map (shorthand for `.`) - const root_target: bun.js_parser.Expr = switch (expr.data) { - .e_string, .e_array => expr, - .e_object => |obj| blk: { + switch (expr.data) { + .e_string, .e_array => return resolveTargetExposesTypes(expr, cache_dir, folder, allocator, 0), + .e_object => |obj| { var has_dot_keys = false; - var dot_value: ?bun.js_parser.Expr = null; for (obj.properties.slice()) |prop| { const key = prop.key orelse continue; if (key.data != .e_string) continue; const s = key.data.e_string.data; if (s.len > 0 and s[0] == '.') { has_dot_keys = true; - if (s.len == 1) dot_value = prop.value; + break; + } + } + if (has_dot_keys) { + // Subpath map: walk every subpath's target. A package + // shaped `{ "./Button": { ... }, "./Card": { ... } }` + // with no `"."` entry still ships types if any of its + // subpath targets resolve to a declaration file. Same + // goes for wildcard subpaths (`./*`) — we can't + // statically resolve the pattern, but if the target + // object contains a `"types"` condition it counts as + // type-shipping just like a non-wildcard subpath. + for (obj.properties.slice()) |prop| { + const value = prop.value orelse continue; + if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, 0)) return true; } + return false; } - // If every key is a subpath (`./x`), the map is purely a - // subpath map and only the `.` entry resolves to the root. - // Otherwise the whole object is the condition tree for `.`. - break :blk if (has_dot_keys) (dot_value orelse return false) else expr; + // No subpath keys: the whole object is the condition tree + // for `.` (shorthand). + return resolveTargetExposesTypes(expr, cache_dir, folder, allocator, 0); }, else => return false, - }; - - return resolveTargetExposesTypes(root_target, cache_dir, folder, allocator, 0); + } } /// Walk a conditional-exports target (string, array, or condition object) @@ -2246,13 +2273,17 @@ fn pathExposesTypes(path: []const u8, cache_dir: FD, folder: []const u8) bool { } } // JavaScript paths → look for the parallel declaration file. + // + // `rel` comes straight from an untrusted `package.json` string; + // bound the concatenation the same way `cacheFileExists` / + // `cacheFileExistsJoin` do further down — `bufPrintZ` returns an + // error when the combined length doesn't fit, which we treat as + // "path too long to plausibly exist, skip". for (js_to_dts_pairs) |pair| { if (bun.strings.endsWith(rel, pair.js)) { - const stem_len = rel.len - pair.js.len; + const stem = rel[0 .. rel.len - pair.js.len]; var buf: bun.PathBuffer = undefined; - @memcpy(buf[0..stem_len], rel[0..stem_len]); - @memcpy(buf[stem_len..][0..pair.dts.len], pair.dts); - const sibling = buf[0 .. stem_len + pair.dts.len]; + const sibling = std.fmt.bufPrint(&buf, "{s}{s}", .{ stem, pair.dts }) catch return false; return cacheFileExists(cache_dir, folder, sibling); } } @@ -2261,9 +2292,7 @@ fn pathExposesTypes(path: []const u8, cache_dir: FD, folder: []const u8) bool { if (cacheFileExistsJoin(cache_dir, folder, rel, "index.d.ts")) return true; inline for (.{ ".d.ts", ".d.mts", ".d.cts" }) |ext| { var buf: bun.PathBuffer = undefined; - @memcpy(buf[0..rel.len], rel); - @memcpy(buf[rel.len..][0..ext.len], ext); - const with_ext = buf[0 .. rel.len + ext.len]; + const with_ext = std.fmt.bufPrint(&buf, "{s}{s}", .{ rel, ext }) catch return false; if (cacheFileExists(cache_dir, folder, with_ext)) return true; } return false; diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 144fa6dfe82..21be9795766 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2093,6 +2093,54 @@ describe("global virtual store", () => { "exports-no-types/index.mjs": "export default {};\n", "exports-no-types/index.cjs": "module.exports = {};\n", + // Subpath-only `"exports"` — no `"."` entry. Component libraries + // often ship like this, forcing consumers onto specific subpaths. + // ATW checks every entry point in the exports map, so any + // subpath that resolves to a declaration file counts as type- + // shipping. + "subpath-only/package.json": JSON.stringify({ + name: "subpath-only", + version: "1.0.0", + exports: { + "./Button": { types: "./Button.d.ts", default: "./Button.js" }, + "./Card": { types: "./Card.d.ts", default: "./Card.js" }, + }, + }), + "subpath-only/Button.js": "module.exports = {};\n", + "subpath-only/Button.d.ts": "export {};\n", + "subpath-only/Card.js": "module.exports = {};\n", + "subpath-only/Card.d.ts": "export {};\n", + + // `"exports"` is set and resolves WITHOUT types (no `types` + // condition, no sibling `.d.*`), but top-level `"types"` points + // at a real declaration file in a separate directory. node16 + // consumers miss these types, but `moduleResolution: node10` + // (still the commonjs default) reads top-level `"types"` and + // resolves them — so the package IS type-shipping for that + // audience. + "exports-no-types-top-types/package.json": JSON.stringify({ + name: "exports-no-types-top-types", + version: "1.0.0", + exports: { + ".": "./dist/index.js", + }, + types: "./types/index.d.ts", + }), + "exports-no-types-top-types/dist/index.js": "module.exports = {};\n", + "exports-no-types-top-types/types/index.d.ts": "export {};\n", + + // Pathologically long `"types"` path (well over macOS's 1024-byte + // PathBuffer). Must not stack-overflow the sibling `@memcpy` / + // `bufPrint` in `pathExposesTypes`; must not crash `bun install` + // either. Resolves as "no types" because the long path doesn't + // exist on disk, so the package lands in the global store. + "long-types/package.json": JSON.stringify({ + name: "long-types", + version: "1.0.0", + types: "./" + "a".repeat(2048) + ".d.ts", + }), + "long-types/index.js": "module.exports = {};\n", + "pure-js/package.json": JSON.stringify({ name: "pure-js", version: "1.0.0", @@ -2122,7 +2170,10 @@ describe("global virtual store", () => { "types-versions", "implicit-index", "sibling-dts", + "subpath-only", + "exports-no-types-top-types", "exports-no-types", + "long-types", "pure-js", ] as const; // Pack all fixtures in parallel — each `bun pm pack` is a debug-build @@ -2182,7 +2233,10 @@ describe("global virtual store", () => { "types-versions": "1.0.0", "implicit-index": "1.0.0", "sibling-dts": "1.0.0", + "subpath-only": "1.0.0", + "exports-no-types-top-types": "1.0.0", "exports-no-types": "1.0.0", + "long-types": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2199,7 +2253,11 @@ describe("global virtual store", () => { // targets have no sibling `.d.*` and no `"types"` condition — // exactly the case where the earlier field-scan heuristic would // have false-positive-d on the presence of `"exports"` alone. - for (const name of ["pure-js", "exports-no-types"] as const) { + // `long-types` declares `"types": "./aaaa…a.d.ts"` longer than + // `PathBuffer` — the sibling/bufPrint paths must handle overflow + // gracefully, treat it as non-existent, and leave the package + // symlinked. + for (const name of ["pure-js", "exports-no-types", "long-types"] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(true); expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); @@ -2229,6 +2287,13 @@ describe("global virtual store", () => { "types-versions", "implicit-index", "sibling-dts", + // Every subpath carries a `types` condition → at least one entry + // point exposes declarations → package ships types. + "subpath-only", + // `"exports"` resolves without types, but top-level `"types"` + // points at a real .d.ts for node10 consumers. The fall-through + // after the exports walk catches this. + "exports-no-types-top-types", ] as const; for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); @@ -2273,7 +2338,7 @@ describe("global virtual store", () => { expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); } - for (const name of ["pure-js", "exports-no-types"] as const) { + for (const name of ["pure-js", "exports-no-types", "long-types"] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(true); expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); From a371e0af3d2a768965ce16acc73f513bb218a173 Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 4 May 2026 19:03:46 +0000 Subject: [PATCH 10/12] install: wildcard-subpath types, real stack-overflow coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review nits from the previous round: 1. Wildcard subpath targets (`"./icons/*.d.ts"`) can't be fstat'd — cacheFileExists was calling existsAt on the literal '*' path and returning false, so packages with only wildcard subpaths were treated as not shipping types. pathExposesTypes now short-circuits when the path contains '*': trust the declared extension. Any pattern ending in a declaration (`.d.ts`/`.d.mts`/`.d.cts`) or TypeScript source extension (`.ts`/`.tsx`/`.mts`/`.cts`) counts as type-shipping. JS patterns without a sibling declaration still return false — if the author wanted types treated they'd have added a `types` condition. 2. The long-types fixture used a `.d.ts` suffix, which takes the early-return branch in pathExposesTypes and never reaches either of the two `bufPrint catch return false` guards the prior commit added. And 2048 chars doesn't overflow Linux's 4096-byte PathBuffer. Replaced with two fixtures: * long-types-js: `types: "./<5000 'a's>.js"` routes through the js_to_dts_pairs sibling bufPrint site. * long-types-bare: `types: "./<5000 'a's>"` (no extension) routes through the extensionless append-`.d.*` site. Both exceed macOS (1024) and Linux (4096) PathBuffer ceilings, so the guards actually fire. Verified by stashing src/: unfixed code panics on the debug build (test times out); fixed code runs clean. Added wildcard-types and wildcard-no-types fixtures to lock in the wildcard extension rule and its negative. --- src/install/isolated_install.zig | 35 ++++++- test/cli/install/isolated-install.test.ts | 118 ++++++++++++++++++---- 2 files changed, 126 insertions(+), 27 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index 9d2fac14539..ab392550880 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -2165,11 +2165,14 @@ fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const // Subpath map: walk every subpath's target. A package // shaped `{ "./Button": { ... }, "./Card": { ... } }` // with no `"."` entry still ships types if any of its - // subpath targets resolve to a declaration file. Same - // goes for wildcard subpaths (`./*`) — we can't - // statically resolve the pattern, but if the target - // object contains a `"types"` condition it counts as - // type-shipping just like a non-wildcard subpath. + // subpath targets resolve to a declaration file. + // + // Wildcard subpaths (`"./*"`, `"./icons/*"`) are handled + // inside `pathExposesTypes`: a pattern target whose + // literal path contains `*` can't be fstat'd, so we + // trust the declared extension — any wildcard whose + // target ends in `.d.ts` / `.d.mts` / `.d.cts` / `.ts` + // / `.tsx` / `.mts` / `.cts` counts as type-shipping. for (obj.properties.slice()) |prop| { const value = prop.value orelse continue; if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, 0)) return true; @@ -2252,6 +2255,18 @@ fn pathExposesTypes(path: []const u8, cache_dir: FD, folder: []const u8) bool { // Normalize leading "./" const rel = if (bun.strings.hasPrefixComptime(path, "./")) path[2..] else path; + // Wildcard subpath targets (`"./icons/*.d.ts"`) can't be + // `existsAt`-checked literally — the `*` stands for a consumer- + // supplied name the resolver substitutes at import time. Trust the + // declared extension: a pattern ending in a declaration or TS + // source extension means the author wrote a types entry point for + // those subpaths. We don't try to match JS patterns against hidden + // sibling `.d.ts` files — if the author wanted that treated as + // type-shipping they'd have added a `types` condition. + if (bun.strings.containsChar(rel, '*')) { + return hasTypeExtension(rel); + } + // Strip any trailing "/" — `"types": "./dist/"` points at a directory // whose resolver fallback is implicit `index.d.ts` inside it. if (rel.len > 0 and rel[rel.len - 1] == '/') { @@ -2298,6 +2313,16 @@ fn pathExposesTypes(path: []const u8, cache_dir: FD, folder: []const u8) bool { return false; } +fn hasTypeExtension(rel: []const u8) bool { + for (type_decl_extensions) |ext| { + if (bun.strings.endsWith(rel, ext)) return true; + } + for ([_][]const u8{ ".ts", ".tsx", ".mts", ".cts" }) |ext| { + if (bun.strings.endsWith(rel, ext)) return true; + } + return false; +} + fn cacheFileExists(cache_dir: FD, folder: []const u8, subpath: []const u8) bool { var buf: bun.PathBuffer = undefined; const joined = std.fmt.bufPrintZ(&buf, "{s}/{s}", .{ folder, subpath }) catch return false; diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 21be9795766..6966935af8e 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2129,17 +2129,63 @@ describe("global virtual store", () => { "exports-no-types-top-types/dist/index.js": "module.exports = {};\n", "exports-no-types-top-types/types/index.d.ts": "export {};\n", - // Pathologically long `"types"` path (well over macOS's 1024-byte - // PathBuffer). Must not stack-overflow the sibling `@memcpy` / - // `bufPrint` in `pathExposesTypes`; must not crash `bun install` - // either. Resolves as "no types" because the long path doesn't - // exist on disk, so the package lands in the global store. - "long-types/package.json": JSON.stringify({ - name: "long-types", + // Pathologically long `"types"` path — exercises the two + // `bufPrint(&buf, "{s}{s}", ...) catch return false` guards in + // `pathExposesTypes`: + // 1. `.js` suffix (with no literal sibling `.d.ts`) routes + // through the `js_to_dts_pairs` sibling-concatenation site. + // 2. Extensionless path routes through the + // `.d.ts`/`.d.mts`/`.d.cts` append-extension site. + // 5000 chars exceeds both macOS's 1024-byte and Linux's 4096- + // byte `PathBuffer` ceilings, so the `bufPrint` actually fails + // with `NoSpaceLeft` and the guard returns false on every + // platform. Without the guards this would overflow the stack + // buffer with attacker-controlled bytes (ReleaseFast) or + // panic (Debug / ReleaseSafe). + "long-types-js/package.json": JSON.stringify({ + name: "long-types-js", version: "1.0.0", - types: "./" + "a".repeat(2048) + ".d.ts", + types: "./" + "a".repeat(5000) + ".js", }), - "long-types/index.js": "module.exports = {};\n", + "long-types-js/index.js": "module.exports = {};\n", + + "long-types-bare/package.json": JSON.stringify({ + name: "long-types-bare", + version: "1.0.0", + types: "./" + "a".repeat(5000), + }), + "long-types-bare/index.js": "module.exports = {};\n", + + // Wildcard subpath `"exports"` — component libraries publishing + // per-component entry points often use this shape. The `*` in + // the target path can't be fstat'd literally; the resolver + // trusts the declared extension so any wildcard target ending + // in `.d.ts` / `.d.mts` / `.d.cts` / `.ts` / `.tsx` / `.mts` / + // `.cts` counts as type-shipping. + "wildcard-types/package.json": JSON.stringify({ + name: "wildcard-types", + version: "1.0.0", + exports: { + "./icons/*": { types: "./icons/*.d.ts", default: "./icons/*.js" }, + }, + }), + "wildcard-types/icons/Heart.js": "module.exports = {};\n", + "wildcard-types/icons/Heart.d.ts": "export {};\n", + + // Wildcard subpath whose target has NO `types` condition and + // whose pattern ends in `.js`. We don't try to prove a sibling + // `.d.ts` exists for wildcard JS targets (would require globbing + // the extracted tree) — if the author wanted types treated, the + // convention is to add a `types` condition. Matches pre-PR + // behavior: no detected types signal → package is eligible. + "wildcard-no-types/package.json": JSON.stringify({ + name: "wildcard-no-types", + version: "1.0.0", + exports: { + "./icons/*": "./icons/*.js", + }, + }), + "wildcard-no-types/icons/Heart.js": "module.exports = {};\n", "pure-js/package.json": JSON.stringify({ name: "pure-js", @@ -2172,8 +2218,11 @@ describe("global virtual store", () => { "sibling-dts", "subpath-only", "exports-no-types-top-types", + "wildcard-types", "exports-no-types", - "long-types", + "wildcard-no-types", + "long-types-js", + "long-types-bare", "pure-js", ] as const; // Pack all fixtures in parallel — each `bun pm pack` is a debug-build @@ -2235,8 +2284,11 @@ describe("global virtual store", () => { "sibling-dts": "1.0.0", "subpath-only": "1.0.0", "exports-no-types-top-types": "1.0.0", + "wildcard-types": "1.0.0", "exports-no-types": "1.0.0", - "long-types": "1.0.0", + "wildcard-no-types": "1.0.0", + "long-types-js": "1.0.0", + "long-types-bare": "1.0.0", "pure-js": "1.0.0", }, }), @@ -2248,16 +2300,28 @@ describe("global virtual store", () => { // Packages without any resolvable declaration file stay in the // global store → symlinked into `/links/-`. - // `pure-js` has no types-related package.json signals at all; - // `exports-no-types` has an `"exports"` map whose resolved JS - // targets have no sibling `.d.*` and no `"types"` condition — - // exactly the case where the earlier field-scan heuristic would - // have false-positive-d on the presence of `"exports"` alone. - // `long-types` declares `"types": "./aaaa…a.d.ts"` longer than - // `PathBuffer` — the sibling/bufPrint paths must handle overflow - // gracefully, treat it as non-existent, and leave the package - // symlinked. - for (const name of ["pure-js", "exports-no-types", "long-types"] as const) { + // * `pure-js`: no types-related package.json signals at all. + // * `exports-no-types`: `"exports"` map whose resolved JS targets + // have no sibling `.d.*` and no `"types"` condition — exactly + // the case where the earlier field-scan heuristic would have + // false-positive-d on the presence of `"exports"` alone. + // * `wildcard-no-types`: wildcard subpath whose target has no + // types condition; we don't glob-expand JS patterns so no + // types signal fires. + // * `long-types-js`: `types: "./<5000 'a's>.js"` — routes into + // the `js_to_dts_pairs` bufPrint site with a combined length + // that overflows both macOS (1024) and Linux (4096) `PathBuffer` + // ceilings. The guard returns false; install succeeds; package + // stays symlinked. + // * `long-types-bare`: `types: "./<5000 'a's>"` (no extension) — + // same thing for the extensionless append-`.d.*` site. + for (const name of [ + "pure-js", + "exports-no-types", + "wildcard-no-types", + "long-types-js", + "long-types-bare", + ] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(true); expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); @@ -2294,6 +2358,10 @@ describe("global virtual store", () => { // points at a real .d.ts for node10 consumers. The fall-through // after the exports walk catches this. "exports-no-types-top-types", + // Wildcard subpath whose target has a `types` condition — the + // pattern can't be fstat'd literally but the declared `.d.ts` + // extension is enough of a signal. + "wildcard-types", ] as const; for (const name of typeShippingNames) { const entry = join(bunDir, `${name}@1.0.0`); @@ -2338,7 +2406,13 @@ describe("global virtual store", () => { expect(lstatSync(entry).isSymbolicLink()).toBe(false); expect(lstatSync(entry).isDirectory()).toBe(true); } - for (const name of ["pure-js", "exports-no-types", "long-types"] as const) { + for (const name of [ + "pure-js", + "exports-no-types", + "wildcard-no-types", + "long-types-js", + "long-types-bare", + ] as const) { const entry = join(bunDir, `${name}@1.0.0`); expect(lstatSync(entry).isSymbolicLink()).toBe(true); expect(readlinkSync(entry)).toMatch(new RegExp(`links[/\\\\]${name}@1\\.0\\.0-[0-9a-f]{16}$`)); From 41e880bb4ce7430fd36ac71dad2a34249855338c Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 4 May 2026 19:43:00 +0000 Subject: [PATCH 11/12] install: node10 main-sibling; drop unused allocator parameter Two review-driven tightenings: 1. Add node10 `main`-sibling resolution to the fall-through chain. When no `types`/`typings` field is set, TypeScript's legacy `node` module resolution (still the default under `module: "commonjs"`) strips the extension from `main` and probes `.d.ts` before falling back to `./index.d.ts`. Catches the classic `tsc --declaration --outDir lib` shape: `{ "main": "./lib/index.js" }` with `lib/index.d.ts` next to it and no explicit `types` field. 2. Drop the unused `allocator` parameter from `exportsExposesTypes` and `resolveTargetExposesTypes`. Residue from the b95ff7e ATW rewrite: threaded through six call sites but never dereferenced (the .e_string arm reads `str.data` directly, and the object-key classification reads `key.data.e_string.data` directly). Added `main-sibling` fixture to lock in the new fallback. --- src/install/isolated_install.zig | 45 ++++++++++++++++------- test/cli/install/isolated-install.test.ts | 20 ++++++++++ 2 files changed, 52 insertions(+), 13 deletions(-) diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index ab392550880..7538386dd5f 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -1974,10 +1974,15 @@ pub fn installIsolatedPackages( /// separate "node10" column. A package shaped `{ exports: { ".": /// "./dist/index.js" }, types: "./types/index.d.ts" }` ships types /// to node10 consumers even if the exports walk says no. -/// 3. If `"typesVersions"` is present and non-empty, TS treats the +/// 3. node10 `"main"`-sibling — when neither `"types"` nor `"typings"` +/// is set, TS strips the extension from `"main"` and probes +/// `.d.ts`. A package shaped `{ "main": "./lib/index.js" }` +/// with `lib/index.d.ts` present ships types this way even when +/// the package root has no `index.d.ts`. +/// 4. If `"typesVersions"` is present and non-empty, TS treats the /// package as type-shipping regardless of subpath content (it'll /// pick the matching range and look up subpaths there). -/// 4. Last resort — implicit `index.d.ts` in the package root, the +/// 5. Last resort — implicit `index.d.ts` in the package root, the /// conventional no-`exports` / no-`types` fallback. /// /// Any positive hit means the package's declaration file(s) are what a @@ -2098,7 +2103,7 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ // conditions. Covers `moduleResolution: node16 / nodenext / // bundler` consumers. if (json.asProperty("exports")) |prop| { - if (exportsExposesTypes(prop.expr, cache_dir, folder, arena_allocator)) return true; + if (exportsExposesTypes(prop.expr, cache_dir, folder)) return true; // When `exports` doesn't resolve to a declaration, fall through // to the top-level signals: `moduleResolution: node10` (still // the default under `module: "commonjs"`, and what ATW's @@ -2121,7 +2126,22 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ } } - // 3. `"typesVersions"` — presence-only. TS resolves subpaths through + // 3. node10 `"main"`-sibling — when no `types`/`typings` field is + // set, TypeScript's legacy `node` module resolution (still the + // default under `module: "commonjs"`, and ATW's "node10" column) + // strips the extension from `"main"` and probes + // `.d.ts` before falling back to `./index.d.ts`. + // Catches the classic `tsc --declaration --outDir lib` shape + // (`{ "main": "./lib/index.js" }` with `lib/index.d.ts` on disk + // and no explicit `"types"` field). `pathExposesTypes` handles + // the `.js`→sibling-`.d.ts` pairing via `js_to_dts_pairs`. + if (json.asProperty("main")) |prop| { + if (prop.expr.asString(arena_allocator)) |s| { + if (pathExposesTypes(s, cache_dir, folder)) return true; + } + } + + // 4. `"typesVersions"` — presence-only. TS resolves subpaths through // whichever version range matches the consumer's TS version; we // have no TS version to match here, so treat any non-empty map as // a type-shipping signal. @@ -2129,7 +2149,7 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ if (prop.expr.data == .e_object and prop.expr.data.e_object.properties.len > 0) return true; } - // 4. Implicit `index.d.ts` — the no-`exports`/no-`types` fallback. + // 5. Implicit `index.d.ts` — the no-`exports`/no-`types` fallback. if (cacheFileExists(cache_dir, folder, "index.d.ts")) return true; return false; @@ -2142,14 +2162,14 @@ fn resolvePackageExposesTypes(source_bytes: []const u8, cache_dir: FD, folder: [ /// a package whose only declared subpaths are `"./Button"` and /// `"./Card"` still ships types to any consumer doing /// `import … from "pkg/Button"`. -fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const u8, allocator: std.mem.Allocator) bool { +fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const u8) bool { // `exports` can be: // "exports": "./index.js" → string (shorthand for `.`) // "exports": [ ... ] → array of fallbacks (shorthand for `.`) // "exports": { ".": ..., "./sub": ... } → subpath map // "exports": { "import": ..., "require": ... } → condition map (shorthand for `.`) switch (expr.data) { - .e_string, .e_array => return resolveTargetExposesTypes(expr, cache_dir, folder, allocator, 0), + .e_string, .e_array => return resolveTargetExposesTypes(expr, cache_dir, folder, 0), .e_object => |obj| { var has_dot_keys = false; for (obj.properties.slice()) |prop| { @@ -2175,13 +2195,13 @@ fn exportsExposesTypes(expr: bun.js_parser.Expr, cache_dir: FD, folder: []const // / `.tsx` / `.mts` / `.cts` counts as type-shipping. for (obj.properties.slice()) |prop| { const value = prop.value orelse continue; - if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, 0)) return true; + if (resolveTargetExposesTypes(value, cache_dir, folder, 0)) return true; } return false; } // No subpath keys: the whole object is the condition tree // for `.` (shorthand). - return resolveTargetExposesTypes(expr, cache_dir, folder, allocator, 0); + return resolveTargetExposesTypes(expr, cache_dir, folder, 0); }, else => return false, } @@ -2196,7 +2216,6 @@ fn resolveTargetExposesTypes( target: bun.js_parser.Expr, cache_dir: FD, folder: []const u8, - allocator: std.mem.Allocator, depth: u8, ) bool { if (depth > 16) return false; @@ -2210,7 +2229,7 @@ fn resolveTargetExposesTypes( // resolvable target wins. For our yes/no question the order // doesn't matter — any alternative exposing types is enough. for (array.items.slice()) |item| { - if (resolveTargetExposesTypes(item, cache_dir, folder, allocator, depth + 1)) return true; + if (resolveTargetExposesTypes(item, cache_dir, folder, depth + 1)) return true; } return false; }, @@ -2230,7 +2249,7 @@ fn resolveTargetExposesTypes( const value = prop.value orelse continue; if (key.data != .e_string) continue; if (key.data.e_string.eqlComptime("types")) { - if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, depth + 1)) return true; + if (resolveTargetExposesTypes(value, cache_dir, folder, depth + 1)) return true; } } for (obj.properties.slice()) |prop| { @@ -2238,7 +2257,7 @@ fn resolveTargetExposesTypes( const value = prop.value orelse continue; if (key.data != .e_string) continue; if (key.data.e_string.eqlComptime("types")) continue; - if (resolveTargetExposesTypes(value, cache_dir, folder, allocator, depth + 1)) return true; + if (resolveTargetExposesTypes(value, cache_dir, folder, depth + 1)) return true; } return false; }, diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 6966935af8e..ea9e0e6b4ea 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2057,6 +2057,20 @@ describe("global virtual store", () => { "implicit-index/index.js": "module.exports = {};\n", "implicit-index/index.d.ts": "export {};\n", + // node10 main-sibling: classic `tsc --declaration --outDir lib` + // shape. No `exports`/`types`/`typings`/`typesVersions`, no root + // `index.d.ts`, but `"main"` points at `./lib/index.js` and + // `lib/index.d.ts` sits next to it. TypeScript'"'"'s legacy `node` + // resolution strips the `.js` extension from `main` and probes + // `.d.ts` → finds the declaration. + "main-sibling/package.json": JSON.stringify({ + name: "main-sibling", + version: "1.0.0", + main: "./lib/index.js", + }), + "main-sibling/lib/index.js": "module.exports = {};\n", + "main-sibling/lib/index.d.ts": "export {};\n", + // `"exports"` set without a `types` condition, but the resolved // JS target has a sibling `.d.ts` — TS picks that up via the // JS-to-declaration pairing. Common for packages that haven't @@ -2215,6 +2229,7 @@ describe("global virtual store", () => { "exports-types-array", "types-versions", "implicit-index", + "main-sibling", "sibling-dts", "subpath-only", "exports-no-types-top-types", @@ -2281,6 +2296,7 @@ describe("global virtual store", () => { "exports-types-array": "1.0.0", "types-versions": "1.0.0", "implicit-index": "1.0.0", + "main-sibling": "1.0.0", "sibling-dts": "1.0.0", "subpath-only": "1.0.0", "exports-no-types-top-types": "1.0.0", @@ -2350,6 +2366,10 @@ describe("global virtual store", () => { "exports-types-array", "types-versions", "implicit-index", + // node10 `main`-sibling: no explicit `types` field, but + // `main: "./lib/index.js"` has `lib/index.d.ts` next to it. The + // classic `tsc --declaration --outDir lib` output shape. + "main-sibling", "sibling-dts", // Every subpath carries a `types` condition → at least one entry // point exposes declarations → package ships types. From d832cb7d8495f60eb49a1e7453831af39c0c649d Mon Sep 17 00:00:00 2001 From: robobun Date: Mon, 4 May 2026 20:12:17 +0000 Subject: [PATCH 12/12] test(isolated-install): fix shell-escape leak in main-sibling fixture comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `'"'"'` sequence on the TypeScript'"'"'s comment line was a bash single-quote-escape idiom that leaked into the source when the comment was added in 41e880b. Cosmetic only — lives in a // comment, no test or runtime impact — but still unintended garbage in committed source. --- test/cli/install/isolated-install.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index ea9e0e6b4ea..0cfb4843baa 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -2060,7 +2060,7 @@ describe("global virtual store", () => { // node10 main-sibling: classic `tsc --declaration --outDir lib` // shape. No `exports`/`types`/`typings`/`typesVersions`, no root // `index.d.ts`, but `"main"` points at `./lib/index.js` and - // `lib/index.d.ts` sits next to it. TypeScript'"'"'s legacy `node` + // `lib/index.d.ts` sits next to it. TypeScript's legacy `node` // resolution strips the `.js` extension from `main` and probes // `.d.ts` → finds the declaration. "main-sibling/package.json": JSON.stringify({