Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
22 changes: 3 additions & 19 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3336,13 +3336,9 @@
/// `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,7 +3404,7 @@
}

let candidates = package_index.get(&name_hash)?.as_slice();
for &id in candidates {

Check failure on line 3407 in src/install/lockfile/bun.lock.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

manual implementation of `Iterator::find`
if (id as usize) < pkg_resolutions.len()
&& pkg_resolutions[id as usize]
.satisfies_dependency_version(range, string_buf, string_buf)
Expand All @@ -3417,18 +3413,6 @@
}
}

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
}

Expand Down
153 changes: 153 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,159 @@ 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.
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[]>;
kept: string[];
absent?: string[];
};

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"),
},
kept: ['"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,
},
kept: ['"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"),
},
kept: ['"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"),
},
kept: ['"no-deps": ["no-deps@1.0.0"', '"one-dep/no-deps": ["no-deps@1.0.1"'],
},
],
];

async function writeProject(packageDir: string, shape: Shape) {
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,
}),
);
}

it.each(shapes)("re-saving keeps %s", async (_, 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.kept) {
expect(saved).toContain(entry);
}
for (const entry of shape.absent ?? []) {
expect(saved).not.toContain(entry);
}

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

const [, nestedCopy] = shapes[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
83 changes: 83 additions & 0 deletions test/cli/install/migration/pnpm-lock-v9.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ const PEER_DEPS_TOO_1_0_0_INTEGRITY =
"sha512-sBx0TKrsB8FkRN2lzkDjMuctPGEKn1TmNUBv3dJOtnZM8nd255o5ZAPRpAI2XFLHZAavBlK/e73cZNwnUxlRog==";
const ONE_OPTIONAL_PEER_DEP_1_0_2_INTEGRITY =
"sha512-S25U8/QXGIKfn/AWtsce1aVMnDjDL+ykFtAufpsuKGad32NlsCpi9TDuXvzoTQ+MdaZpGV3c4xghUZUsNeMp4A==";
const STRICT_PEER_DEP_1_0_0_INTEGRITY =
"sha512-bz2RC/Fp4Nvc9aIiHB6Szko9m6sxNy/clIHnTAGeD9VSpQJTvlPAJqJ09lWo7N3q4JNLEqDTf3Mn+zNUsYOKWQ==";
const LOCAL_TARBALL_INTEGRITY =
"sha512-HP/5Rgt3pVFLzjmN9qJJ6vZMgCwoCIl/m2bPndYT283CUqnmFiMx0GeeIJ7SyK6TYoJM78SEvFEOQie++caHqw==";

Expand Down Expand Up @@ -1471,6 +1473,87 @@ snapshots:
`);
});

test("a peer nothing in the lockfile satisfies keeps the version pnpm resolved it to", async () => {
// strict-peer-dep wants no-deps@^2.0.0; pnpm resolved it to 1.0.0 (the snapshot suffix) while
// one-dep brings in 1.0.1. Neither satisfies the range, so there is nothing to rebind the
// peer by; binding it to the highest version present would drop pnpm's 1.0.0 from the tree.
const { packageDir } = await verdaccio.createTestDir({
bunfigOpts: { linker: "hoisted" },
files: {
"package.json": JSON.stringify({
name: "unsatisfied-peer",
dependencies: { "one-dep": "1.0.0", "strict-peer-dep": "1.0.0" },
}),
"pnpm-lock.yaml": `lockfileVersion: '9.0'

settings:
autoInstallPeers: true

importers:

.:
dependencies:
one-dep:
specifier: 1.0.0
version: 1.0.0
strict-peer-dep:
specifier: 1.0.0
version: 1.0.0(no-deps@1.0.0)

packages:

no-deps@1.0.0:
resolution: {integrity: ${NO_DEPS_1_0_0_INTEGRITY}}

no-deps@1.0.1:
resolution: {integrity: ${NO_DEPS_1_0_1_INTEGRITY}}

one-dep@1.0.0:
resolution: {integrity: ${ONE_DEP_1_0_0_INTEGRITY}}

strict-peer-dep@1.0.0:
resolution: {integrity: ${STRICT_PEER_DEP_1_0_0_INTEGRITY}}
peerDependencies:
no-deps: ^2.0.0

snapshots:

no-deps@1.0.0: {}

no-deps@1.0.1: {}

one-dep@1.0.0:
dependencies:
no-deps: 1.0.1

strict-peer-dep@1.0.0(no-deps@1.0.0):
dependencies:
no-deps: 1.0.0
`,
},
});

const { stderr, exitCode } = await migrate(packageDir);

expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml");
expect(exitCode).toBe(0);

const bunLock = await bunLockOf(packageDir);
expect(bunLock).toContain(`"no-deps": ["no-deps@1.0.1"`);
expect(bunLock).toContain(`"strict-peer-dep/no-deps": ["no-deps@1.0.0"`);

const install = await run(packageDir, "install", "--frozen-lockfile");

expect(install.stderr).not.toContain("error:");
expect(install.exitCode).toBe(0);
expect(nodeModulesPackages(packageDir)).toMatchInlineSnapshot(`
"node_modules/no-deps/no-deps@1.0.1
node_modules/one-dep/one-dep@1.0.0
node_modules/strict-peer-dep/node_modules/no-deps/no-deps@1.0.0
node_modules/strict-peer-dep/strict-peer-dep@1.0.0"
`);
});

// pnpm11/lockfile/fs convertToLockfileObject: every variant joins packages[removeSuffix(key)]
const peerVariantPackageJsons = {
"package.json": JSON.stringify({ name: "v9-peer-variants", workspaces: ["apps/*"] }),
Expand Down
Loading