Skip to content

install: don't mark a placed peer optional when saving a migrated bun.lock #37289

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

Closed
robobun wants to merge 5 commits into from
Closed

install: don't mark a placed peer optional when saving a migrated bun.lock #37289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
72795f0
install: don't mark a placed peer optional when saving a migrated bun…
robobun Aug 9, 2026
f21d09d
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 9, 2026
b244db3
shorten comments flagged by review
robobun Aug 9, 2026
26ccf90
compress comment to two lines
robobun Aug 9, 2026
60cb27f
reduce comment to one line
robobun Aug 9, 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
49 changes: 23 additions & 26 deletions src/install/lockfile/bun.lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ impl Stringifier {

let mut tree_deps_sort_buf: Vec<DependencyID> = Vec::new();
let mut pkg_deps_sort_buf: Vec<DependencyID> = Vec::new();
let mut entry_path_buf: Vec<u8> = Vec::new();

Self::write_indent(writer, *indent)?;
writer.write_all(b"\"packages\": {")?;
Expand Down Expand Up @@ -707,29 +708,23 @@ impl Stringifier {
Self::write_indent(writer, *indent)?;
}

writer.write_byte(b'"')?;
// relative_path is empty string for root resolutions
write!(
writer,
"{}",
bun_core::fmt::format_json_string_utf8(
relative_path,
bun_core::fmt::JSONFormatterUTF8Options { quote: false }
),
)?;
let dep = &deps_buf[dep_id as usize];
let dep_name = dep.name.slice(buf);

// The entry key doubles as the tree path this entry's dependencies resolve from.
entry_path_buf.clear();
entry_path_buf.extend_from_slice(relative_path);
if *depth != 0 {
writer.write_byte(b'/')?;
entry_path_buf.push(b'/');
}

let dep = &deps_buf[dep_id as usize];
let dep_name = dep.name.slice(buf);
entry_path_buf.extend_from_slice(dep_name);
let entry_path: &[u8] = entry_path_buf.as_slice();

write!(
writer,
"{}\": ",
"\"{}\": ",
bun_core::fmt::format_json_string_utf8(
dep_name,
entry_path,
bun_core::fmt::JSONFormatterUTF8Options { quote: false }
),
)?;
Expand Down Expand Up @@ -826,7 +821,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand All @@ -851,7 +846,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand Down Expand Up @@ -881,7 +876,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand Down Expand Up @@ -910,7 +905,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand Down Expand Up @@ -957,7 +952,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand Down Expand Up @@ -1005,7 +1000,7 @@ impl Stringifier {
&mut optional_peers_buf,
extern_strings,
&pkg_map,
relative_path,
entry_path,
&mut path_buf,
)?;

Expand Down Expand Up @@ -1054,7 +1049,8 @@ impl Stringifier {
optional_peers_buf: &mut Vec<String>,
extern_strings: &[ExternalString],
pkg_map: &PkgMap<()>,
relative_path: &[u8],
// The package's own tree path (its `packages` key).
pkg_path: &[u8],
path_buf: &mut [u8],
) -> Result<(), WriteError> {
// `optional_peers_buf` is cleared at the fn tail.
Expand Down Expand Up @@ -1109,7 +1105,7 @@ impl Stringifier {
&& pkg_map.map.len() > 0
{
if pkg_map
.find_resolution(relative_path, dep, buf, path_buf)
.find_resolution(pkg_path, dep, buf, path_buf)
.is_err()
{
optional_peers_buf.push(dep.name);
Expand Down Expand Up @@ -1230,7 +1226,8 @@ impl Stringifier {
workspace_versions: &VersionHashMap,
optional_peers_buf: &mut Vec<String>,
pkg_map: &PkgMap<()>,
relative_path: &[u8],
// The workspace package's tree path ("" for the root package).
pkg_path: &[u8],
path_buf: &mut [u8],
) -> Result<(), WriteError> {
// `optional_peers_buf` is cleared at the fn tail.
Expand Down Expand Up @@ -1351,7 +1348,7 @@ impl Stringifier {
&& !dep.behavior.contains(Behavior::OPTIONAL)
&& pkg_map.map.len() > 0
{
if let Err(err) = pkg_map.find_resolution(relative_path, dep, buf, path_buf) {
if let Err(err) = pkg_map.find_resolution(pkg_path, dep, buf, path_buf) {
if err == ResolveError::Unresolvable {
optional_peers_buf.push(dep.name);
}
Expand Down
69 changes: 69 additions & 0 deletions test/cli/install/migration/migrate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,75 @@ test.concurrent.each([
},
);

test.concurrent(
"migrated bun.lock with a nested-placed peer of a file: dependency round-trips --frozen-lockfile",
async () => {
// delta is a regular peer of gamma with only a nested placement ("gamma/delta").
// The migration writer must not also list it in "optionalPeers", or reloading
// drops the entry and --frozen-lockfile fails on an unchanged tree.
await using testDir = tempDir("migrate-nested-peer-fixed-point", {
"package.json": JSON.stringify({
name: "sandbox",
version: "1.0.0",
workspaces: ["packages/ws-a"],
dependencies: { gamma: "file:vendor/gamma" },
}),
"vendor/gamma/package.json": JSON.stringify({
name: "gamma",
version: "1.0.0",
peerDependencies: { delta: "*" },
}),
"vendor/delta/package.json": JSON.stringify({ name: "delta", version: "2.0.0" }),
"packages/ws-a/package.json": JSON.stringify({
name: "ws-a",
version: "0.1.0",
dependencies: { delta: "file:../../vendor/delta" },
}),
// What `npm install --package-lock-only` produces for this tree.
"package-lock.json": JSON.stringify({
name: "sandbox",
version: "1.0.0",
lockfileVersion: 3,
requires: true,
packages: {
"": {
name: "sandbox",
version: "1.0.0",
dependencies: { gamma: "file:vendor/gamma" },
workspaces: ["packages/ws-a"],
},
"node_modules/gamma": { resolved: "vendor/gamma", link: true },
"vendor/gamma": { name: "gamma", version: "1.0.0", peerDependencies: { delta: "*" } },
"node_modules/delta": { resolved: "vendor/delta", link: true },
"vendor/delta": { name: "delta", version: "2.0.0" },
"node_modules/ws-a": { resolved: "packages/ws-a", link: true },
"packages/ws-a": { name: "ws-a", version: "0.1.0", dependencies: { delta: "file:../../vendor/delta" } },
},
}),
});

const first = await install(testDir);
expect(first.stderr).toContain("migrated lockfile from package-lock.json");
expect(first.exitCode).toBe(0);

const lock = await Bun.file(join(testDir, "bun.lock")).text();
// The peer's resolution is recorded right below in the same lockfile, so the
// peer is not optional.
expect(lock).toContain('"gamma/delta"');
expect(lock).not.toContain("optionalPeers");

// An unchanged tree satisfies --frozen-lockfile...
const frozen = await install(testDir, "--frozen-lockfile");
expect(frozen.stderr).not.toContain("lockfile had changes");
expect(frozen.exitCode).toBe(0);

// ...and a plain re-install does not rewrite the lockfile.
const second = await install(testDir);
expect(second.exitCode).toBe(0);
expect(await Bun.file(join(testDir, "bun.lock")).text()).toBe(lock);
},
);

test.concurrent("package-lock.json migration does not platform-skip a regular file: tarball dependency", async () => {
// Same divergence as the folder variant, for a `LocalTarball` resolution. npm records
// the packed package's `os`/`cpu` arrays in its lockfile entry, and a fresh resolve of
Expand Down