fix(pacquet): match pnpm peer-suffix rendering for walk-ancestor peers (#12267)

* fix(deps-resolver): carry full peer suffix for walk-ancestor peers

When a node's resolved peer is a walk-ancestor still in progress (e.g.
`@eslint-community/eslint-utils` peer-depends on its parent `eslint`),
`build_peer_id` had no depPath for it yet and fell back to the
`name@version` cycle form, freezing a collapsed suffix like
`eslint-utils@4.9.1(eslint@10.4.1)` instead of pnpm's
`eslint-utils@4.9.1(eslint@10.4.1(supports-color@8.1.1))`. Because the
depPath is the graph key, the truncation cascaded to every reference.

Port pnpm's deferred `calculateDepPath`: leave the walk (and the
provisional depPaths `find_hit` reads) untouched, capture each node's
NodeId-level edges, then recompute depPaths in a post-walk pass that
resolves each peer to its full depPath and collapses to `name@version`
only for genuine cycles — detected as peer-graph SCCs via iterative
Tarjan. The graph is rebuilt from the per-node records keyed by the
corrected depPaths, so wrongly-collapsed nodes split and correctly-
collapsed ones still merge.

Also fixes a related divergence: snapshot `dependencies` now carry only
the node's own resolved peers, mirroring pnpm's
`childrenNodeIds: { ...children, ...resolvedPeers }`, so a descendant's
optional peer (e.g. `debug`'s `supports-color`) is no longer materialized
as the parent's dependency.

Refs pnpm/pnpm#12266.

* fix(deps-resolver): don't hoist optional peers against run-resolved versions

`getHoistableOptionalPeers` hoists an optional missing peer only when a
preferred version is already in scope. pnpm reads its static
`ctx.allPreferredVersions` (wanted lockfile + manifests, empty on a fresh
install) for that decision and never folds in versions resolved during
the run. pacquet was feeding every freshly-resolved transitive version
into the same map, so an optional peer like `debug`'s `supports-color`
got hoisted against a deep-tree provider (e.g. `jest-worker`'s
`supports-color`) that pnpm never considers — resolving the peer where
pnpm leaves it bare, and non-deterministically (the outcome depended on
whether the provider landed in the map before the optional pass ran).

Snapshot the preferred-versions map before any run-resolved version is
folded in and use that snapshot for the optional-peer hoist. The
required-peer hoist keeps using the run-extended map: a required peer is
auto-installed either way, and reusing an in-tree version matches pnpm's
picker dedup (covered by the existing auto_install_* tests).

New regression test `optional_peer_only_in_resolved_tree_is_not_hoisted`,
verified to fail without the fix.

Refs pnpm/pnpm#12266.

* fix(deps-resolver): restore min-depth tie-break in build_final_graph

`build_final_graph` keyed a graph node's `depth` off the per-NodeId
`NodeRecord`, but the `pure_pkgs` and `find_hit` fast paths short-circuit
before a `NodeRecord` is created — they only lower the depth on the inline
`self.graph`, which the rebuild discards. So a package first walked at a
deeper depth and later revisited shallower kept the deeper depth, dropping
the `Math.min` tie-break those fast paths' own comments preserve.

Recompute the minimum tree depth per final depPath from `node_dep_paths`
(which records every walked NodeId, revisits included) and use it for each
rebuilt node. No lockfile-visible change today — `DependenciesGraphNode.depth`
isn't serialized and the hoister computes its own BFS depth — but it keeps
the graph's depth correct for any future consumer. Regression introduced by
the deferred depPath rebuild; guarded by a new test.

Refs pnpm/pnpm#12266.

* test(deps-resolver): port resolvePeers "peer's own peer shared with sibling"

Ports upstream's `resolvePeers.ts` "a peer's own peer is shared with a
sibling that peer-depends both" — `plugin`'s `parser` peer must resolve the
`typescript@1.0.0` that `plugin` itself uses, not be shadowed by a top-level
`parser` that resolved `typescript@2.0.0`. Exercises the depPath suffix
machinery reworked for the peer-suffix fixes; pacquet already passes it.

Also records the peer-resolution porting status (resolvePeers.ts +
hoistPeers.test.ts) in plans/TEST_PORTING.md: what's ported/covered and the
remaining gaps (multi-project peer separation, npm-alias peers, the
peer-conflict reporting edge cases, and the unported lockedPeerContext
series).

Refs pnpm/pnpm#12266.
This commit is contained in:
Zoltan Kochan
2026-06-08 23:10:06 +02:00
committed by GitHub
parent e4d2fe025e
commit bafcfbaefc
5 changed files with 736 additions and 17 deletions

View File

@@ -78,11 +78,17 @@ pub struct ResolveImporterOptions {
/// Seed for the preferred-versions tie-break table. The
/// orchestrator extends this in place as packages are walked —
/// each newly-resolved `name@version` lands as a plain
/// [`VersionSelectorType::Version`] entry so [`hoist_peers`] and
/// [`get_hoistable_optional_peers`] can reuse it. Pass the result
/// of `get_preferred_versions_from_lockfile_and_manifests` from
/// the `lockfile-preferred-versions` crate, or an empty map when
/// no lockfile + manifest seeding is available.
/// [`VersionSelectorType::Version`] entry so the [`hoist_peers`]
/// (required-peer) picker can reuse a version a sibling already
/// brought. The [`get_hoistable_optional_peers`] picker instead
/// reads a snapshot taken *before* any run-resolved version is
/// folded in — mirroring upstream's static
/// [`ctx.allPreferredVersions`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolveDependencies.ts#L340-L342)
/// — so an optional peer is never hoisted against a deep-tree
/// provider pnpm can't see. Pass the result of
/// `get_preferred_versions_from_lockfile_and_manifests` from the
/// `lockfile-preferred-versions` crate, or an empty map when no
/// lockfile + manifest seeding is available.
pub all_preferred_versions: PreferredVersions,
/// Configured `patchedDependencies`, grouped by package name. The
@@ -292,6 +298,17 @@ where
let initial_wanted =
importer_direct_wanted_specs(manifest, dependency_groups, auto_install_peers, &catalogs)?;
let mut direct = extend_tree(&ctx, resolver, initial_wanted, importer_id).await?;
// The optional-peer hoist must only consider versions that were
// already in scope before this run — the wanted lockfile + manifests
// — mirroring upstream's static
// [`ctx.allPreferredVersions`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolveDependencies.ts#L340-L342).
// Feeding freshly-resolved transitive versions into that decision
// would hoist an optional peer (e.g. `debug`'s `supports-color`)
// against a deep-tree provider pnpm never sees, resolving the peer
// where pnpm leaves it bare. The required-peer hoist keeps using the
// run-extended map below: a required peer is auto-installed either
// way, and reusing an in-tree version matches pnpm's picker dedup.
let optional_hoist_preferred_versions = all_preferred_versions.clone();
update_preferred_versions_with_ctx(&ctx, &mut all_preferred_versions);
let mut parent_pkg_aliases: HashSet<String> =
@@ -365,8 +382,10 @@ where
if all_missing_optional_peers.is_empty() {
break;
}
let hoisted_optional =
get_hoistable_optional_peers(&all_missing_optional_peers, &all_preferred_versions);
let hoisted_optional = get_hoistable_optional_peers(
&all_missing_optional_peers,
&optional_hoist_preferred_versions,
);
if hoisted_optional.is_empty() {
break;
}

View File

@@ -715,6 +715,71 @@ async fn auto_install_does_not_hoist_when_root_already_has_dep() {
);
}
/// An *optional* peer that is only available deep in the freshly
/// resolved tree (brought by a sibling, not pinned in the wanted
/// lockfile) must NOT be hoisted, so the consumer's depPath stays bare.
/// `getHoistableOptionalPeers` reads upstream's static
/// `ctx.allPreferredVersions` (lockfile + manifests, empty on a fresh
/// install) — never the run-resolved versions. Feeding the latter in
/// would resolve the optional peer where pnpm leaves it unresolved, and
/// non-deterministically at that (the result would depend on resolution
/// order). Regression test for <https://github.com/pnpm/pnpm/issues/12266>.
#[tokio::test]
async fn optional_peer_only_in_resolved_tree_is_not_hoisted() {
let mut table = HashMap::new();
table.insert(
("needs-opt".to_string(), "1.0.0".to_string()),
fake_result(
"needs-opt",
"1.0.0",
serde_json::json!({
"name": "needs-opt",
"version": "1.0.0",
"peerDependencies": { "opt": "^1.0.0" },
"peerDependenciesMeta": { "opt": { "optional": true } },
}),
),
);
// `provider` pulls `opt@1.0.0` deep in the tree — a sibling of
// `needs-opt`, not an ancestor, so `opt` is out of `needs-opt`'s
// resolution scope.
table.insert(
("provider".to_string(), "1.0.0".to_string()),
fake_result(
"provider",
"1.0.0",
serde_json::json!({
"name": "provider",
"version": "1.0.0",
"dependencies": { "opt": "1.0.0" },
}),
),
);
table.insert(
("opt".to_string(), "1.0.0".to_string()),
fake_result("opt", "1.0.0", serde_json::json!({ "name": "opt", "version": "1.0.0" })),
);
let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) };
let (_tmp, manifest) = fake_manifest(serde_json::json!({
"needs-opt": "1.0.0",
"provider": "1.0.0",
}));
let result = resolve_importer(&resolver, &manifest, [DependencyGroup::Prod], default_opts())
.await
.unwrap();
// `opt` must not be hoisted to the importer, and `needs-opt`'s
// optional peer must stay unresolved — bare `needs-opt@1.0.0`.
let direct: Vec<&str> =
result.peers_result.direct_dependencies_by_alias.keys().map(String::as_str).collect();
assert!(!direct.contains(&"opt"), "optional peer `opt` must not be hoisted: {direct:?}");
assert_eq!(
result.peers_result.direct_dependencies_by_alias.get("needs-opt"),
Some(&DepPath::from("needs-opt@1.0.0".to_string())),
);
}
/// Port of "don't install the same missing peer dependency twice": a
/// transitive chain where each layer adds the same peer must produce a
/// single hoisted entry.

