Skip to content

install: keep a peer nothing in bun.lock satisfies where the file records it #38892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 3 commits into
base: main
Choose a base branch
from
+431 −29
Open

install: keep a peer nothing in bun.lock satisfies where the file records it #38892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fbb385d
install: keep a peer nothing in bun.lock satisfies where the file rec…
robobun Aug 15, 2026
2ff1795
install: write the peer candidate scan as find; frozen install of the…
robobun Aug 15, 2026
e2d1e20
install: pin the boundaries of reading an unsatisfied peer's binding …
robobun Aug 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 8 additions & 29 deletions src/install/lockfile/bun.lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3336,13 +3336,9 @@ fn deferred_peer_range<'a>(
/// `install_peer`): scan the package ids recorded for the dependency's
/// name — `package_index` lists are kept ordered by descending
/// `Resolution::order` — and take the first whose resolution satisfies
/// the range. When nothing satisfies, fall back to the highest-ordered
/// candidate, and only when it is the same kind as the dependency (the
/// "incorrect peer dependency" case; the fresh resolver inspects only
/// `list[0]` there, and reproducing its choice exactly is the point of
/// this helper). Returns `None` when no package with the name exists
/// or the fallback is a different kind; the caller then falls back to
/// the path walk. Edges `deferred_peer_range` rejects also return `None`.
/// the range. Returns `None` when no candidate satisfies it (the tree the
/// caller falls back to is the only record of the resolver's "incorrect
/// peer dependency" pick) and for the edges `deferred_peer_range` rejects.
Comment thread
robobun marked this conversation as resolved.
///
/// Peer edges cannot be resolved from the printed tree the way regular
/// edges are: a peer never materializes its own `node_modules` path when
Expand Down Expand Up @@ -3408,28 +3404,11 @@ pub(crate) fn resolve_peer_dep_version_based(
}

let candidates = package_index.get(&name_hash)?.as_slice();
for &id in candidates {
if (id as usize) < pkg_resolutions.len()
&& pkg_resolutions[id as usize]
.satisfies_dependency_version(range, string_buf, string_buf)
{
return Some(id);
}
}

let &first = candidates.first()?;
if (first as usize) < pkg_resolutions.len() {
let res_tag = pkg_resolutions[first as usize].tag;
let ver_tag = range.tag;
if (res_tag == ResolutionTag::Npm && ver_tag == DependencyVersionTag::Npm)
|| (res_tag == ResolutionTag::Git && ver_tag == DependencyVersionTag::Git)
|| (res_tag == ResolutionTag::Github && ver_tag == DependencyVersionTag::Github)
{
return Some(first);
}
}

None
candidates.iter().copied().find(|&id| {
pkg_resolutions
.get(id as usize)
.is_some_and(|res| res.satisfies_dependency_version(range, string_buf, string_buf))
})
}

// Taking `&mut BinaryLockfile` plus a `&mut Dependency` that
Expand Down
238 changes: 238 additions & 0 deletions test/cli/install/bun-lock.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,244 @@ it.each([
await run(["install", "--frozen-lockfile"]);
});

