Skip to content

install: re-resolve a peer row instead of binding it to a stale bun.lock leftover #38901

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 7 commits into
base: main
Choose a base branch
from
+389 −113
Open

install: re-resolve a peer row instead of binding it to a stale bun.lock leftover #38901

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8598687
install: re-resolve a peer row instead of binding it to a stale lockf…
robobun Aug 15, 2026
b5522aa
install: limit the peer leftover rule to root and workspace entries, …
robobun Aug 15, 2026
be2e83a
install: shorten the peer binding comments
robobun Aug 15, 2026
8510010
install: one-line peer binding comments, leave the bun.lock loader co…
robobun Aug 15, 2026
b0aeaa6
install: a root entry's peer copy also keeps a workspace entry from r…
robobun Aug 15, 2026
f6c6fbd
install: rebind the passed-over peer copy on any manifest miss, not o…
robobun Aug 15, 2026
e2c732a
test: rewriting only the root's peer entry installs its copy for an u…
robobun Aug 15, 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
238 changes: 126 additions & 112 deletions src/install/PackageManager/PackageManagerEnqueue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2282,107 +2282,16 @@
success_fn: SuccessFn,
) -> crate::Result<Option<ResolvedPackageResult>> {
if install_peer && behavior.is_peer() {
if let Some(index) = this.lockfile.package_index.get(&name_hash) {
let resolutions = this.lockfile.packages.items_resolution();
match index {
PackageIndexEntry::Id(existing_id) => {
let existing_id = *existing_id;
if (existing_id as usize) < resolutions.len() {
let existing_resolution = resolutions[existing_id as usize];
if resolution_satisfies_dependency(this, &existing_resolution, version) {
success_fn(this, dependency_id, existing_id);
return Ok(Some(ResolvedPackageResult {
// we must fetch it from the packages array again, incase the package array mutates the value in the `successFn`
package: *this.lockfile.packages.get(existing_id as usize),
..Default::default()
}));
}

let res_tag = resolutions[existing_id as usize].tag;
let ver_tag = version.tag;
if (res_tag == ResolutionTag::Npm
&& ver_tag == dependency::version::Tag::Npm)
|| (res_tag == ResolutionTag::Git
&& ver_tag == dependency::version::Tag::Git)
|| (res_tag == ResolutionTag::Github
&& ver_tag == dependency::version::Tag::Github)
{
let existing_package = this.lockfile.packages.get(existing_id as usize);
this.log_mut().add_warning_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"incorrect peer dependency \"{}@{}\"",
existing_package
.name
.fmt(this.lockfile.buffers.string_bytes.as_slice()),
existing_package.resolution.fmt(
this.lockfile.buffers.string_bytes.as_slice(),
bun_fmt::PathSep::Auto
),
),
);
success_fn(this, dependency_id, existing_id);
return Ok(Some(ResolvedPackageResult {
// we must fetch it from the packages array again, incase the package array mutates the value in the `successFn`
package: *this.lockfile.packages.get(existing_id as usize),
..Default::default()
}));
}
}
}
PackageIndexEntry::Ids(list) => {
for &existing_id in list.iter() {
if (existing_id as usize) < resolutions.len() {
let existing_resolution = resolutions[existing_id as usize];
if resolution_satisfies_dependency(this, &existing_resolution, version)
{
success_fn(this, dependency_id, existing_id);
return Ok(Some(ResolvedPackageResult {
package: *this.lockfile.packages.get(existing_id as usize),
..Default::default()
}));
}
}
}

if (list[0] as usize) < resolutions.len() {
let res_tag = resolutions[list[0] as usize].tag;
let ver_tag = version.tag;
if (res_tag == ResolutionTag::Npm
&& ver_tag == dependency::version::Tag::Npm)
|| (res_tag == ResolutionTag::Git
&& ver_tag == dependency::version::Tag::Git)
|| (res_tag == ResolutionTag::Github
&& ver_tag == dependency::version::Tag::Github)
{
let existing_package_id = list[0];
let existing_package =
this.lockfile.packages.get(existing_package_id as usize);
this.log_mut().add_warning_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"incorrect peer dependency \"{}@{}\"",
existing_package
.name
.fmt(this.lockfile.buffers.string_bytes.as_slice()),
existing_package.resolution.fmt(
this.lockfile.buffers.string_bytes.as_slice(),
bun_fmt::PathSep::Auto
),
),
);
success_fn(this, dependency_id, list[0]);
return Ok(Some(ResolvedPackageResult {
// we must fetch it from the packages array again, incase the package array mutates the value in the `successFn`
package: *this.lockfile.packages.get(existing_package_id as usize),
..Default::default()
}));
}
}
}
}
if let Some((existing_id, satisfied)) =
existing_peer_target(this, name_hash, version, dependency_id)
{
return Ok(Some(bind_existing_peer(
this,
dependency_id,
existing_id,
satisfied,
success_fn,
)));
}
}

Expand Down Expand Up @@ -2549,13 +2458,28 @@

break 'blk Some(result);
}
Npm::FindVersionResult::Err(err_type) => match err_type {
Npm::FindVersionError::TooRecent
| Npm::FindVersionError::AllVersionsTooRecent => {
return Err(crate::Error::TooRecentVersion);
Npm::FindVersionResult::Err(err_type) => {
// The leftover `existing_peer_target` passed over is all there is.
if install_peer && behavior.is_peer() {
if let Some(id) = highest_peer_candidate(&this.lockfile, name_hash, version)
{
return Ok(Some(bind_existing_peer(
this,
dependency_id,
id,
false,
success_fn,
)));
}
}
Npm::FindVersionError::NotFound => None, // Handle below with existing logic
},
match err_type {
Npm::FindVersionError::TooRecent
| Npm::FindVersionError::AllVersionsTooRecent => {
return Err(crate::Error::TooRecentVersion);
}
Npm::FindVersionError::NotFound => None, // Handle below with existing logic
}
}
};