View File

@@ -207,6 +207,7 @@ pub fn resolve_peers(tree: &mut ResolvedTree, opts: ResolvePeersOptions) -> Reso
pure_pkgs: HashSet::new(),
peers_cache: HashMap::new(),
parent_pkgs_of_node: HashMap::new(),
node_records: HashMap::new(),
};
walker.walk()
}
@@ -242,6 +243,7 @@ pub fn resolve_peers_workspace(
pure_pkgs: HashSet::new(),
peers_cache: HashMap::new(),
parent_pkgs_of_node: HashMap::new(),
node_records: HashMap::new(),
};
let mut direct_dependencies_by_importer: BTreeMap<String, BTreeMap<String, DepPath>> =
@@ -257,20 +259,30 @@ pub fn resolve_peers_workspace(
walker.opts.modules_dir = importer.modules_dir.clone();
let importer_parents = walker.build_importer_parents_from(&importer.direct);
let parent_chain_names: Vec<String> = Vec::new();
let mut direct_by_alias = BTreeMap::new();
for dep in &importer.direct {
let output =
walker.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0);
direct_by_alias.insert(dep.alias.clone(), output.dep_path);
walker.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0);
}
direct_dependencies_by_importer.insert(importer.id.clone(), direct_by_alias);
let issues = std::mem::take(&mut walker.issues);
if !issues.bad.is_empty() || !issues.missing.is_empty() {
peer_dependency_issues_by_importer.insert(importer.id.clone(), issues);
}
}
walker.patch_pending_peer_edges();
let mut graph = walker.graph;
// Recompute depPaths with full peer suffixes once, after every
// importer is walked, then rebuild the graph and re-key each
// importer's direct deps.
let final_dep_paths = walker.build_final_dep_paths();
for importer in importers {
let direct_by_alias: BTreeMap<String, DepPath> = importer
.direct
.iter()
.map(|dep| {
(dep.alias.clone(), walker.final_dep_path_of(&dep.node_id, &final_dep_paths))
})
.collect();
direct_dependencies_by_importer.insert(importer.id.clone(), direct_by_alias);
}
let mut graph = walker.build_final_graph(&final_dep_paths);
if dedupe_injected_deps_enabled {
dedupe_injected_deps(
@@ -401,6 +413,10 @@ struct Walker<'tree> {
/// [`parentPkgsOfNode`](https://github.com/pnpm/pnpm/blob/c86c423bdc/installing/deps-resolver/src/resolvePeers.ts#L356)
/// + [`parentPackagesMatch`](https://github.com/pnpm/pnpm/blob/c86c423bdc/installing/deps-resolver/src/resolvePeers.ts#L701-L731).
parent_pkgs_of_node: HashMap<NodeId, HashMap<String, ParentPkgInfo>>,
/// Per-`NodeId` snapshot captured at graph-insert time, consumed by
/// the post-walk [`Walker::build_final_dep_paths`] /
/// [`Walker::build_final_graph`] pass. See [`NodeRecord`].
node_records: HashMap<NodeId, NodeRecord>,
}
/// Per-peer-name snapshot stored on [`Walker::parent_pkgs_of_node`].
@@ -443,6 +459,30 @@ struct PendingPeerEdge {
peer_node_id: NodeId,
}
/// Per-`NodeId` data captured during the walk so the post-walk
/// [`Walker::build_final_dep_paths`] pass can recompute each node's
/// depPath with its resolved peers' *full* suffixes — matching pnpm's
/// deferred [`calculateDepPath`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolvePeers.ts#L629),
/// which awaits each pending peer's depPath and collapses to
/// `name@version` only for genuinely detected cycles.
///
/// The walk itself is left untouched: it still computes the provisional
/// depPaths that [`Walker::find_hit`] reads, so peer-resolution and
/// cache decisions are byte-for-byte identical. Only the rendered
/// depPaths change, which is why a node whose suffix was previously
/// collapsed by the cycle fallback now splits into its own graph entry.
struct NodeRecord {
/// `alias → child/peer NodeId` edges, in the same shape the inline
/// `graph_children` map carries (children overlaid with resolved-peer
/// edges) but holding NodeIds, so the rebuild can map each edge to
/// its final depPath.
edges: BTreeMap<String, NodeId>,
transitive_peer_dependencies: HashSet<String>,
depth: i32,
installable: bool,
is_pure: bool,
}
/// Sentinel for "this node's subtree is still missing peer `X`". The
/// `range` + `optional` payload mirrors upstream's `MissingPeers` map
/// value; pacquet records them for upstream parity (and for the
@@ -479,13 +519,21 @@ impl<'tree> Walker<'tree> {
// `self.tree.direct`.
let direct: Vec<DirectDep> = self.tree.direct.clone();
for dep in &direct {
let output =
self.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0);
direct_by_alias.insert(dep.alias.clone(), output.dep_path);
self.resolve_node(dep.node_id.clone(), &importer_parents, &parent_chain_names, 0);
}
self.patch_pending_peer_edges();
// Recompute depPaths so each resolved peer carries its full
// suffix (the cycle fallback during the walk collapses peers
// that are walk-ancestors), then rebuild the graph from the
// per-node records keyed by the corrected depPaths.
let final_dep_paths = self.build_final_dep_paths();
for dep in &direct {
direct_by_alias
.insert(dep.alias.clone(), self.final_dep_path_of(&dep.node_id, &final_dep_paths));
}
let graph = self.build_final_graph(&final_dep_paths);
ResolvePeersResult {
graph: self.graph,
graph,
direct_dependencies_by_alias: direct_by_alias,
peer_dependency_issues: self.issues,
}
@@ -889,6 +937,32 @@ impl<'tree> Walker<'tree> {
let is_pure = all_resolved_peers.is_empty() && all_missing_peers.is_empty();
// Capture this node's NodeId-level edges + metadata for the
// post-walk [`Walker::build_final_dep_paths`] rebuild. Edges are
// the node's regular children overlaid with its *own* resolved
// peers — mirroring upstream's
// [`childrenNodeIds: { ...children, ...resolvedPeers }`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/src/resolvePeers.ts#L700-L705),
// where `resolvedPeers` is this node's own peer resolution, not
// the descendants' peers bubbled up for the suffix. A peer a
// descendant resolved (e.g. `debug`'s optional `supports-color`)
// is symlinked at the descendant that declares it, so it must
// not appear in this node's dependencies. Carries NodeIds so the
// rebuild can resolve each to its corrected final depPath.
let mut record_edges: BTreeMap<String, NodeId> = children_map.clone();
for (peer_alias, peer_node_id) in &own_resolved_peers {
record_edges.insert(peer_alias.clone(), peer_node_id.clone());
}
self.node_records.insert(
node_id.clone(),
NodeRecord {
edges: record_edges,
transitive_peer_dependencies: transitive_peer_dependencies.clone(),
depth: tree_node.depth,
installable: tree_node.installable,
is_pure,
},
);
// Record this walk's outcome in the per-`pkgIdWithPatchHash`
// caches. Pure subtrees go in [`Self::pure_pkgs`] for the
// fast-path early return at the top of [`resolve_node`];
@@ -1047,6 +1121,261 @@ impl<'tree> Walker<'tree> {
PeerId::Pair { name, version }
}
/// Resolve `node_id` to the depPath the rebuilt graph should key /
/// reference it by. Prefers the corrected `final_dep_paths` entry,
/// falls back to the provisional `node_dep_paths` value (correct for
/// peerless nodes, whose depPath is just their `pkgIdWithPatchHash`),
/// then to a bare `link:` id, then to the package id.
fn final_dep_path_of(
&self,
node_id: &NodeId,
final_dep_paths: &HashMap<NodeId, DepPath>,
) -> DepPath {
if let Some(dep_path) = final_dep_paths.get(node_id) {
return dep_path.clone();
}
if let Some(dep_path) = self.node_dep_paths.get(node_id) {
return dep_path.clone();
}
if let Some(dep_path) = link_node_id_as_dep_path(node_id) {
return dep_path;
}
let pkg_id = &self.tree.dependencies_tree[node_id].resolved_package_id;
DepPath::from(self.tree.packages[pkg_id].id.clone())
}
/// One resolved-peer slot of `node_id`'s suffix, computed against the
/// already-finalized depPaths. Mirrors [`Self::build_peer_id`] but
/// substitutes the provisional cycle fallback with a strongly-
/// connected-component test: a peer is collapsed to `name@version`
/// only when it shares a peer-graph SCC with `node_id` (a genuine
/// cycle, matching pnpm's `cyclicPeerAliases` branch). Non-cyclic
/// peers carry their full depPath, which is the parity fix.
fn final_peer_id(
&self,
peer_alias: &str,
peer_node_id: &NodeId,
node_scc: usize,
scc_of: &HashMap<NodeId, usize>,
final_dep_paths: &HashMap<NodeId, DepPath>,
) -> PeerId {
if let NodeId::Leaf(id) = peer_node_id
&& let Some(rel) = id.strip_prefix("link:")
{
return PeerId::Pair {
name: peer_alias.to_string(),
version: link_path_to_peer_version(rel),
};
}
let pair = || {
let tree_node = &self.tree.dependencies_tree[peer_node_id];
let pkg = &self.tree.packages[&tree_node.resolved_package_id];
let (name, version) = pkg_name_version(&pkg.result);
PeerId::Pair { name, version }
};
if self.opts.dedupe_peers && self.tree.dependencies_tree.contains_key(peer_node_id) {
return pair();
}
if scc_of.get(peer_node_id) == Some(&node_scc) {
return pair();
}
PeerId::DepPath(self.final_dep_path_of(peer_node_id, final_dep_paths))
}
/// Recompute every node's depPath with its resolved peers' *full*
/// suffixes. Genuine peer cycles (detected as multi-node peer-graph
/// SCCs, or self-loops) keep the `name@version` collapse; every other
/// peer slot carries the peer's own depPath. This is the synchronous
/// equivalent of pnpm's deferred `calculateDepPath` + `analyzeGraph`
/// cycle detection.
fn build_final_dep_paths(&self) -> HashMap<NodeId, DepPath> {
let (sccs, scc_of) = self.peer_sccs();
let mut final_dep_paths: HashMap<NodeId, DepPath> = HashMap::new();
// SCCs come out of Tarjan in reverse-topological order, so a
// node's cross-SCC peers are already finalized when we reach it.
for (scc_index, scc) in sccs.iter().enumerate() {
for node_id in scc {
let Some(peers) = self.node_external_peers.get(node_id) else { continue };
if peers.is_empty() {
continue;
}
let mut peer_ids: Vec<PeerId> = peers
.iter()
.map(|(peer_alias, peer_node_id)| {
self.final_peer_id(
peer_alias,
peer_node_id,
scc_index,
&scc_of,
&final_dep_paths,
)
})
.collect();
peer_ids.sort_by_key(PeerId::as_segment);
peer_ids.dedup_by_key(|peer_id| peer_id.as_segment());
let suffix =
create_peer_dep_graph_hash(&peer_ids, self.opts.peers_suffix_max_length);
let pkg_id = &self.tree.dependencies_tree[node_id].resolved_package_id;
let dep_path =
DepPath::from(format!("{}{}", self.tree.packages[pkg_id].id, suffix));
final_dep_paths.insert(node_id.clone(), dep_path);
}
}
final_dep_paths
}
/// Strongly-connected components of the peer graph (node → resolved
/// peers, restricted to peers that themselves carry peers — peerless
/// peers can't close a cycle). Iterative Tarjan, returning the SCCs
/// in reverse-topological order plus a `NodeId → SCC index` map.
fn peer_sccs(&self) -> (Vec<Vec<NodeId>>, HashMap<NodeId, usize>) {
let participants: HashSet<&NodeId> = self
.node_external_peers
.iter()
.filter(|(_, peers)| !peers.is_empty())
.map(|(node_id, _)| node_id)
.collect();
let neighbors = |node_id: &NodeId| -> Vec<NodeId> {
self.node_external_peers
.get(node_id)
.into_iter()
.flat_map(|peers| peers.values())
.filter(|peer| participants.contains(*peer))
.cloned()
.collect()
};
let mut index_of: HashMap<NodeId, u32> = HashMap::new();
let mut low_of: HashMap<NodeId, u32> = HashMap::new();
let mut on_stack: HashSet<NodeId> = HashSet::new();
let mut tarjan_stack: Vec<NodeId> = Vec::new();
let mut sccs: Vec<Vec<NodeId>> = Vec::new();
let mut scc_of: HashMap<NodeId, usize> = HashMap::new();
let mut next_index: u32 = 0;
// Explicit DFS stack of (node, neighbors, cursor) so deep peer
// graphs don't overflow the call stack.
for root in &participants {
if index_of.contains_key(*root) {
continue;
}
let mut work: Vec<(NodeId, Vec<NodeId>, usize)> =
vec![((*root).clone(), neighbors(root), 0)];
while let Some((node_id, succ, cursor)) = work.last_mut() {
if *cursor == 0 {
index_of.insert(node_id.clone(), next_index);
low_of.insert(node_id.clone(), next_index);
next_index += 1;
on_stack.insert(node_id.clone());
tarjan_stack.push(node_id.clone());
}
if *cursor < succ.len() {
let child = succ[*cursor].clone();
*cursor += 1;
if !index_of.contains_key(&child) {
let child_succ = neighbors(&child);
work.push((child, child_succ, 0));
} else if on_stack.contains(&child) {
let node_low = low_of[node_id];
let child_index = index_of[&child];
low_of.insert(node_id.clone(), node_low.min(child_index));
}
continue;
}
// All successors visited — close this node.
let node_id = node_id.clone();
if low_of[&node_id] == index_of[&node_id] {
let scc_index = sccs.len();
let mut component = Vec::new();
while let Some(member) = tarjan_stack.pop() {
on_stack.remove(&member);
scc_of.insert(member.clone(), scc_index);
let is_root = member == node_id;
component.push(member);
if is_root {
break;
}
}
sccs.push(component);
}
work.pop();
if let Some((parent, _, _)) = work.last() {
let parent_low = low_of[parent];
let node_low = low_of[&node_id];
low_of.insert(parent.clone(), parent_low.min(node_low));
}
}
}
(sccs, scc_of)
}
/// Rebuild the depPath-keyed graph from the per-`NodeId`
/// [`NodeRecord`]s using the corrected `final_dep_paths`. Nodes that
/// resolve to the same final depPath merge (taking the smallest
/// `depth`, like the inline build); nodes whose suffix was
/// previously collapsed by the cycle fallback now split into
/// distinct entries.
fn build_final_graph(&self, final_dep_paths: &HashMap<NodeId, DepPath>) -> DependenciesGraph {
// Minimum tree depth across *every* occurrence that resolves to a
// given final depPath. `pure_pkgs` / `find_hit` revisits
// short-circuit before a [`NodeRecord`] is created, so iterating
// `node_records` alone would restore the first (possibly deeper)
// walk's depth and miss a later shallower revisit. `node_dep_paths`
// carries every walked NodeId, so recompute the `Math.min` depth
// tie-break here — the inline build threaded it through `self.graph`,
// which this rebuild discards.
let mut min_depth: HashMap<DepPath, i32> = HashMap::new();
for node_id in self.node_dep_paths.keys() {
let Some(tree_node) = self.tree.dependencies_tree.get(node_id) else { continue };
let dep_path = self.final_dep_path_of(node_id, final_dep_paths);
min_depth
.entry(dep_path)
.and_modify(|depth| *depth = (*depth).min(tree_node.depth))
.or_insert(tree_node.depth);
}
let mut graph = DependenciesGraph::new();
for (node_id, record) in &self.node_records {
let dep_path = self.final_dep_path_of(node_id, final_dep_paths);
let depth = min_depth.get(&dep_path).copied().unwrap_or(record.depth);
let pkg_id = self.tree.dependencies_tree[node_id].resolved_package_id.clone();
let pkg = &self.tree.packages[&pkg_id];
let children: BTreeMap<String, DepPath> = record
.edges
.iter()
.map(|(alias, edge_node_id)| {
(alias.clone(), self.final_dep_path_of(edge_node_id, final_dep_paths))
})
.collect();
let resolved_peer_names: HashSet<String> = self
.node_external_peers
.get(node_id)
.map(|peers| peers.keys().cloned().collect())
.unwrap_or_default();
graph
.entry(dep_path.clone())
.and_modify(|node| {
if node.depth > depth {
node.depth = depth;
}
})
.or_insert(DependenciesGraphNode {
dep_path,
resolved_package_id: pkg_id.clone(),
resolve_result: Arc::clone(&pkg.result),
children,
peer_dependencies: pkg.peer_dependencies.clone(),
transitive_peer_dependencies: record.transitive_peer_dependencies.clone(),
resolved_peer_names,
depth,
installable: record.installable,
is_pure: record.is_pure,
optional: pkg.optional,
});
}
graph
}
/// Realize the `(alias → NodeId)` children of `node_id` if it's
/// currently a [`TreeChildren::Lazy`] entry; return the realized
/// map (cloned for the caller). On a [`TreeChildren::Realized`]

View File

@@ -921,6 +921,283 @@ mod peers {
);
}
/// A package's graph-node `depth` is the minimum across all
/// occurrences, even when the shallower one short-circuits through the
/// `pure_pkgs` fast path (which has no `NodeRecord`). `p` is reached at
/// depth 2 via `a → b → p` (walked first, so its record carries depth
/// 2) and at depth 1 via `c → p` (a pure-pkgs revisit). The rebuilt
/// graph must record depth 1. Regression guard for the `build_final_graph`
/// depth tie-break.
#[tokio::test]
async fn shallower_pure_pkgs_revisit_lowers_graph_depth() {
let mut table = HashMap::new();
table.insert(
("a".to_string(), "1.0.0".to_string()),
fake_result(
"a",
"1.0.0",
serde_json::json!({
"name": "a",
"version": "1.0.0",
"dependencies": { "b": "1.0.0" }
}),
),
);
table.insert(
("b".to_string(), "1.0.0".to_string()),
fake_result(
"b",
"1.0.0",
serde_json::json!({
"name": "b",
"version": "1.0.0",
"dependencies": { "p": "1.0.0" }
}),
),
);
table.insert(
("c".to_string(), "1.0.0".to_string()),
fake_result(
"c",
"1.0.0",
serde_json::json!({
"name": "c",
"version": "1.0.0",
"dependencies": { "p": "1.0.0" }
}),
),
);
// `p` has a dep `q` so it gets per-occurrence NodeIds (a shared
// leaf would already carry its minimum depth in the tree). Its
// whole subtree is peer-free, so the second, shallower occurrence
// under `c` takes the `pure_pkgs` fast path — which records no
// `NodeRecord`, the case the rebuild must still account for.
table.insert(
("p".to_string(), "1.0.0".to_string()),
fake_result(
"p",
"1.0.0",
serde_json::json!({
"name": "p",
"version": "1.0.0",
"dependencies": { "q": "1.0.0" }
}),
),
);
table.insert(
("q".to_string(), "1.0.0".to_string()),
fake_result("q", "1.0.0", serde_json::json!({ "name": "q", "version": "1.0.0" })),
);
let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) };
// `a` precedes `c` so `p` is first walked at depth 2 (`a → b → p`),
// then revisited at depth 1 (`c → p`).
let (_tmp, manifest) = fake_manifest(serde_json::json!({ "a": "1.0.0", "c": "1.0.0" }));
let mut tree = resolve_dependency_tree(
&resolver,
&manifest,
[DependencyGroup::Prod],
ResolveDependencyTreeOptions {
base_opts: ResolveOptions::default(),
patched_dependencies: None,
manifest_hook: None,
pnpmfile_hook: None,
read_package_log: None,
},
)
.await
.unwrap();
let result = resolve_peers(&mut tree, ResolvePeersOptions::default());
let p_node = &result.graph[&DepPath::from("p@1.0.0".to_string())];
assert_eq!(p_node.depth, 1, "p's graph depth must be the minimum (1), not 2");
}
/// Port of upstream's
/// [`"a peer's own peer is shared with a sibling that peer-depends both"`](https://github.com/pnpm/pnpm/blob/894ea6af2c/installing/deps-resolver/test/resolvePeers.ts#L1207).
/// `plugin` peer-depends both `parser` and `typescript`; `parser`
/// peer-depends `typescript`. `plugin` and `parser` live under
/// `umbrella` (under `app`, which also brings `typescript@1.0.0`), while
/// the importer also has a top-level `typescript@2.0.0` + `parser@1.0.0`.
/// `plugin`'s `parser` must resolve `typescript@1.0.0` — the version
/// `plugin` itself uses — not be shadowed by the top-level `parser` that
/// resolved `typescript@2.0.0`. Exercises the depPath suffix machinery.
#[tokio::test]
async fn peers_own_peer_shared_with_sibling_that_peer_depends_both() {
let mut table = HashMap::new();
for version in ["1.0.0", "2.0.0"] {
table.insert(
("typescript".to_string(), version.to_string()),
fake_result(
"typescript",
version,
serde_json::json!({ "name": "typescript", "version": version }),
),
);
}
table.insert(
("parser".to_string(), "1.0.0".to_string()),
fake_result(
"parser",
"1.0.0",
serde_json::json!({
"name": "parser",
"version": "1.0.0",
"peerDependencies": { "typescript": "*" }
}),
),
);
table.insert(
("plugin".to_string(), "1.0.0".to_string()),
fake_result(
"plugin",
"1.0.0",
serde_json::json!({
"name": "plugin",
"version": "1.0.0",
"peerDependencies": { "parser": "*", "typescript": "*" }
}),
),
);
table.insert(
("umbrella".to_string(), "1.0.0".to_string()),
fake_result(
"umbrella",
"1.0.0",
serde_json::json!({
"name": "umbrella",
"version": "1.0.0",
"dependencies": { "plugin": "1.0.0", "parser": "1.0.0" },
"peerDependencies": { "typescript": "*" }
}),
),
);
table.insert(
("app".to_string(), "1.0.0".to_string()),
fake_result(
"app",
"1.0.0",
serde_json::json!({
"name": "app",
"version": "1.0.0",
"dependencies": { "typescript": "1.0.0", "umbrella": "1.0.0" }
}),
),
);
let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) };
let (_tmp, manifest) = fake_manifest(serde_json::json!({
"typescript": "2.0.0",
"parser": "1.0.0",
"app": "1.0.0",
}));
let mut tree = resolve_dependency_tree(
&resolver,
&manifest,
[DependencyGroup::Prod],
ResolveDependencyTreeOptions {
base_opts: ResolveOptions::default(),
patched_dependencies: None,
manifest_hook: None,
pnpmfile_hook: None,
read_package_log: None,
},
)
.await
.unwrap();
let result = resolve_peers(&mut tree, ResolvePeersOptions::default());
let keys: Vec<String> = result.graph.keys().map(|dp| dp.as_str().to_string()).collect();
assert!(
keys.contains(
&"plugin@1.0.0(parser@1.0.0(typescript@1.0.0))(typescript@1.0.0)".to_string()
),
"plugin's parser must resolve typescript@1.0.0; got: {keys:?}",
);
assert!(
!keys.contains(
&"plugin@1.0.0(parser@1.0.0(typescript@2.0.0))(typescript@1.0.0)".to_string()
),
"plugin's parser must not be shadowed by the top-level typescript@2.0.0: {keys:?}",
);
}
/// A peer that is a walk-ancestor must still carry its own peer
/// suffix in the dependent's depPath. `a` is a direct dep with peer
/// `c`; its child `b` peer-depends on `a`. While `b`'s suffix is
/// being built, `a` is mid-walk (in-progress) so its depPath isn't
/// finalized yet. The post-walk [`build_final_dep_paths`] pass must
/// resolve `a` to its full `a@1.0.0(c@1.0.0)` — not the collapsed
/// `a@1.0.0` the cycle fallback would emit (pnpm only collapses
/// genuine cycles, and `a→b→a` here resolves because `a` and `b`
/// don't form a peer-graph SCC). Regression test for
/// <https://github.com/pnpm/pnpm/issues/12266>.
#[tokio::test]
async fn ancestor_peer_carries_its_own_suffix() {
let mut table = HashMap::new();
table.insert(
("a".to_string(), "1.0.0".to_string()),
fake_result(
"a",
"1.0.0",
serde_json::json!({
"name": "a",
"version": "1.0.0",
"dependencies": { "b": "1.0.0" },
"peerDependencies": { "c": "*" }
}),
),
);
table.insert(
("b".to_string(), "1.0.0".to_string()),
fake_result(
"b",
"1.0.0",
serde_json::json!({
"name": "b",
"version": "1.0.0",
"peerDependencies": { "a": "*" }
}),
),
);
table.insert(
("c".to_string(), "1.0.0".to_string()),
fake_result("c", "1.0.0", serde_json::json!({ "name": "c", "version": "1.0.0" })),
);
let resolver = StubResolver { table, calls: Mutex::new(Vec::new()) };
let (_tmp, manifest) = fake_manifest(serde_json::json!({ "a": "1.0.0", "c": "1.0.0" }));
let mut tree = resolve_dependency_tree(
&resolver,
&manifest,
[DependencyGroup::Prod],
ResolveDependencyTreeOptions {
base_opts: ResolveOptions::default(),
patched_dependencies: None,
manifest_hook: None,
pnpmfile_hook: None,
read_package_log: None,
},
)
.await
.unwrap();
let result = resolve_peers(&mut tree, ResolvePeersOptions::default());
let mut keys: Vec<String> = result.graph.keys().map(|dp| dp.as_str().to_string()).collect();
keys.sort();
assert_eq!(
keys,
vec![
"a@1.0.0(c@1.0.0)".to_string(),
"b@1.0.0(a@1.0.0(c@1.0.0))".to_string(),
"c@1.0.0".to_string(),
],
);
// `c` is `a`'s peer, not `b`'s — it must not leak into `b`'s
// dependencies (only `b`'s own peer `a` is a child of `b`).
let b_node = &result.graph[&DepPath::from("b@1.0.0(a@1.0.0(c@1.0.0))".to_string())];
let b_children: Vec<&str> = b_node.children.keys().map(String::as_str).collect();
assert_eq!(b_children, vec!["a"]);
}
/// Shared fixture for the `dedupe_peers_*` pair: react@18 plus
/// `@emotion/react@11` (peer: react) plus `@emotion/styled@11`
/// (peers: react, @emotion/react). Mirrors upstream's

