Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
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
50 changes: 43 additions & 7 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,18 @@ impl<T> PkgMap<T> {
self.map.contains_key(path)
}

/// The entry printed inside `pkg_path`'s own `node_modules`: the first probe of `find_resolution`.
fn get_nested(&self, pkg_path: &[u8], dep_name: &[u8], path_buf: &mut [u8]) -> Option<&T> {
let len = pkg_path.len() + 1 + dep_name.len();
if len > path_buf.len() {
return None;
}
path_buf[..pkg_path.len()].copy_from_slice(pkg_path);
path_buf[pkg_path.len()] = b'/';
path_buf[pkg_path.len() + 1..len].copy_from_slice(dep_name);
self.get(&path_buf[..len])
}

fn find_resolution(
&self,
pkg_path: &[u8],
Expand Down Expand Up @@ -3088,13 +3100,15 @@ pub(crate) fn parse_into_binary_lockfile(
let dep_id: DependencyID = _dep_id;
let dep = &mut dependencies[dep_id as usize];

// Every entry at the root level is a hoisted one, so none of them is a recorded peer binding.
let peer_res_id = resolve_peer_dep_version_based(
dep,
catalogs,
package_index,
overrides,
pkg_resolutions,
string_buf,
|| None,
);
let Some(res_id) =
peer_res_id.or_else(|| pkg_map.get(dep.name.slice(string_buf)).copied())
Expand Down Expand Up @@ -3177,6 +3191,7 @@ pub(crate) fn parse_into_binary_lockfile(
overrides,
pkg_resolutions,
string_buf,
|| pkg_map.get(workspace_node_modules).copied(),
);
let Some(res_id) = peer_res_id.or_else(|| {
pkg_map
Expand Down Expand Up @@ -3241,13 +3256,19 @@ pub(crate) fn parse_into_binary_lockfile(
continue 'deps;
}

let dep_name = dep.name.slice(string_buf);
let peer_res_id = resolve_peer_dep_version_based(
dep,
catalogs,
package_index,
overrides,
pkg_resolutions,
string_buf,
|| {
pkg_map
.get_nested(pkg_path, dep_name, &mut path_buf[..])
.copied()
},
);
let res_id = match peer_res_id {
Some(id) => id,
Expand Down Expand Up @@ -3336,13 +3357,23 @@ 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. When nothing satisfies, the binding is `nested()`, the entry
/// the printed tree has inside the dependent's own `node_modules`, if any:
/// the hoister nests a peer under its dependent exactly when nothing
/// enclosing satisfies it, so that entry is the binding the file records
/// (1.3.x bound every peer by path and wrote many such entries), and it is
/// also what node resolves from the dependent; binding anything else would
/// leave it without a dependent and drop it from the lockfile on the next
/// save. A tree written by this version has the fallback candidate there,
/// nothing, or a copy a dependency of the dependent could not hoist any
/// further, which node resolves from the dependent as well. Without such
/// an entry, 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`.
///
/// 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 All @@ -3367,6 +3398,7 @@ pub(crate) fn resolve_peer_dep_version_based(
overrides: &OverrideMap,
pkg_resolutions: &[Resolution],
string_buf: &[u8],
nested: impl FnOnce() -> Option<PackageID>,
) -> Option<PackageID> {
let range = deferred_peer_range(dep, catalogs, string_buf)?;
// `package_index` is keyed by real package names; `range` (not `dep.name`) carries them for aliases.
Expand Down Expand Up @@ -3417,6 +3449,10 @@ pub(crate) fn resolve_peer_dep_version_based(
}
}

if let Some(id) = nested() {
return Some(id);
}

let &first = candidates.first()?;
if (first as usize) < pkg_resolutions.len() {
let res_tag = pkg_resolutions[first as usize].tag;
Expand Down
2 changes: 2 additions & 0 deletions src/install/pnpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ fn resolve_peer_like_bun_lock(lockfile: &Lockfile, dep: &Dependency) -> Option<P
&lockfile.overrides,
lockfile.packages.items_resolution(),
string_bytes!(lockfile),
// No printed tree yet; whatever this binds is what the first save nests.
|| None,
)
}

Expand Down
50 changes: 50 additions & 0 deletions test/cli/install/bun-lock.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,56 @@ it("--frozen-lockfile keeps a package that an older lockfile lists only as an op
expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBeTrue();
});

// bun 1.3.x bound every peer to the entry the tree placed next to it, so a
// bun.lock it wrote can nest a copy under a dependent whose peer range no
// version in the lockfile satisfies. Loading has to bind the peer to that copy,
// as 1.3.x did: bound to the hoisted version instead, the copy is not installed
// and has no dependent left, so the next save drops it.
it("installs and keeps the copy an older lockfile nests under a dependent whose peer range nothing satisfies", async () => {
const { packageDir, packageJson } = await registry.createTestDir({
bunfigOpts: { saveTextLockfile: true, linker: "hoisted" },
});
const run = makeInstallRunner(packageDir);
const dependencies = { "one-dep": "1.0.0", "strict-peer-dep": "1.0.0" };
const nestedEntry = '"strict-peer-dep/no-deps": ["no-deps@1.0.0"';
await Promise.all([
write(packageJson, JSON.stringify({ name: "foo", dependencies })),
write(
join(packageDir, "bun.lock"),
JSON.stringify({
lockfileVersion: 1,
configVersion: 1,
workspaces: { "": { name: "foo", dependencies } },
packages: {
"no-deps": ["no-deps@1.0.1", "", {}, ""],
"one-dep": ["one-dep@1.0.0", "", { dependencies: { "no-deps": "1.0.1" } }, ""],
// strict-peer-dep wants no-deps@^2.0.0; neither copy satisfies it, and
// this nested one is the binding an earlier install recorded.
"strict-peer-dep": ["strict-peer-dep@1.0.0", "", { peerDependencies: { "no-deps": "^2.0.0" } }, ""],
"strict-peer-dep/no-deps": ["no-deps@1.0.0", "", {}, ""],
},
}),
),
]);

await run(["install", "--frozen-lockfile"]);
const installedVersion = (...segments: string[]) =>
file(join(packageDir, "node_modules", ...segments, "package.json"))
.json()
.then(({ version }) => version)
.catch(() => null);
expect(
await Promise.all([installedVersion("no-deps"), installedVersion("strict-peer-dep", "node_modules", "no-deps")]),
).toEqual(["1.0.1", "1.0.0"]);

await run(["install", "--lockfile-only"]);
const saved = await file(join(packageDir, "bun.lock")).text();
expect(saved).toContain(nestedEntry);

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

// The optional-peer-hoist-* fixtures are described in
// registry/packages/create-optional-peer-hoist-packages.ts. In short: consumer
// has an optional peer on target, and deep -> deep-child reaches target@1.0.0
Expand Down
Loading