// When no version in the lockfile satisfies a required peer's range, the resolver binds the
// edge to whichever version it saw first and later installs keep that binding, so the only
// record of it is the tree: the version is printed next to the dependent when it conflicts
// with the version hoisted above, and otherwise the edge was deduped onto the hoisted one.
// Loading has to read that record. Picking the highest version in the file instead moved
// the edge whenever the file held another out-of-range version, and the re-save then
// printed a different tree: the recorded copy dropped, or a new nested copy added. These
// shapes are what a lockfile looks like once the package that provided the version the
// peer was first bound to has left the project. The record only holds while nothing in the
// file satisfies the range: once a satisfying version enters the file, loading binds the peer
// to it by version (as a fresh install would) and the next save drops the recorded copy.
describe("loading bun.lock keeps a peer nothing in the file satisfies where the file records it", () => {
const pkg = (nameAndVersion: string, info: object = {}) => [nameAndVersion, "", info, ""];
const oneDep = pkg("one-dep@1.0.0", { dependencies: { "no-deps": "1.0.1" } });
const strictPeerDep = pkg("strict-peer-dep@1.0.0", { peerDependencies: { "no-deps": "^2.0.0" } });

type Shape = {
root: Record<string, unknown>;
workspaces?: Record<string, Record<string, unknown>>;
packages: Record<string, unknown[]>;
saved: string[];
absent?: string[];
/** Set when the shape names packages the test registry does not have, so only `--lockfile-only` can run. */
unpublished?: true;
};

const shapes: [string, Shape][] = [
[
"a package's peer on the copy printed next to it",
{
root: { dependencies: { "one-dep": "1.0.0", "strict-peer-dep": "1.0.0" } },
packages: {
"no-deps": pkg("no-deps@1.0.1"),
"one-dep": oneDep,
"strict-peer-dep": strictPeerDep,
"strict-peer-dep/no-deps": pkg("no-deps@1.0.0"),
},
saved: ['"no-deps": ["no-deps@1.0.1"', '"strict-peer-dep/no-deps": ["no-deps@1.0.0"'],
},
],
[
"a package's peer on the copy hoisted above it",
{
// one-dep is a devDependency so that its no-deps@1.0.1 is hoisted first and holds the
// root slot; the higher 1.1.0 is nested, and the peer was deduped onto the root copy.
root: {
devDependencies: { "one-dep": "1.0.0" },
dependencies: { "normal-dep-and-dev-dep": "1.0.1", "strict-peer-dep": "1.0.0" },
},
packages: {
"no-deps": pkg("no-deps@1.0.1"),
"normal-dep-and-dev-dep": pkg("normal-dep-and-dev-dep@1.0.1", { dependencies: { "no-deps": "1.1.0" } }),
"normal-dep-and-dev-dep/no-deps": pkg("no-deps@1.1.0"),
"one-dep": oneDep,
"strict-peer-dep": strictPeerDep,
},
saved: ['"no-deps": ["no-deps@1.0.1"', '"normal-dep-and-dev-dep/no-deps": ["no-deps@1.1.0"'],
absent: ['"strict-peer-dep/no-deps"'],
},
],
[
"a workspace's peer on the copy printed next to it",
{
// workspace `a` is hoisted before `w`, so its no-deps holds the root slot
root: { workspaces: ["packages/*"] },
workspaces: {
"packages/a": { name: "a", version: "1.0.0", dependencies: { "no-deps": "1.0.1" } },
"packages/w": { name: "w", version: "1.0.0", peerDependencies: { "no-deps": "^2.0.0" } },
},
packages: {
"a": ["a@workspace:packages/a"],
"w": ["w@workspace:packages/w"],
"no-deps": pkg("no-deps@1.0.1"),
"w/no-deps": pkg("no-deps@1.0.0"),
},
saved: ['"no-deps": ["no-deps@1.0.1"', '"w/no-deps": ["no-deps@1.0.0"'],
},
],
[
"the root's own peer on the copy at the root",
{
root: { dependencies: { "one-dep": "1.0.0" }, peerDependencies: { "no-deps": "^2.0.0" } },
packages: {
"no-deps": pkg("no-deps@1.0.0"),
"one-dep": oneDep,
"one-dep/no-deps": pkg("no-deps@1.0.1"),
},
saved: ['"no-deps": ["no-deps@1.0.0"', '"one-dep/no-deps": ["no-deps@1.0.1"'],
},
],
[
"a package printed at two paths on the copy printed next to its last path",
{
// dup@1.0.0 is printed under both parents because the root holds dup@2.0.0. Its peer's
// copy is printed under z-parent/dup only: under a-parent/dup it was deduped onto the
// root's own no-deps, which root dependencies do regardless of range. The loader binds
// the package's edges once per printed path and the last path wins, so the record is
// read back here because z-parent sorts after a-parent; were the parents named the
// other way round, the root's copy would win and the next save would rewrite the entry to it.
root: {
dependencies: { "a-parent": "1.0.0", "dup": "2.0.0", "no-deps": "1.0.1", "z-parent": "1.0.0" },
},
packages: {
"a-parent": pkg("a-parent@1.0.0", { dependencies: { dup: "1.0.0" } }),
"dup": pkg("dup@2.0.0"),
"no-deps": pkg("no-deps@1.0.1"),
"z-parent": pkg("z-parent@1.0.0", { dependencies: { "dup": "1.0.0", "no-deps": "1.1.0" } }),
"a-parent/dup": pkg("dup@1.0.0", { peerDependencies: { "no-deps": "^2.0.0" } }),
"z-parent/dup": pkg("dup@1.0.0", { peerDependencies: { "no-deps": "^2.0.0" } }),
"z-parent/no-deps": pkg("no-deps@1.1.0"),
"z-parent/dup/no-deps": pkg("no-deps@1.0.0"),
},
saved: ['"z-parent/dup/no-deps": ["no-deps@1.0.0"'],
absent: ['"a-parent/dup/no-deps"'],
unpublished: true,
},
],
];

async function writeProject(packageDir: string, shape: Pick<Shape, "root" | "workspaces" | "packages">) {
await write(join(packageDir, "package.json"), JSON.stringify({ name: "foo", ...shape.root }));
for (const [path, manifest] of Object.entries(shape.workspaces ?? {})) {
await write(join(packageDir, path, "package.json"), JSON.stringify(manifest));
}
await write(
join(packageDir, "bun.lock"),
JSON.stringify({
lockfileVersion: 1,
configVersion: 0,
workspaces: { "": { name: "foo", ...shape.root, workspaces: undefined }, ...shape.workspaces },
packages: shape.packages,
}),
);
}

// Re-saves the shape once, checks the entries it must and must not print, and checks that
// the result is a fixed point: a further re-save leaves it alone and a frozen install accepts it.
async function resave(shape: Shape) {
const { packageDir } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } });
const run = makeInstallRunner(packageDir);
await writeProject(packageDir, shape);