let find_result = match find_result_opt {
Expand Down Expand Up @@ -2906,14 +2830,104 @@
.map(|locked| (locked, buf))
}

fn resolution_satisfies_dependency(
/// The package to bind a deferred peer row to and whether it satisfies the row; the highest-or-nothing fallback is what `resolve_peer_dep_version_based` rebinds to on load.
fn existing_peer_target(
this: &PackageManager,
resolution: &Resolution,
dependency: &dependency::Version,
name_hash: PackageNameHash,
version: &dependency::Version,
row: DependencyID,
) -> Option<(PackageID, bool)> {
let lockfile: &Lockfile::Lockfile = &this.lockfile;
let candidates = lockfile.package_index.get(&name_hash)?.as_slice();
let pkg_res = lockfile.packages.items_resolution();
let buf = lockfile.buffers.string_bytes.as_slice();
if let Some(&id) = candidates.iter().find(|&&id| {
(id as usize) < pkg_res.len()
&& pkg_res[id as usize].satisfies_dependency_version(version, buf, buf)
}) {
return Some((id, true));
}
let highest = highest_peer_candidate(lockfile, name_hash, version)?;
(!would_revive_leftover(lockfile, row, highest)).then_some((highest, false))
}

/// `package_index` lists the highest version first.
fn highest_peer_candidate(
lockfile: &Lockfile::Lockfile,
name_hash: PackageNameHash,
version: &dependency::Version,
) -> Option<PackageID> {
let &highest = lockfile.package_index.get(&name_hash)?.as_slice().first()?;
let resolution = lockfile.packages.items_resolution().get(highest as usize)?;
let same_kind = matches!(
(resolution.tag, version.tag),
(ResolutionTag::Npm, dependency::version::Tag::Npm)
| (ResolutionTag::Git, dependency::version::Tag::Git)
| (ResolutionTag::Github, dependency::version::Tag::Github)
);
same_kind.then_some(highest)
}

fn bind_existing_peer(
this: &mut PackageManager,
dependency_id: DependencyID,
existing_id: PackageID,
satisfied: bool,
success_fn: SuccessFn,
) -> ResolvedPackageResult {
if !satisfied {
let existing_package = this.lockfile.packages.get(existing_id as usize);
this.log_mut().add_warning_fmt(
None,
bun_ast::Loc::EMPTY,
format_args!(
"incorrect peer dependency \"{}@{}\"",
existing_package
.name
.fmt(this.lockfile.buffers.string_bytes.as_slice()),
existing_package.resolution.fmt(
this.lockfile.buffers.string_bytes.as_slice(),
bun_fmt::PathSep::Auto
),
),
);
}
success_fn(this, dependency_id, existing_id);
ResolvedPackageResult {
// we must fetch it from the packages array again, incase the package array mutates the value in the `successFn`
package: *this.lockfile.packages.get(existing_id as usize),
..Default::default()
}
}

/// `row` is a root or workspace `peerDependencies` entry and `package_id` is held only by non-root peer rows (usually this entry's own earlier install): nothing provides it, and it is not a root row's copy, which `Tree::hoist_dependency` dedupes every other peer onto regardless of range.
fn would_revive_leftover(
lockfile: &Lockfile::Lockfile,
row: DependencyID,
package_id: PackageID,
) -> bool {
let buf = this.lockfile.buffers.string_bytes.as_slice();
resolution.satisfies_dependency_version(dependency, buf, buf)
if package_id >= lockfile.loaded_package_count || !lockfile.is_workspace_dependency(row) {
return false;
}
let deps = lockfile.buffers.dependencies.as_slice();
let resolutions = lockfile.buffers.resolutions.as_slice();
let mut owned_rows = lockfile
.packages
.items_dependencies()
.iter()
.zip(lockfile.packages.items_resolutions())
.enumerate()
.flat_map(|(owner, (dep_slice, res_slice))| {
dep_slice
.get(deps)
.iter()
.zip(res_slice.get(resolutions))
.map(move |(dep, &resolved)| (owner, dep, resolved))
});
!owned_rows.any(|(owner, dep, resolved)| {
resolved == package_id && (owner == 0 || !dep.behavior.is_peer())
})
}

Check warning on line 2930 in src/install/PackageManager/PackageManagerEnqueue.rs

View check run for this annotation

Claude / Claude Code Review

Root-only peer edit silently violates an unchanged member's peer constraint on the first install 

The inverse of the tested "member edited, root unchanged" case — root peer `^1.0.0`→`^2.0.0`, member A stays `^1.0.0` — is silent on the install that introduces the conflict: root's row falls through (A's row is `owner != 0 && is_peer()` so `would_revive_leftover`'s scan skips it) and resolves 2.0.0, A satisfies-scan-binds the leftover 1.1.0 with `satisfied=true`, then `Tree::hoist_dependency` dedupes A onto root's 2.0.0 at the pre-existing `// TODO: warning about peer dependency version mismatc
Comment thread
claude[bot] marked this conversation as resolved.

fn patched_package_satisfying(
this: &PackageManager,
Expand Down
Loading
Loading