fix(resolver): keep hoisted peer providers out of the root peer context (#12847)

With autoInstallPeers, peers resolved inside a dependency subtree are
attached to the root importer's direct dependencies so other subtrees
can reuse them. Those entries alias tree nodes that already have a
position deep in the graph, and the peer-resolution pass walked them a
second time as root children. The two walks raced on the shared
per-node dep-path state, so a package inside a self-contained closure
could get its peers bound to the root project's incompatible version
of a peer instead of the provider next to it in the tree, producing a
lockfile that mixes both versions and a peer mismatch at run time.

The peer-resolution pass now keeps the attached providers visible as
root-level peer providers but resolves their own peers only at their
true tree position, falling back to the root context only when that
position was pruned by the peers cache. All pruned providers are
resolved in a single fallback pass, because a pass only detects peer
cycles among its own children and mutually peer-depending providers
would otherwise await each other's dep path forever. The fix lands in both stacks:
the TypeScript `@pnpm/installing.deps-resolver` and pacquet's
`resolving-deps-resolver` crate.

Fixes https://github.com/pnpm/pnpm/issues/4993

---------

Signed-off-by: C. Spencer Beggs <spencer@beggs.codes>
Co-authored-by: Zoltan Kochan <zoltankochan@gmail.com>
This commit is contained in:
C. Spencer Beggs
2026-07-09 16:00:20 -04:00
committed by GitHub
parent a8ad82d4dd
commit fecfe8334b
19 changed files with 653 additions and 19 deletions

View File

@@ -0,0 +1,6 @@
---
"@pnpm/installing.deps-resolver": patch
"pnpm": patch
---
Fixed peer dependency resolution with `autoInstallPeers` when a workspace package depends on a version of a package that a transitive dependency's self-contained closure also provides for itself. The peer providers that are attached to the root project for reuse are no longer peer-resolved a second time in the root context, so packages inside such a closure no longer get their peers bound to the root project's incompatible version [#4993](https://github.com/pnpm/pnpm/issues/4993).

View File

@@ -1720,6 +1720,103 @@ async fn auto_install_peers_skips_meta_only_optional_peers() {
drop((dir, mock_instance));
}
/// Mirror of the TS test "a root dependency does not override the
/// peers provided inside a self-contained subtree"
/// (`deps-installer/test/install/autoInstallPeers.ts`).
///
/// `@pnpm.e2e/closure-plugins` provides every peer of its own subtree
/// (`closure-lib-a` and `closure-lib-b` peer-depend on each other and
/// on `closure-peer-x`, and all three are its regular dependencies).
/// The root project additionally depends on the incompatible
/// `closure-peer-x@2.0.0`. With `autoInstallPeers` (the default), the
/// peers resolved inside the subtree are attached to the root project
/// for reuse — but they must not be peer-resolved again in the root
/// context, where `closure-peer-x@2.0.0` is the nearest provider, or
/// the subtree's peer graph gets a mix of both versions.
#[tokio::test]
async fn root_dependency_does_not_override_peers_of_self_contained_subtree() {
let mock_instance = TestRegistry::start();
let dir = tempdir().unwrap();
let store_dir = dir.path().join("pacquet-store");
let project_root = dir.path().join("project");
let modules_dir = project_root.join("node_modules");
let virtual_store_dir = modules_dir.join(".pacquet");
let manifest_path = dir.path().join("package.json");
let mut manifest = PackageManifest::create_if_needed(manifest_path.clone()).unwrap();
manifest.add_dependency("@pnpm.e2e/closure-plugins", "1.0.0", DependencyGroup::Prod).unwrap();
manifest.add_dependency("@pnpm.e2e/closure-peer-x", "2.0.0", DependencyGroup::Prod).unwrap();
manifest.save().unwrap();
let mut config = Config::new();
config.store_dir = store_dir.into();
config.modules_dir = modules_dir.clone();
config.virtual_store_dir = virtual_store_dir;
config.registry = mock_instance.url();
let config = config.leak();
Install {
tarball_mem_cache: Default::default(),
http_client: &Default::default(),
http_client_arc: std::sync::Arc::new(Default::default()),
config,
manifest: &manifest,
emit_initial_manifest: true,
lockfile: MaybeLazyLockfile::Loaded(None),
lockfile_path: None,
dependency_groups: [DependencyGroup::Prod, DependencyGroup::Dev, DependencyGroup::Optional],
frozen_lockfile: false,
prefer_frozen_lockfile: None,
ignore_manifest_check: false,
skip_runtimes: false,
trust_lockfile: false,
update_checksums: false,
is_full_install: true,
supported_architectures: None,
node_linker: pacquet_config::NodeLinker::default(),
lockfile_only: false,
dry_run: false,
resolved_packages: &Default::default(),
update_seed_policy: crate::UpdateSeedPolicy::KeepAll,
auth_override: None,
resolution_observer: None,
catalogs_override: None,
disable_optimistic_repeat_install: false,
pnpmfile_hook_override: None,
workspace_projects_override: None,
}
.run::<SilentReporter>()
.await
.expect("install should succeed");
let content =
std::fs::read_to_string(dir.path().join(Lockfile::FILE_NAME)).expect("read pnpm-lock.yaml");
assert!(
!content.contains("(@pnpm.e2e/closure-peer-x@2.0.0)"),
"no peer inside the self-contained subtree may bind to the root's \
closure-peer-x@2.0.0; lockfile:\n{content}",
);
assert!(
content.contains("(@pnpm.e2e/closure-peer-x@1.0.0)"),
"the subtree's peers must bind to its own closure-peer-x@1.0.0; lockfile:\n{content}",
);
// The root keeps its own explicitly declared version.
let lockfile: Lockfile = serde_saphyr::from_str(&content).expect("parse pnpm-lock.yaml");
let root_deps = lockfile
.root_project()
.expect("root importer recorded")
.dependencies
.as_ref()
.expect("dependencies map");
let peer_x_key = pacquet_lockfile::PkgName::parse("@pnpm.e2e/closure-peer-x").unwrap();
let root_peer_x = root_deps.get(&peer_x_key).expect("closure-peer-x recorded at root");
assert_eq!(root_peer_x.version.to_string(), "2.0.0");
drop((dir, mock_instance));
}
/// A v9 lockfile fixture pinned to a placeholder package whose
/// integrity is bogus on purpose. Pacquet enforces tarball integrity
/// on the install path, so any test that lets the install reach the

View File

@@ -285,6 +285,12 @@ pub(crate) struct ImporterHoistState {
importer_id: String,
ctx: TreeCtx,
direct: Vec<DirectDep>,
/// `NodeIds` appended to `direct` by
/// [`Self::append_resolved_peer_providers`]. Threaded into
/// [`ResolvePeersOptions::hoisted_peer_provider_node_ids`] so the
/// peer walk resolves them at their tree position instead of the
/// importer root.
hoisted_peer_provider_node_ids: HashSet<crate::NodeId>,
parent_pkg_aliases: HashSet<String>,
all_missing_optional_peers: BTreeMap<String, Vec<String>>,
all_preferred_versions: PreferredVersions,
@@ -357,6 +363,7 @@ impl ImporterHoistState {
importer_id: importer_id.to_string(),
ctx,
direct,
hoisted_peer_provider_node_ids: HashSet::new(),
parent_pkg_aliases,
all_missing_optional_peers: BTreeMap::new(),
all_preferred_versions,
@@ -379,6 +386,7 @@ impl ImporterHoistState {
lockfile_dir: self.lockfile_dir.clone(),
modules_dir: self.modules_dir.clone(),
hoist_missing_scope: None,
hoisted_peer_provider_node_ids: self.hoisted_peer_provider_node_ids.clone(),
}
}
@@ -517,6 +525,7 @@ impl ImporterHoistState {
node_id: node_id.clone(),
id: tree_node.resolved_package_id.clone(),
});
self.hoisted_peer_provider_node_ids.insert(node_id.clone());
self.parent_pkg_aliases.insert(alias.clone());
}
}
@@ -558,9 +567,11 @@ impl ImporterHoistState {
/// The importer's direct-dep envelopes for the workspace-wide peer
/// pass (which recomputes peers across importers; the per-importer
/// pass would be discarded).
pub(crate) fn into_direct(self) -> Vec<DirectDep> {
self.direct
/// pass would be discarded), together with the `NodeIds` of the peer
/// providers among them (see
/// [`ResolvePeersOptions::hoisted_peer_provider_node_ids`]).
pub(crate) fn into_direct(self) -> (Vec<DirectDep>, HashSet<crate::NodeId>) {
(self.direct, self.hoisted_peer_provider_node_ids)
}
/// Run the final per-importer peer pass and emit the result. Used

View File

@@ -137,6 +137,19 @@ pub struct ResolvePeersOptions {
/// per-importer snapshots shared across every hoist-loop
/// iteration.
pub hoist_missing_scope: Option<std::sync::Arc<HoistMissingScope>>,
/// `NodeIds` of the peer providers the auto-install-peers loop
/// attached to importers' direct dependencies for reuse by other
/// subtrees. Each such node keeps its original position inside
/// the dependency tree, so the walk resolves its peers there;
/// walking it a second time as a root child would bind its peers
/// against the importer's own dependencies instead of the
/// providers next to it in the tree, and the root-context result
/// would overwrite the in-place one in `node_dep_paths`. The walk
/// therefore skips these direct entries unless the tree position
/// was pruned (an ancestor hit the peers cache) and nothing else
/// resolved them.
pub hoisted_peer_provider_node_ids: std::collections::HashSet<NodeId>,
}
/// See [`ResolvePeersOptions::hoist_missing_scope`].
@@ -184,6 +197,7 @@ impl Default for ResolvePeersOptions {
lockfile_dir: None,
modules_dir: None,
hoist_missing_scope: None,
hoisted_peer_provider_node_ids: std::collections::HashSet::new(),
}
}
}
@@ -331,12 +345,35 @@ pub fn resolve_peers_workspace(
let parent_node_ids: Vec<NodeId> = Vec::new();
let parent_pkg_ids_chain: Vec<String> = Vec::new();
let importer_parent_dep_paths = walker.parent_dep_paths_from_refs(&importer_parents);
for dep in &importer.direct {
let (own_direct, provider_direct): (Vec<&DirectDep>, Vec<&DirectDep>) = importer
.direct
.iter()
.partition(|dep| !walker.opts.hoisted_peer_provider_node_ids.contains(&dep.node_id));
for dep in &own_direct {
walker
.parent_pkgs_of_node
.insert(dep.node_id.clone(), importer_parent_dep_paths.clone());
}
for dep in &importer.direct {
for dep in &own_direct {
walker.resolve_node(
dep.node_id.clone(),
&importer_parents,
&parent_chain_names,
&parent_node_ids,
&parent_pkg_ids_chain,
);
}
// See ResolvePeersOptions::hoisted_peer_provider_node_ids — a
// provider is normally resolved at its tree position during the
// walk above; only one whose position was pruned still needs the
// root-context fallback.
for dep in &provider_direct {
if walker.node_dep_paths.contains_key(&dep.node_id) {
continue;
}
walker
.parent_pkgs_of_node
.insert(dep.node_id.clone(), importer_parent_dep_paths.clone());
walker.resolve_node(
dep.node_id.clone(),
&importer_parents,
@@ -605,10 +642,33 @@ impl Walker<'_> {
// `self.tree.direct`.
let direct: Vec<DirectDep> = self.tree.direct.clone();
let importer_parent_dep_paths = self.parent_dep_paths_from_refs(&importer_parents);
for dep in &direct {
let (own_direct, provider_direct): (Vec<&DirectDep>, Vec<&DirectDep>) = direct
.iter()
.partition(|dep| !self.opts.hoisted_peer_provider_node_ids.contains(&dep.node_id));
for dep in &own_direct {
self.parent_pkgs_of_node.insert(dep.node_id.clone(), importer_parent_dep_paths.clone());
}
for dep in &direct {
for dep in &own_direct {
let output = self.resolve_node(
dep.node_id.clone(),
&importer_parents,
&parent_chain_names,
&parent_node_ids,
&parent_pkg_ids_chain,
);
for (peer_alias, peer_node_id) in output.auto_install_resolved_peers {
self.resolved_peer_providers_by_alias.insert(peer_alias, peer_node_id);
}
}
// See ResolvePeersOptions::hoisted_peer_provider_node_ids — a
// provider is normally resolved at its tree position during the
// walk above; only one whose position was pruned still needs the
// root-context fallback.
for dep in &provider_direct {
if self.node_dep_paths.contains_key(&dep.node_id) {
continue;
}
self.parent_pkgs_of_node.insert(dep.node_id.clone(), importer_parent_dep_paths.clone());
let output = self.resolve_node(
dep.node_id.clone(),
&importer_parents,

View File

@@ -1,4 +1,7 @@
use super::{NodeRecord, ResolvePeersOptions, Walker, resolve_peers, satisfies_with_prereleases};
use super::{
ImporterPeerInput, NodeRecord, ResolvePeersOptions, Walker, resolve_peers,
resolve_peers_workspace, satisfies_with_prereleases,
};
use crate::{
dependencies_graph::{DependenciesGraph, PeerDependencyIssues},
node_id::NodeId,
@@ -1203,6 +1206,201 @@ fn shared_package_optional_transitive_peer_resolves_deterministically() {
}
}
/// A hoisted peer provider whose tree position was never visited (nothing in
/// the walk enumerates its node) must still be resolved by the root-context
/// fallback so consumers that bound it get a depPath.
#[test]
fn pruned_hoisted_provider_falls_back_to_root_resolution() {
let prov = NodeId::leaf("prov@1.0.0");
let consumer = NodeId::next();
let mut tree = ResolvedTree {
direct: vec![
DirectDep {
alias: "consumer".to_string(),
node_id: consumer.clone(),
id: "consumer@1.0.0".to_string(),
},
DirectDep {
alias: "prov".to_string(),
node_id: prov.clone(),
id: "prov@1.0.0".to_string(),
},
],
packages: HashMap::from([
("prov@1.0.0".to_string(), package("prov", "1.0.0", &[], true)),
("consumer@1.0.0".to_string(), package("consumer", "1.0.0", &[("prov", "*")], false)),
]),
dependencies_tree: HashMap::from([
(prov.clone(), tree_node("prov@1.0.0", BTreeMap::new(), 1)),
(consumer, tree_node("consumer@1.0.0", BTreeMap::new(), 0)),
]),
all_peer_dep_names: HashSet::from(["prov".to_string()]),
policy_violations: Vec::new(),
applied_patches: HashSet::new(),
children_by_id: HashMap::new(),
};
let result = resolve_peers(
&mut tree,
ResolvePeersOptions {
hoisted_peer_provider_node_ids: HashSet::from([prov]),
..ResolvePeersOptions::default()
},
);
assert_eq!(
result.direct_dependencies_by_alias.get("prov"),
Some(&DepPath::from("prov@1.0.0")),
"the pruned provider must get a depPath from the fallback",
);
assert!(
result.graph.contains_key(&DepPath::from("consumer@1.0.0(prov@1.0.0)")),
"the consumer must bind the fallback-resolved provider: {:#?}",
result.graph.keys().collect::<Vec<_>>(),
);
}
/// Same as [`pruned_hoisted_provider_falls_back_to_root_resolution`] but
/// through the multi-importer entry point.
#[test]
fn pruned_hoisted_provider_falls_back_in_workspace_pass() {
let prov = NodeId::leaf("prov@1.0.0");
let consumer = NodeId::next();
let importer = ImporterPeerInput {
id: ".".to_string(),
direct: vec![
DirectDep {
alias: "consumer".to_string(),
node_id: consumer.clone(),
id: "consumer@1.0.0".to_string(),
},
DirectDep {
alias: "prov".to_string(),
node_id: prov.clone(),
id: "prov@1.0.0".to_string(),
},
],
root_dir: std::path::PathBuf::from("/repo"),
modules_dir: None,
};
let mut tree = ResolvedTree {
direct: Vec::new(),
packages: HashMap::from([
("prov@1.0.0".to_string(), package("prov", "1.0.0", &[], true)),
("consumer@1.0.0".to_string(), package("consumer", "1.0.0", &[("prov", "*")], false)),
]),
dependencies_tree: HashMap::from([
(prov.clone(), tree_node("prov@1.0.0", BTreeMap::new(), 1)),
(consumer, tree_node("consumer@1.0.0", BTreeMap::new(), 0)),
]),
all_peer_dep_names: HashSet::from(["prov".to_string()]),
policy_violations: Vec::new(),
applied_patches: HashSet::new(),
children_by_id: HashMap::new(),
};
let result = resolve_peers_workspace(
&mut tree,
&[importer],
std::path::Path::new("/repo"),
false,
false,
false,
ResolvePeersOptions {
hoisted_peer_provider_node_ids: HashSet::from([prov]),
..ResolvePeersOptions::default()
},
);
assert_eq!(
result.direct_dependencies_by_importer.get(".").and_then(|deps| deps.get("prov")),
Some(&DepPath::from("prov@1.0.0")),
"the pruned provider must get a depPath from the fallback",
);
assert!(
result.graph.contains_key(&DepPath::from("consumer@1.0.0(prov@1.0.0)")),
"the consumer must bind the fallback-resolved provider: {:#?}",
result.graph.keys().collect::<Vec<_>>(),
);
}
/// Mirror of the TS test "pruned hoisted peer providers that peer-depend on
/// each other are resolved together" (`deps-resolver/test/resolvePeers.ts`):
/// two pruned providers form a peer cycle, so each one's suffix depends on
/// the other's. Both must come out of the fallback with the cycle collapsed
/// to `name@version`, matching the in-place cycle handling.
#[test]
fn pruned_hoisted_providers_with_mutual_peers_resolve() {
let lib_a = NodeId::leaf("lib-a@1.0.0");
let lib_b = NodeId::leaf("lib-b@1.0.0");
let consumer = NodeId::next();
let mut tree = ResolvedTree {
direct: vec![
DirectDep {
alias: "consumer".to_string(),
node_id: consumer.clone(),
id: "consumer@1.0.0".to_string(),
},
DirectDep {
alias: "lib-a".to_string(),
node_id: lib_a.clone(),
id: "lib-a@1.0.0".to_string(),
},
DirectDep {
alias: "lib-b".to_string(),
node_id: lib_b.clone(),
id: "lib-b@1.0.0".to_string(),
},
],
packages: HashMap::from([
("lib-a@1.0.0".to_string(), package("lib-a", "1.0.0", &[("lib-b", "^1.0.0")], true)),
("lib-b@1.0.0".to_string(), package("lib-b", "1.0.0", &[("lib-a", "^1.0.0")], true)),
(
"consumer@1.0.0".to_string(),
package("consumer", "1.0.0", &[("lib-a", "^1.0.0"), ("lib-b", "^1.0.0")], false),
),
]),
dependencies_tree: HashMap::from([
(lib_a.clone(), tree_node("lib-a@1.0.0", BTreeMap::new(), 1)),
(lib_b.clone(), tree_node("lib-b@1.0.0", BTreeMap::new(), 1)),
(consumer, tree_node("consumer@1.0.0", BTreeMap::new(), 0)),
]),
all_peer_dep_names: HashSet::from(["lib-a".to_string(), "lib-b".to_string()]),
policy_violations: Vec::new(),
applied_patches: HashSet::new(),
children_by_id: HashMap::new(),
};
let result = resolve_peers(
&mut tree,
ResolvePeersOptions {
hoisted_peer_provider_node_ids: HashSet::from([lib_a, lib_b]),
..ResolvePeersOptions::default()
},
);
assert_eq!(
result.direct_dependencies_by_alias.get("lib-a"),
Some(&DepPath::from("lib-a@1.0.0(lib-b@1.0.0)")),
"graph keys: {:#?}",
result.graph.keys().collect::<Vec<_>>(),
);
assert_eq!(
result.direct_dependencies_by_alias.get("lib-b"),
Some(&DepPath::from("lib-b@1.0.0(lib-a@1.0.0)")),
"graph keys: {:#?}",
result.graph.keys().collect::<Vec<_>>(),
);
assert!(
result.graph.contains_key(&DepPath::from("consumer@1.0.0(lib-a@1.0.0)(lib-b@1.0.0)")),
"the consumer must bind both fallback-resolved providers: {:#?}",
result.graph.keys().collect::<Vec<_>>(),
);
}
fn tree_node(pkg_id: &str, children: BTreeMap<String, NodeId>, depth: i32) -> DependenciesTreeNode {
DependenciesTreeNode {
resolved_package_id: pkg_id.to_string(),

View File

@@ -253,12 +253,15 @@ where
}
}
let mut per_importer_inputs: Vec<ImporterPeerInput> = Vec::with_capacity(importers.len());
let mut hoisted_peer_provider_node_ids = std::collections::HashSet::new();
for ((importer, state), (project_dir, modules_dir)) in
importers.iter().zip(states).zip(input_dirs)
{
let (direct, importer_provider_node_ids) = state.into_direct();
hoisted_peer_provider_node_ids.extend(importer_provider_node_ids);
per_importer_inputs.push(ImporterPeerInput {
id: importer.id.clone(),
direct: state.into_direct(),
direct,
root_dir: project_dir,
modules_dir,
});
@@ -284,6 +287,7 @@ where
// importer's walk.
modules_dir: None,
hoist_missing_scope: None,
hoisted_peer_provider_node_ids,
};
let peers = resolve_peers_workspace(
&mut merged_tree,

View File

@@ -2152,7 +2152,7 @@ mod peers {
exclude_links_from_lockfile: true,
lockfile_dir: Some(lockfile_dir),
modules_dir: Some(modules_dir),
hoist_missing_scope: None,
..ResolvePeersOptions::default()
},
);

View File

@@ -15,11 +15,10 @@ importers:
packages:
is-negative@https://codeload.github.com/kevva/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c:
resolution: {tarball: https://codeload.github.com/kevva/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c}
resolution: {gitHosted: true, tarball: https://codeload.github.com/kevva/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c}
version: 2.1.0
engines: {node: '>=0.10.0'}
snapshots:
is-negative@https://codeload.github.com/kevva/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c:
dev: false
is-negative@https://codeload.github.com/kevva/is-negative/tar.gz/1d7e288222b53a0cab90a331f1865220ec29560c: {}

View File

@@ -758,3 +758,32 @@ test('a locked optional peer version is not rewritten when a sibling workspace p
expect(optionalPeerVersion).not.toContain('(@pnpm.e2e/peer-c@1.0.0)')
}
})
test('a root dependency does not override the peers provided inside a self-contained subtree', async () => {
// Regression test for the closure-poisoning bug: @pnpm.e2e/closure-plugins
// provides every peer of its own subtree (closure-lib-a and closure-lib-b
// peer-depend on each other and on closure-peer-x, and all of them are
// regular dependencies of closure-plugins). The root project additionally
// depends on the incompatible closure-peer-x@2.0.0. With autoInstallPeers
// enabled, the peers resolved inside the subtree are also attached to the
// root project so other subtrees can reuse them — but they must not be
// peer-resolved again in the root context, where closure-peer-x@2.0.0 is
// the nearest provider, or the subtree's peer graph gets a mix of both
// versions.
const project = prepareEmpty()
await addDependenciesToPackage({}, [
'@pnpm.e2e/closure-plugins@1.0.0',
'@pnpm.e2e/closure-peer-x@2.0.0',
], testDefaults({ autoInstallPeers: true }))
const lockfile = project.readLockfile()
const pluginsSnapshot = lockfile.snapshots['@pnpm.e2e/closure-plugins@1.0.0']
expect(pluginsSnapshot.dependencies?.['@pnpm.e2e/closure-peer-x']).toBe('1.0.0')
expect(pluginsSnapshot.dependencies?.['@pnpm.e2e/closure-lib-a']).not.toContain('closure-peer-x@2.0.0')
expect(pluginsSnapshot.dependencies?.['@pnpm.e2e/closure-lib-b']).not.toContain('closure-peer-x@2.0.0')
for (const depPath of Object.keys(lockfile.snapshots)) {
if (!depPath.startsWith('@pnpm.e2e/closure-lib')) continue
expect(depPath).not.toContain('closure-peer-x@2.0.0')
}
// The root keeps its own explicitly declared version.
expect(lockfile.importers['.'].dependencies?.['@pnpm.e2e/closure-peer-x']?.version).toBe('2.0.0')
})

View File

@@ -77,6 +77,7 @@ interface ProjectToLink {
binsDir: string
declaredDirectDependencies: Set<string>
directNodeIdsByAlias: Map<string, NodeId>
hoistedPeerProviderNodeIds: Set<NodeId>
explicitlyRequestedDirectDependencies: Set<string>
id: ProjectId
linkedDependencies: LinkedDependency[]
@@ -254,6 +255,7 @@ export async function resolveDependencies (
...project.wantedDependencies.flatMap(({ alias, isNew }) => isNew && alias != null ? [alias] : []),
]),
directNodeIdsByAlias: resolvedImporter.directNodeIdsByAlias,
hoistedPeerProviderNodeIds: resolvedImporter.hoistedPeerProviderNodeIds,
explicitlyRequestedDirectDependencies: new Set(
project.wantedDependencies.flatMap(({ alias, bareSpecifier, isNew, prevSpecifier, updateSpec }) =>
alias != null && (isNew === true || updateSpec === true || (prevSpecifier != null && bareSpecifier !== prevSpecifier))

View File

@@ -281,6 +281,13 @@ export interface PkgAddress extends PkgAddressOrLinkBase {
saveCatalogName?: string
lockedPeerContext?: LockedPeerContext
previousDepPath?: DepPath
/**
* A peer dependency provider attached to the root importer so that other
* subtrees can reuse it. Its `nodeId` keeps pointing at the provider's
* original position inside the dependency tree, so the node must be
* peer-resolved there — not in the root context.
*/
hoistedPeerProvider?: boolean
}
export type PkgAddressOrLink = PkgAddress | LinkedDependency
@@ -431,7 +438,10 @@ export async function resolveRootDependencies (
// even those peers should be hoisted that are not autoinstalled
for (const [resolvedPeerName, resolvedPeerAddress] of Object.entries(importerResolutionResult.resolvedPeers ?? {})) {
if (!parentPkgAliases[resolvedPeerName]) {
importerResolutionResult.pkgAddresses.push(resolvedPeerAddress)
importerResolutionResult.pkgAddresses.push({
...resolvedPeerAddress,
hoistedPeerProvider: true,
})
}
}
}

View File

@@ -47,6 +47,7 @@ export interface ResolvedImporters {
[id: string]: {
directDependencies: ResolvedDirectDependency[]
directNodeIdsByAlias: Map<string, NodeId>
hoistedPeerProviderNodeIds: Set<NodeId>
linkedDependencies: LinkedDependency[]
}
}
@@ -357,6 +358,7 @@ export async function resolveDependencyTree<T> (
}
}),
directNodeIdsByAlias: new Map(directNonLinkedDeps.map(({ alias, nodeId }) => [alias, nodeId])),
hoistedPeerProviderNodeIds: new Set(directNonLinkedDeps.filter((dep) => dep.hoistedPeerProvider).map(({ nodeId }) => nodeId)),
linkedDependencies,
}
}

View File

@@ -70,6 +70,8 @@ export interface GenericDependenciesGraphWithResolvedChildren<T extends PartialR
export interface ProjectToResolve {
directNodeIdsByAlias: Map<string, NodeId>
// See PkgAddress.hoistedPeerProvider in resolveDependencies.ts
hoistedPeerProviderNodeIds?: Set<NodeId>
declaredDirectDependencies?: Set<string>
explicitlyRequestedDirectDependencies?: Set<string>
// only the top dependencies that were already installed
@@ -121,7 +123,7 @@ export async function resolvePeers<T extends PartialResolvedPackage> (
const finishingList: FinishingResolutionPromise[] = []
const peersCache = new Map<PkgIdWithPatchHash, PeersCacheItem[]>()
const purePkgs = new Set<PkgIdWithPatchHash>()
for (const { directNodeIdsByAlias, declaredDirectDependencies, explicitlyRequestedDirectDependencies, topParents, rootDir, id } of opts.projects) {
for (const { directNodeIdsByAlias, hoistedPeerProviderNodeIds, declaredDirectDependencies, explicitlyRequestedDirectDependencies, topParents, rootDir, id } of opts.projects) {
const currentProviderSources: CurrentProviderSource[] = [{
directNodeIdsByAlias,
declaredDirectDependencies: declaredDirectDependencies ?? new Set(),
@@ -145,10 +147,25 @@ export async function resolvePeers<T extends PartialResolvedPackage> (
}
}
// eslint-disable-next-line no-await-in-loop
const { finishing } = await resolvePeersOfChildren(Object.fromEntries(directNodeIdsByAlias.entries()), pkgsByName, {
// Hoisted peer providers stay visible as providers (via pkgsByName) but are
// not traversed as direct children: their nodeIds point into subtrees, and
// resolving them a second time in the project's root context would bind
// their peers to the project's own dependencies instead of the providers
// next to them in the tree, racing with the in-place resolution on
// pathsByNodeId and producing peer graphs that mix both contexts.
const ownDirectChildren: Record<string, NodeId> = {}
const hoistedProviderChildren: Record<string, NodeId> = {}
for (const [alias, nodeId] of directNodeIdsByAlias.entries()) {
if (hoistedPeerProviderNodeIds?.has(nodeId)) {
hoistedProviderChildren[alias] = nodeId
} else {
ownDirectChildren[alias] = nodeId
}
}
const parentPkgsOfNode: ParentPkgsOfNode = new Map()
const projectPeersContext = {
allPeerDepNames: opts.allPeerDepNames,
parentPkgsOfNode: new Map(),
parentPkgsOfNode,
dependenciesTree: opts.dependenciesTree,
depGraph,
lockfileDir: opts.lockfileDir,
@@ -168,10 +185,32 @@ export async function resolvePeers<T extends PartialResolvedPackage> (
rootDir,
virtualStoreDir: opts.virtualStoreDir,
virtualStoreDirMaxLength: opts.virtualStoreDirMaxLength,
})
}
// eslint-disable-next-line no-await-in-loop
const { finishing } = await resolvePeersOfChildren(ownDirectChildren, pkgsByName, projectPeersContext)
if (finishing) {
finishingList.push(finishing)
}
// A provider whose tree position was pruned from the traversal (its parent
// hit peersCache, so its children were never visited) still has consumers
// awaiting its dep path, so resolve it here as a last resort. Providers
// visited by the traversal above are recorded in parentPkgsOfNode.
// All pruned providers go into a single resolvePeersOfChildren call:
// its cycle analysis only sees the children of one call, and providers
// frequently peer-depend on each other, so resolving them one by one
// would leave their dep path calculations awaiting each other forever.
const prunedProviderChildren: Record<string, NodeId> = {}
for (const [alias, nodeId] of Object.entries(hoistedProviderChildren)) {
if (parentPkgsOfNode.has(nodeId)) continue
prunedProviderChildren[alias] = nodeId
}
if (Object.keys(prunedProviderChildren).length > 0) {
// eslint-disable-next-line no-await-in-loop
const { finishing } = await resolvePeersOfChildren(prunedProviderChildren, pkgsByName, projectPeersContext)
if (finishing) {
finishingList.push(finishing)
}
}
if (Object.keys(peerDependencyIssues.bad).length > 0 || Object.keys(peerDependencyIssues.missing).length > 0) {
peerDependencyIssuesByProjects[id] = {
...peerDependencyIssues,

View File

@@ -1717,3 +1717,147 @@ describe('dedupePeers', () => {
expect(peerDependencyIssuesByProjects['']?.missing?.['real-pkg']).toBeUndefined()
})
})
test('pruned hoisted peer providers that peer-depend on each other are resolved together', async () => {
// Hoisted peer providers whose tree position was never visited are resolved
// by a root-context fallback. Providers frequently peer-depend on each
// other, and each resolvePeersOfChildren call only detects peer cycles
// among its own children — so all pruned providers must be resolved in one
// call, or their dep path calculations await each other forever.
const libAPkg = {
name: 'lib-a',
pkgIdWithPatchHash: 'lib-a@1.0.0' as PkgIdWithPatchHash,
version: '1.0.0',
peerDependencies: {
'lib-b': { version: '^1.0.0' },
},
id: '' as PkgResolutionId,
}
const libBPkg = {
name: 'lib-b',
pkgIdWithPatchHash: 'lib-b@1.0.0' as PkgIdWithPatchHash,
version: '1.0.0',
peerDependencies: {
'lib-a': { version: '^1.0.0' },
},
id: '' as PkgResolutionId,
}
const consumerPkg = {
name: 'consumer',
pkgIdWithPatchHash: 'consumer@1.0.0' as PkgIdWithPatchHash,
version: '1.0.0',
peerDependencies: {
'lib-a': { version: '^1.0.0' },
'lib-b': { version: '^1.0.0' },
},
id: '' as PkgResolutionId,
}
const { dependenciesGraph, dependenciesByProjectId } = await resolvePeers({
allPeerDepNames: new Set(['lib-a', 'lib-b']),
projects: [
{
directNodeIdsByAlias: new Map([
['consumer', '>consumer@1.0.0>' as NodeId],
['lib-a', '>lib-a@1.0.0>' as NodeId],
['lib-b', '>lib-b@1.0.0>' as NodeId],
]),
hoistedPeerProviderNodeIds: new Set(['>lib-a@1.0.0>' as NodeId, '>lib-b@1.0.0>' as NodeId]),
topParents: [],
rootDir: '' as ProjectRootDir,
id: '.',
},
],
resolvedImporters: {},
dependenciesTree: new Map<NodeId, DependenciesTreeNode<PartialResolvedPackage>>([
['>consumer@1.0.0>' as NodeId, {
children: {},
installable: true,
resolvedPackage: consumerPkg,
depth: 0,
}],
['>lib-a@1.0.0>' as NodeId, {
children: {},
installable: true,
resolvedPackage: libAPkg,
depth: 1,
}],
['>lib-b@1.0.0>' as NodeId, {
children: {},
installable: true,
resolvedPackage: libBPkg,
depth: 1,
}],
]),
virtualStoreDir: '',
lockfileDir: '',
virtualStoreDirMaxLength: 120,
peersSuffixMaxLength: 1000,
workspaceProjectIds: new Set(),
})
expect(Object.keys(dependenciesGraph).sort()).toStrictEqual([
'consumer@1.0.0(lib-a@1.0.0)(lib-b@1.0.0)',
'lib-a@1.0.0(lib-b@1.0.0)',
'lib-b@1.0.0(lib-a@1.0.0)',
])
expect(dependenciesByProjectId['.'].get('lib-a')).toBe('lib-a@1.0.0(lib-b@1.0.0)')
expect(dependenciesByProjectId['.'].get('lib-b')).toBe('lib-b@1.0.0(lib-a@1.0.0)')
})
test('a pruned hoisted peer provider is resolved by the root-context fallback', async () => {
// Mirror of the pacquet test `pruned_hoisted_provider_falls_back_to_root_resolution`:
// a hoisted peer provider whose tree position was never visited must still
// get a dep path so the consumers that bound it can finish.
const provPkg = {
name: 'prov',
pkgIdWithPatchHash: 'prov@1.0.0' as PkgIdWithPatchHash,
version: '1.0.0',
peerDependencies: {} as PeerDependencies,
id: '' as PkgResolutionId,
}
const consumerPkg = {
name: 'consumer',
pkgIdWithPatchHash: 'consumer@1.0.0' as PkgIdWithPatchHash,
version: '1.0.0',
peerDependencies: {
prov: { version: '*' },
},
id: '' as PkgResolutionId,
}
const { dependenciesGraph, dependenciesByProjectId } = await resolvePeers({
allPeerDepNames: new Set(['prov']),
projects: [
{
directNodeIdsByAlias: new Map([
['consumer', '>consumer@1.0.0>' as NodeId],
['prov', '>prov@1.0.0>' as NodeId],
]),
hoistedPeerProviderNodeIds: new Set(['>prov@1.0.0>' as NodeId]),
topParents: [],
rootDir: '' as ProjectRootDir,
id: '.',
},
],
resolvedImporters: {},
dependenciesTree: new Map<NodeId, DependenciesTreeNode<PartialResolvedPackage>>([
['>consumer@1.0.0>' as NodeId, {
children: {},
installable: true,
resolvedPackage: consumerPkg,
depth: 0,
}],
['>prov@1.0.0>' as NodeId, {
children: {},
installable: true,
resolvedPackage: provPkg,
depth: 1,
}],
]),
virtualStoreDir: '',
lockfileDir: '',
virtualStoreDirMaxLength: 120,
peersSuffixMaxLength: 1000,
workspaceProjectIds: new Set(),
})
expect(dependenciesByProjectId['.'].get('prov')).toBe('prov@1.0.0')
expect(Object.keys(dependenciesGraph)).toContain('consumer@1.0.0(prov@1.0.0)')
})

View File

@@ -0,0 +1,8 @@
{
"name": "@pnpm.e2e/closure-lib-a",
"version": "1.0.0",
"peerDependencies": {
"@pnpm.e2e/closure-lib-b": "^1.0.0",
"@pnpm.e2e/closure-peer-x": "^1.0.0"
}
}

View File

@@ -0,0 +1,8 @@
{
"name": "@pnpm.e2e/closure-lib-b",
"version": "1.0.0",
"peerDependencies": {
"@pnpm.e2e/closure-lib-a": "^1.0.0",
"@pnpm.e2e/closure-peer-x": "^1.0.0"
}
}

View File

@@ -0,0 +1,4 @@
{
"name": "@pnpm.e2e/closure-peer-x",
"version": "1.0.0"
}

View File

@@ -0,0 +1,4 @@
{
"name": "@pnpm.e2e/closure-peer-x",
"version": "2.0.0"
}

View File

@@ -0,0 +1,9 @@
{
"name": "@pnpm.e2e/closure-plugins",
"version": "1.0.0",
"dependencies": {
"@pnpm.e2e/closure-lib-a": "1.0.0",
"@pnpm.e2e/closure-lib-b": "1.0.0",
"@pnpm.e2e/closure-peer-x": "1.0.0"
}
}