await run(["install", "--lockfile-only"]);
const saved = await file(join(packageDir, "bun.lock")).text();
for (const entry of shape.saved) {
expect(saved).toContain(entry);
}
for (const entry of shape.absent ?? []) {
expect(saved).not.toContain(entry);
}

await run(["install", "--lockfile-only"]);
if (!shape.unpublished) await run(["install", "--frozen-lockfile"]);
expect(await file(join(packageDir, "bun.lock")).text()).toBe(saved);
}

it.each(shapes)("re-saving keeps %s", (_, shape) => resave(shape));

const [, nestedCopy] = shapes[0];

it("re-saving rebinds the peer once a version satisfying its range enters the file", () =>
resave({
// The first shape after `bun add one-fixed-dep@2.0.0` brought in no-deps@2.0.0: the
// recorded 1.0.0 is no longer the binding, so the re-save replaces it instead of keeping it.
root: { dependencies: { ...(nestedCopy.root.dependencies as object), "one-fixed-dep": "2.0.0" } },
packages: {
...nestedCopy.packages,
"one-fixed-dep": pkg("one-fixed-dep@2.0.0", { dependencies: { "no-deps": "2.0.0" } }),
"one-fixed-dep/no-deps": pkg("no-deps@2.0.0"),
},
saved: [
'"no-deps": ["no-deps@1.0.1"',
'"one-fixed-dep/no-deps": ["no-deps@2.0.0"',
'"strict-peer-dep/no-deps": ["no-deps@2.0.0"',
],
absent: ["no-deps@1.0.0"],
}));

it("a package's peer on the root's own out-of-range copy binds to it, not to the higher version nested elsewhere", async () => {
// Nothing is printed for this edge: either binding dedupes onto the root's copy when the
// tree is built, so the file is the same both ways and both linkers install the root's copy.
// The binding itself is what `pm why` reports (and what a tree built without the root's
// copy, such as `--production` when it is a devDependency, installs next to the dependent).
const { packageDir } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } });
await writeProject(packageDir, {
root: { dependencies: { "no-deps": "1.0.0", "one-dep": "1.0.0", "strict-peer-dep": "1.0.0" } },
packages: {
"no-deps": pkg("no-deps@1.0.0"),
"one-dep": oneDep,
"one-dep/no-deps": pkg("no-deps@1.0.1"),
"strict-peer-dep": strictPeerDep,
},
});

const { out } = await makeInstallRunner(packageDir)(["pm", "why", "no-deps"]);
expect(out).toMatchInlineSnapshot(`
"no-deps@1.0.0
├─ foo (requires 1.0.0)
└─ peer strict-peer-dep@1.0.0 (requires ^2.0.0)
└─ foo (requires 1.0.0)

no-deps@1.0.1
└─ one-dep@1.0.0 (requires 1.0.1)
└─ foo (requires 1.0.0)

"
`);
});

it("the hoisted linker installs the recorded copy next to the dependent", async () => {
const { packageDir } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true, linker: "hoisted" } });
const run = makeInstallRunner(packageDir);
await writeProject(packageDir, nestedCopy);

await run(["install", "--frozen-lockfile"]);
expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toMatchObject({
version: "1.0.1",
});
expect(
await file(join(packageDir, "node_modules", "strict-peer-dep", "node_modules", "no-deps", "package.json")).json(),
).toMatchObject({ version: "1.0.0" });
});

it("the isolated linker links the dependent against the recorded copy", async () => {
const { packageDir } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true, linker: "isolated" } });
const run = makeInstallRunner(packageDir);
await writeProject(packageDir, nestedCopy);

await run(["install", "--frozen-lockfile"]);
const bunDir = join(packageDir, "node_modules", ".bun");
const entries = (await readdirSorted(bunDir)).filter(entry => entry.startsWith("strict-peer-dep@"));
expect(entries).toHaveLength(1);
expect(await file(join(bunDir, entries[0], "node_modules", "no-deps", "package.json")).json()).toMatchObject({
version: "1.0.0",
});
});
});

it("adding a dependency keeps an optional peer on the package bun.lock bound it to while that package stays next to it", async () => {
const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: true } });
const run = makeInstallRunner(packageDir);
Expand Down
Loading
Loading