View File

@@ -836,3 +836,32 @@ Tracks pnpm/pnpm#12196. The `catalogMode` mismatch gate landed earlier (pnpm#117
- Settings drift comparison is field-by-field on `WorkspaceStateSettings::PartialEq` rather than the upstream `Object.entries` walk. Equivalent in behavior: any field present in the cached state but `None` in today's `current_settings` (or vice versa) trips the check.
- The settings construction is shared between the writer (`build_workspace_state` in `install.rs`) and the reader (`current_settings` in `optimistic_repeat_install.rs`) so adding a tracked field on one side automatically updates the other.
## Peer Resolution (`installing/deps-resolver/test/resolvePeers.ts`, `hoistPeers.test.ts`)
Status of the upstream peer-resolution suites, audited while landing the
lockfile-parity peer fixes (pnpm/pnpm#12266, pnpm/pnpm#12267).
### `hoistPeers.test.ts` — fully ported
- [x] All 11 cases (`hoistPeers` × 8 + `getHoistableOptionalPeers` × 3) are ported in `hoist_peers/tests.rs`, plus two prerelease siblings pacquet adds.
### `resolvePeers.ts` — ported / covered
- [x] `transitive peers use version-only suffixes``dedupe_peers_propagates_transitive_peer_to_parent`.
- [x] `uses version-only peer suffixes without nested dep paths``dedupe_peers_collapses_nested_peer_suffixes` / `no_dedupe_peers_keeps_nested_peer_suffixes`.
- [x] `resolve peer dependencies of cyclic dependencies``cyclic_peer_dependencies_resolve_cleanly`.
- [x] `when a package is referenced twice … still try to resolve it in the other occurrence``revisit_resolves_peer_in_one_occurrence_misses_in_other`.
- [x] `should return from where the bad peer dependency is resolved``bad_peer_inside_subtree_records_resolved_from_parent`.
- [x] `should find peer dependency conflicts` — covered by `bad_peer_version_is_reported`.
- [x] `a peer's own peer is shared with a sibling that peer-depends both` — ported as `peers_own_peer_shared_with_sibling_that_peer_depends_both`.
- [x] Walk-ancestor suffix propagation (no direct upstream case — pacquet-specific manifestation of the deferred `calculateDepPath`) — `ancestor_peer_carries_its_own_suffix`.
- [x] Optional peer not hoisted from the run-resolved tree (resolveRootDependencies behavior behind `getHoistableOptionalPeers`) — `optional_peer_only_in_resolved_tree_is_not_hoisted`.
- [x] `build_final_graph` min-depth tie-break across `pure_pkgs`/`find_hit` revisits (pacquet-specific) — `shallower_pure_pkgs_revisit_lowers_graph_depth`.
### `resolvePeers.ts` — not yet ported
- [ ] `multi-project: different peer versions produce different instances` — needs the multi-importer `resolve_peers_workspace` harness; general workspace peer-separation, partially covered by `dedupes_when_the_same_package_appears_in_two_subtrees`.
- [ ] `resolve peer dependencies with npm aliases` — npm-alias peer suffixes.
- [ ] `should find peer dependency conflicts when the peer is an optional peer of one of the dependencies`, `should ignore conflicts between missing optional peer dependencies`, `should pick the single wanted peer dependency range`, `should return the intersection of two compatible ranges`, the two prerelease-warning cases — peer-issue reporting edge cases.
- [ ] The `lockedPeerContext` / `resolvedPeerProviderPaths` series (`prefers a compatible locked provider …`, the six `does not replace …` cases, `does not reuse a locked provider outside the current peer range`) — pacquet hasn't ported `lockedPeerContext`/`resolvedPeerProviderPaths`, so these gate on that feature, not on the lockfile-parity peer fixes.