From 71a4934d3fb78945e97269fd018c74c32558e168 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 25 Jul 2026 20:37:28 +0200 Subject: [PATCH] fix(resolving-deps-resolver): apply overrides and the workspace root's pin to hoisted peers (#13369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two peer-hoisting fixes, in both the TypeScript CLI and pacquet. overrides now govern peers pnpm auto-installs. Overrides are applied by the read-package hook, which only sees manifests; a peer nobody declares never passes through one, so it resolved against its declared peer range and could introduce a second copy of the package the override pinned. The required-peer picker now consults the override set first. The per-edge lookup that backs the hook is factored out and shared, so the two paths cannot drift: hooks/read-package-hook exports createDependencyOverrider, and pacquet's VersionsOverrider grows override_for_undeclared_dependency. Parent-scoped overrides cannot apply (no parent manifest), and a convergence override still applies only when its version satisfies the edge's range. The convergence collector does not see these edges — a range no manifest declares would skew the staleness verdict. An override redirects a hoist rather than creating one: without autoInstallPeers, a peer neither the graph nor the workspace root provides stays uninstalled. The overrider is handed the directory of the importer the peer is hoisted into, so a link: / file: target stays relative to it. getHoistableOptionalPeers now bounds its candidates by the workspace root's own specifier for the package, the same input hoistPeers short-circuits on. It previously maximised over every version in the graph, so an optional peer declared as `*` — the shape peerDependenciesMeta produces — pulled the newest version any importer had resolved into a sibling that declares nothing, next to the version the root pins. The alias-then-package-name precedence for finding that root dependency is now one helper shared by both pickers, and the root's specifier is read through getPeerVersionRange so a workspace: range or an npm: alias bounds the candidates as well. Measured on vercel/next.js: the lockfile diff between the two stacks drops from 718 to 138 lines, the two stacks' resolved package key sets and every importer block now match, and the only remaining difference is three snapshot instances pacquet does not deduplicate. Related to pnpm/pnpm#13320. --- ...onal-peer-hoist-respects-workspace-root.md | 7 + .../overrides-reach-auto-installed-peers.md | 9 + .../src/install_with_fresh_lockfile.rs | 16 +- pnpm/crates/package-manager/src/overrides.rs | 31 +++ .../package-manager/src/overrides/tests.rs | 43 +++ .../src/hoist_peers.rs | 82 ++++-- .../src/hoist_peers/tests.rs | 254 +++++++++++++++++- .../crates/resolving-deps-resolver/src/lib.rs | 3 +- .../src/resolve_importer.rs | 20 +- .../src/resolve_importer/tests.rs | 1 + .../src/resolve_workspace/tests.rs | 1 + .../src/createVersionsOverrider.ts | 153 ++++++++--- pnpm11/hooks/read-package-hook/src/index.ts | 1 + .../test/createVersionOverrider.test.ts | 40 ++- .../src/getPeerDependencyIssues.ts | 3 +- .../deps-installer/src/install/index.ts | 2 + .../deps-resolver/src/hoistPeers.ts | 72 ++++- .../deps-resolver/src/resolveDependencies.ts | 19 +- .../src/resolveDependencyTree.ts | 2 + .../deps-resolver/test/hoistPeers.test.ts | 133 +++++++++ 20 files changed, 803 insertions(+), 89 deletions(-) create mode 100644 .changeset/optional-peer-hoist-respects-workspace-root.md create mode 100644 .changeset/overrides-reach-auto-installed-peers.md diff --git a/.changeset/optional-peer-hoist-respects-workspace-root.md b/.changeset/optional-peer-hoist-respects-workspace-root.md new file mode 100644 index 0000000000..b4f96c44a1 --- /dev/null +++ b/.changeset/optional-peer-hoist-respects-workspace-root.md @@ -0,0 +1,7 @@ +--- +"@pnpm/installing.deps-resolver": patch +"pacquet": patch +"pnpm": patch +--- + +An auto-installed *optional* peer is no longer hoisted at a version the workspace root's own dependency on that package excludes. `resolvePeersFromWorkspaceRoot` already made the workspace root's specifier decide which version a missing *required* peer is installed at; the optional-peer picker ignored it and always took the highest version present anywhere in the graph. In a workspace whose root pins `postcss: 8.5.10`, an importer that depends on `webpack` and declares no `postcss` of its own got `postcss@8.5.22` hoisted for `terser-webpack-plugin`'s optional `postcss` peer, leaving two `postcss@8.5.x` instances in the graph [#13320](https://github.com/pnpm/pnpm/issues/13320). diff --git a/.changeset/overrides-reach-auto-installed-peers.md b/.changeset/overrides-reach-auto-installed-peers.md new file mode 100644 index 0000000000..876fccee0d --- /dev/null +++ b/.changeset/overrides-reach-auto-installed-peers.md @@ -0,0 +1,9 @@ +--- +"@pnpm/hooks.read-package-hook": minor +"@pnpm/installing.deps-resolver": minor +"@pnpm/installing.deps-installer": patch +"pacquet": patch +"pnpm": patch +--- + +`overrides` now also govern peers that pnpm auto-installs. Previously an override only rewrote dependencies declared in a manifest, so a peer nobody declares — installed because `autoInstallPeers` is on — resolved against its declared peer range and could bring in a second copy of the very package the override pinned. For example, with `overrides: { react: npm:react@19.2.0 }` and a lone `lucide-react` dependency, pnpm installed `react@18.3.1`; it now installs the pinned `react@19.2.0` [#13320](https://github.com/pnpm/pnpm/issues/13320). diff --git a/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs b/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs index ad0aa018f6..ec472826a3 100644 --- a/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs +++ b/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs @@ -29,8 +29,8 @@ use pacquet_reporter::{ }; use pacquet_resolving_default_resolver::DefaultResolver; use pacquet_resolving_deps_resolver::{ - ManifestHook, ResolveDependencyTreeError, ResolveImporterError, ResolveImporterOptions, - UpdateDepth, + DependencyOverrider, ManifestHook, ResolveDependencyTreeError, ResolveImporterError, + ResolveImporterOptions, UpdateDepth, }; use pacquet_resolving_git_resolver::{GitFetchContext, GitResolver, RealGitProbe, RealGitRunner}; use pacquet_resolving_local_resolver::{ @@ -1141,6 +1141,17 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { compose_manifest_hooks(compat_package_extensions_hook, package_extensions_hook), overrides_hook, ); + let override_bare_specifier: Option> = + versions_overrider.as_ref().and_then(|overrider| { + if overrider.is_empty() { + None + } else { + let overrider = Arc::clone(overrider); + Some(Arc::new(move |name: &str, range: &str, pkg_dir: &Path| { + overrider.override_for_undeclared_dependency(name, range, pkg_dir) + }) as Arc) + } + }); // Seed `allPreferredVersions` from every importer's manifest + // the wanted lockfile's snapshots (when an existing one is @@ -1418,6 +1429,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { // The per-importer hoist loop mutates its own copy, so // clone the shared seed's map here (deref past the `Arc`). all_preferred_versions: importer_preferred_versions.as_ref().clone(), + override_bare_specifier: override_bare_specifier.clone(), patched_dependencies: patched_dependencies.clone(), // `pick_lowest_direct` / `subdep_published_by` are // authoritative from `resolve_workspace` (it computes diff --git a/pnpm/crates/package-manager/src/overrides.rs b/pnpm/crates/package-manager/src/overrides.rs index 99b38149bf..414661ae85 100644 --- a/pnpm/crates/package-manager/src/overrides.rs +++ b/pnpm/crates/package-manager/src/overrides.rs @@ -368,6 +368,37 @@ impl VersionsOverrider { } } + /// Resolve the specifier the override set imposes on a dependency + /// edge that has no declaring manifest — a peer pnpm auto-installs. + /// `"-"` means the edge is dropped. Parent-scoped overrides never + /// apply: there is no parent manifest to match them against. + /// `pkg_dir` is the directory of the package the edge is added to, so + /// a `link:` / `file:` target stays relative to it instead of + /// hard-coding this machine's layout into the lockfile. + /// + /// Such an edge never reaches [`Self::apply`], so the convergence + /// collector must not see it either — a range no manifest declares + /// would skew the staleness verdict. + #[must_use] + pub fn override_for_undeclared_dependency( + &self, + dep_name: &str, + dep_spec: &str, + pkg_dir: &Path, + ) -> Option { + if let Some(chosen) = self.choose_override(&[], dep_name, dep_spec) { + if chosen.inner.new_bare_specifier == "-" { + return Some("-".to_string()); + } + return Some(chosen.local_target.as_ref().map_or_else( + || chosen.inner.new_bare_specifier.clone(), + |target| resolve_local_override_spec(target, Some(pkg_dir)), + )); + } + self.converge_applies(dep_name, dep_spec) + .then(|| self.converge[dep_name].new_bare_specifier.clone()) + } + fn choose_override<'b>( &'b self, applicable_parent_scoped: &[&'b ResolvedOverride], diff --git a/pnpm/crates/package-manager/src/overrides/tests.rs b/pnpm/crates/package-manager/src/overrides/tests.rs index 93cf14a32e..4450770b4b 100644 --- a/pnpm/crates/package-manager/src/overrides/tests.rs +++ b/pnpm/crates/package-manager/src/overrides/tests.rs @@ -24,6 +24,12 @@ fn dep_spec<'a>(manifest: &'a PackageManifest, group: &str, name: &str) -> Optio manifest.value().get(group)?.get(name)?.as_str() } +/// [`VersionsOverrider::override_for_undeclared_dependency`] for an edge +/// added to `/workspace/pkg`. +fn undeclared(overrider: &VersionsOverrider, name: &str, spec: &str) -> Option { + overrider.override_for_undeclared_dependency(name, spec, Path::new("/workspace/pkg")) +} + #[test] fn generic_override_rewrites_dependencies_spec() { let overrides = parsed(&[("foo", "1.0.0")]); @@ -452,3 +458,40 @@ fn apply_to_arc_clones_when_only_a_peer_matches() { Some(">=8.18.0"), ); } + +#[test] +fn override_for_undeclared_dependency_applies_generic_overrides() { + let overrides = parsed(&[("react", "npm:react@19.2.0"), ("zoo@^1", "1.0.0")]); + let overrider = VersionsOverrider::new(&overrides, Path::new("/workspace")); + + assert_eq!(undeclared(&overrider, "react", "^18.0.0").as_deref(), Some("npm:react@19.2.0")); + assert_eq!(undeclared(&overrider, "zoo", "^1.5.0").as_deref(), Some("1.0.0")); + assert_eq!(undeclared(&overrider, "zoo", "^2.0.0"), None); + assert_eq!(undeclared(&overrider, "qar", "^1.0.0"), None); +} + +#[test] +fn override_for_undeclared_dependency_resolves_a_local_target_relative_to_the_package_dir() { + let overrides = parsed(&[("qar", "link:../qar")]); + let overrider = VersionsOverrider::new(&overrides, Path::new("/workspace")); + + assert_eq!(undeclared(&overrider, "qar", "^1.0.0").as_deref(), Some("link:../../qar")); +} + +#[test] +fn override_for_undeclared_dependency_ignores_parent_scoped_overrides() { + let overrides = parsed(&[("foo>react", "19.2.0")]); + let overrider = VersionsOverrider::new(&overrides, Path::new("/workspace")); + + assert_eq!(undeclared(&overrider, "react", "^18.0.0"), None); +} + +#[test] +fn override_for_undeclared_dependency_applies_converge_only_within_range() { + let overrides = parsed(&[("react@", "18.3.1")]); + let overrider = VersionsOverrider::new(&overrides, Path::new("/workspace")); + + assert_eq!(undeclared(&overrider, "react", "^18.0.0").as_deref(), Some("18.3.1")); + assert_eq!(undeclared(&overrider, "react", "^19.0.0"), None); + assert!(overrider.converge_declared_ranges().is_empty()); +} diff --git a/pnpm/crates/resolving-deps-resolver/src/hoist_peers.rs b/pnpm/crates/resolving-deps-resolver/src/hoist_peers.rs index 3d0f9f11a2..1ba44b7aa0 100644 --- a/pnpm/crates/resolving-deps-resolver/src/hoist_peers.rs +++ b/pnpm/crates/resolving-deps-resolver/src/hoist_peers.rs @@ -2,7 +2,7 @@ //! "what to add to the importer's direct deps" map. Used by the //! orchestrator (`resolve_importer`) inside its hoist loop. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, path::Path}; use node_semver::{Range, Version}; use pacquet_resolving_resolver_base::{ @@ -30,12 +30,26 @@ pub struct MissingPeerInfo { pub range: String, } +/// Applies `overrides` to a peer nobody declares as a dependency. +/// Such a peer has no manifest for the override hook to rewrite, so +/// without this it would resolve against its declared peer range and +/// silently produce the second copy the override exists to prevent. +/// Returns the overriding specifier, `"-"` when the override drops the +/// peer, or `None` when no override claims it. The path is the directory +/// of the importer the peer is hoisted into — the manifest that would +/// have declared the dependency — which a `link:` / `file:` override +/// target is made relative to. +pub type DependencyOverrider = dyn Fn(&str, &str, &Path) -> Option + Send + Sync; + /// Options for [`hoist_peers`]. -#[derive(Debug)] pub struct HoistPeersOptions<'a> { pub auto_install_peers: bool, pub all_preferred_versions: &'a PreferredVersions, pub workspace_root_deps: &'a [WorkspaceRootDep], + pub override_bare_specifier: Option<&'a DependencyOverrider>, + /// Directory of the importer the peers are hoisted into. Only read + /// to resolve a local override target; see [`DependencyOverrider`]. + pub project_dir: &'a Path, } /// Pick a specifier for each missing required peer. Returns a map of @@ -50,23 +64,27 @@ pub fn hoist_peers( for (peer_name, info) in missing_required_peers { let range = &info.range; - if let Some(dep) = - opts.workspace_root_deps.iter().find(|root_dep| &root_dep.alias == peer_name) - && let Some(spec) = &dep.normalized_bare_specifier + let root_bare_specifier = find_workspace_root_dep(opts.workspace_root_deps, peer_name) + .and_then(|dep| dep.normalized_bare_specifier.as_ref()); + // An override redirects a hoist; it must never create one, or + // disabling auto-install-peers would still install a peer nobody + // depends on. Only the workspace root's own dependency hoists a + // peer that auto-install-peers is not asking for, so that is the + // one hoist an override still governs here; the deduplication + // below installs nothing new either way. + let overrider = (opts.auto_install_peers || root_bare_specifier.is_some()) + .then_some(opts.override_bare_specifier) + .flatten(); + if let Some(overridden) = + overrider.and_then(|overrider| overrider(peer_name, range, opts.project_dir)) { - dependencies.insert(peer_name.clone(), spec.clone()); + if overridden != "-" { + dependencies.insert(peer_name.clone(), overridden); + } continue; } - let mut by_pkg_name: Vec<&WorkspaceRootDep> = opts - .workspace_root_deps - .iter() - .filter(|root_dep| &root_dep.pkg_name == peer_name) - .collect(); - by_pkg_name.sort_by(|a, b| a.alias.cmp(&b.alias)); - if let Some(dep) = by_pkg_name.first() - && let Some(spec) = &dep.normalized_bare_specifier - { + if let Some(spec) = root_bare_specifier { dependencies.insert(peer_name.clone(), spec.clone()); continue; } @@ -144,10 +162,22 @@ pub fn hoist_peers( pub fn get_hoistable_optional_peers( all_missing_optional_peers: &BTreeMap>, all_preferred_versions: &PreferredVersions, + workspace_root_deps: &[WorkspaceRootDep], ) -> BTreeMap { let mut optional_dependencies = BTreeMap::new(); for (peer_name, ranges) in all_missing_optional_peers { let Some(selectors) = all_preferred_versions.get(peer_name) else { continue }; + // The workspace root's own specifier bounds the candidates the + // same way it short-circuits `hoist_peers` above. Maximizing over + // every version in the graph instead lets one importer's newer + // resolution be hoisted into a sibling that declares nothing, + // adding a second instance of a package the root already pins. A + // scheme specifier bounds them through the version body + // `get_peer_version_range` extracts; one with no version body + // yields `*` and leaves them unbounded. + let root_range = find_workspace_root_dep(workspace_root_deps, peer_name) + .and_then(|dep| dep.normalized_bare_specifier.as_deref()) + .and_then(|spec| get_peer_version_range(spec).parse::().ok()); // An unparsable range is satisfied by nothing, so bailing on the // peer matches failing the check per candidate. let Ok(parsed_ranges) = @@ -165,6 +195,9 @@ pub fn get_hoistable_optional_peers( continue; } let Ok(version) = version_str.parse::() else { continue }; + if root_range.as_ref().is_some_and(|range| !range.satisfies(&version)) { + continue; + } // Strict, unlike the required-peer picker above: an optional // peer nobody declared is installed only to deduplicate, so a // prerelease its range rejects is not worth splitting a @@ -183,6 +216,25 @@ pub fn get_hoistable_optional_peers( optional_dependencies } +/// The root dependency that provides `peer_name`: an alias match wins +/// over a package-name match (an `npm:` alias can install the same +/// package under a different slot), and among package-name matches the +/// lexicographically first alias wins so the pick is stable. Only a +/// dependency that has a normalized specifier is a candidate — the +/// callers have nothing to install or bound the peer with otherwise. +fn find_workspace_root_dep<'a>( + workspace_root_deps: &'a [WorkspaceRootDep], + peer_name: &str, +) -> Option<&'a WorkspaceRootDep> { + let candidates = + || workspace_root_deps.iter().filter(|dep| dep.normalized_bare_specifier.is_some()); + candidates().find(|root_dep| root_dep.alias == peer_name).or_else(|| { + candidates() + .filter(|root_dep| root_dep.pkg_name == peer_name) + .min_by(|a, b| a.alias.cmp(&b.alias)) + }) +} + /// Highest version from `versions` that satisfies `range`, including /// prereleases. Returns `None` if no candidate satisfies. fn max_satisfying<'a>(versions: &'a [&'a str], range: &str) -> Option<&'a str> { diff --git a/pnpm/crates/resolving-deps-resolver/src/hoist_peers/tests.rs b/pnpm/crates/resolving-deps-resolver/src/hoist_peers/tests.rs index 6bec3195fb..7cd3a65c3d 100644 --- a/pnpm/crates/resolving-deps-resolver/src/hoist_peers/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/hoist_peers/tests.rs @@ -1,14 +1,16 @@ //! Tests for [`super::hoist_peers`] and //! [`super::get_hoistable_optional_peers`]. -use std::collections::BTreeMap; +use std::{collections::BTreeMap, path::Path}; use pacquet_resolving_resolver_base::{ PreferredVersions, VersionSelectorEntry, VersionSelectorType, VersionSelectorWithWeight, }; use pretty_assertions::assert_eq; -use super::{HoistPeersOptions, MissingPeerInfo, get_hoistable_optional_peers, hoist_peers}; +use super::{ + HoistPeersOptions, MissingPeerInfo, WorkspaceRootDep, get_hoistable_optional_peers, hoist_peers, +}; fn preferred(entries: &[(&str, &[(&str, VersionSelectorEntry)])]) -> PreferredVersions { let mut map = PreferredVersions::new(); @@ -34,7 +36,13 @@ fn opts( auto_install_peers: bool, all_preferred_versions: &PreferredVersions, ) -> HoistPeersOptions<'_> { - HoistPeersOptions { auto_install_peers, all_preferred_versions, workspace_root_deps: &[] } + HoistPeersOptions { + auto_install_peers, + all_preferred_versions, + workspace_root_deps: &[], + override_bare_specifier: None, + project_dir: Path::new("/workspace"), + } } #[test] @@ -237,7 +245,7 @@ fn get_hoistable_optional_peers_picks_a_version_that_satisfies_all_optional_rang )]); let mut missing = BTreeMap::new(); missing.insert("foo".to_string(), vec!["2".to_string(), "2.1".to_string()]); - let result = get_hoistable_optional_peers(&missing, &preferred); + let result = get_hoistable_optional_peers(&missing, &preferred, &[]); let mut expected = BTreeMap::new(); expected.insert("foo".to_string(), "2.1.0".to_string()); assert_eq!(result, expected); @@ -254,7 +262,7 @@ fn get_hoistable_optional_peers_picks_the_highest_satisfying_version() { )]); let mut missing = BTreeMap::new(); missing.insert("foo".to_string(), vec!["2".to_string(), "2.1".to_string()]); - let result = get_hoistable_optional_peers(&missing, &preferred); + let result = get_hoistable_optional_peers(&missing, &preferred, &[]); let mut expected = BTreeMap::new(); expected.insert("foo".to_string(), "2.1.1".to_string()); assert_eq!(result, expected); @@ -277,7 +285,7 @@ fn get_hoistable_optional_peers_handles_version_selector_with_weight() { )]); let mut missing = BTreeMap::new(); missing.insert("jsdom".to_string(), vec!["*".to_string()]); - let result = get_hoistable_optional_peers(&missing, &preferred); + let result = get_hoistable_optional_peers(&missing, &preferred, &[]); let mut expected = BTreeMap::new(); expected.insert("jsdom".to_string(), "27.4.0".to_string()); assert_eq!(result, expected); @@ -299,7 +307,7 @@ fn get_hoistable_optional_peers_rejects_prerelease_against_non_prerelease_range( preferred(&[("react", &[("18.0.0-rc.1", plain(VersionSelectorType::Version))])]); let mut missing = BTreeMap::new(); missing.insert("react".to_string(), vec!["^18.0.0".to_string()]); - let result = get_hoistable_optional_peers(&missing, &preferred); + let result = get_hoistable_optional_peers(&missing, &preferred, &[]); assert_eq!(result, BTreeMap::new()); } @@ -314,8 +322,238 @@ fn get_hoistable_optional_peers_rejects_prerelease_within_the_range_span() { )]); let mut missing = BTreeMap::new(); missing.insert("jest-util".to_string(), vec!["^29.0.0 || ^30.0.0".to_string()]); - let result = get_hoistable_optional_peers(&missing, &preferred); + let result = get_hoistable_optional_peers(&missing, &preferred, &[]); let mut expected = BTreeMap::new(); expected.insert("jest-util".to_string(), "29.7.0".to_string()); assert_eq!(result, expected); } + +#[test] +fn installs_auto_installed_peer_at_the_overridden_specifier() { + let preferred = preferred(&[("react", &[("18.3.1", plain(VersionSelectorType::Version))])]); + let root_deps = [WorkspaceRootDep { + alias: "react".to_string(), + pkg_name: "react".to_string(), + normalized_bare_specifier: Some("18.3.1".to_string()), + }]; + let overrider = |name: &str, _range: &str, _pkg_dir: &Path| { + (name == "react").then(|| "npm:react@19.2.0".to_string()) + }; + let opts = HoistPeersOptions { + auto_install_peers: true, + all_preferred_versions: &preferred, + workspace_root_deps: &root_deps, + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("react", "^16.5.1 || ^17.0.0 || ^18.0.0")]); + let mut expected = BTreeMap::new(); + expected.insert("react".to_string(), "npm:react@19.2.0".to_string()); + assert_eq!(result, expected); +} + +#[test] +fn override_does_not_install_a_peer_nothing_provides_without_auto_install_peers() { + let preferred = PreferredVersions::new(); + let overrider = + |_name: &str, _range: &str, _pkg_dir: &Path| Some("npm:react@19.2.0".to_string()); + let opts = HoistPeersOptions { + auto_install_peers: false, + all_preferred_versions: &preferred, + workspace_root_deps: &[], + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("react", "^18.0.0")]); + assert_eq!(result, BTreeMap::new()); +} + +#[test] +fn leaves_a_deduplicating_hoist_to_the_graph_without_auto_install_peers() { + let preferred = preferred(&[("react", &[("18.3.1", plain(VersionSelectorType::Version))])]); + let overrider = + |_name: &str, _range: &str, _pkg_dir: &Path| Some("npm:react@19.2.0".to_string()); + let opts = HoistPeersOptions { + auto_install_peers: false, + all_preferred_versions: &preferred, + workspace_root_deps: &[], + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("react", "^18.0.0")]); + let mut expected = BTreeMap::new(); + expected.insert("react".to_string(), "18.3.1".to_string()); + assert_eq!(result, expected); +} + +#[test] +fn override_redirects_the_workspace_roots_hoist_without_auto_install_peers() { + let empty = PreferredVersions::new(); + let root_deps = [WorkspaceRootDep { + alias: "react".to_string(), + pkg_name: "react".to_string(), + normalized_bare_specifier: Some("18.3.1".to_string()), + }]; + let overrider = + |_name: &str, _range: &str, _pkg_dir: &Path| Some("npm:react@19.2.0".to_string()); + let opts = HoistPeersOptions { + auto_install_peers: false, + all_preferred_versions: &empty, + workspace_root_deps: &root_deps, + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("react", "^18.0.0")]); + let mut expected = BTreeMap::new(); + expected.insert("react".to_string(), "npm:react@19.2.0".to_string()); + assert_eq!(result, expected); +} + +#[test] +fn passes_the_importer_directory_to_the_overrider() { + let preferred = preferred(&[("react", &[("18.3.1", plain(VersionSelectorType::Version))])]); + let overrider = + |_name: &str, _range: &str, pkg_dir: &Path| Some(format!("link:{}", pkg_dir.display())); + let opts = HoistPeersOptions { + auto_install_peers: true, + all_preferred_versions: &preferred, + workspace_root_deps: &[], + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace/packages/app"), + }; + let result = hoist_peers(&opts, &[missing("react", "^18.0.0")]); + let mut expected = BTreeMap::new(); + expected.insert("react".to_string(), "link:/workspace/packages/app".to_string()); + assert_eq!(result, expected); +} + +#[test] +fn leaves_peer_removed_by_an_override_uninstalled() { + let preferred = preferred(&[("react", &[("18.3.1", plain(VersionSelectorType::Version))])]); + let overrider = |_name: &str, _range: &str, _pkg_dir: &Path| Some("-".to_string()); + let opts = HoistPeersOptions { + auto_install_peers: true, + all_preferred_versions: &preferred, + workspace_root_deps: &[], + override_bare_specifier: Some(&overrider), + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("react", "^18.0.0")]); + assert_eq!(result, BTreeMap::new()); +} + +#[test] +fn get_hoistable_optional_peers_stays_within_the_workspace_roots_range() { + let preferred = preferred(&[( + "postcss", + &[ + ("8.5.10", plain(VersionSelectorType::Version)), + ("8.5.22", plain(VersionSelectorType::Version)), + ], + )]); + let mut missing = BTreeMap::new(); + missing.insert("postcss".to_string(), vec!["*".to_string()]); + let root_deps = [WorkspaceRootDep { + alias: "postcss".to_string(), + pkg_name: "postcss".to_string(), + normalized_bare_specifier: Some("8.5.10".to_string()), + }]; + + let result = get_hoistable_optional_peers(&missing, &preferred, &root_deps); + let mut expected = BTreeMap::new(); + expected.insert("postcss".to_string(), "8.5.10".to_string()); + assert_eq!(result, expected); + + let unbounded = get_hoistable_optional_peers(&missing, &preferred, &[]); + let mut expected_unbounded = BTreeMap::new(); + expected_unbounded.insert("postcss".to_string(), "8.5.22".to_string()); + assert_eq!(unbounded, expected_unbounded); +} + +#[test] +fn skips_a_workspace_root_dep_without_a_specifier_in_favor_of_one_with() { + let root_deps = [ + WorkspaceRootDep { + alias: "postcss".to_string(), + pkg_name: "postcss".to_string(), + normalized_bare_specifier: None, + }, + WorkspaceRootDep { + alias: "zz-postcss".to_string(), + pkg_name: "postcss".to_string(), + normalized_bare_specifier: Some("8.5.10".to_string()), + }, + ]; + let empty = PreferredVersions::new(); + let opts = HoistPeersOptions { + auto_install_peers: true, + all_preferred_versions: &empty, + workspace_root_deps: &root_deps, + override_bare_specifier: None, + project_dir: Path::new("/workspace"), + }; + let result = hoist_peers(&opts, &[missing("postcss", "^8.0.0")]); + let mut expected = BTreeMap::new(); + expected.insert("postcss".to_string(), "8.5.10".to_string()); + assert_eq!(result, expected); + + let preferred = preferred(&[( + "postcss", + &[ + ("8.5.10", plain(VersionSelectorType::Version)), + ("9.0.0", plain(VersionSelectorType::Version)), + ], + )]); + let mut missing_optional = BTreeMap::new(); + missing_optional.insert("postcss".to_string(), vec!["*".to_string()]); + let optional = get_hoistable_optional_peers(&missing_optional, &preferred, &root_deps); + assert_eq!(optional, expected); +} + +#[test] +fn get_hoistable_optional_peers_bounds_by_a_scheme_prefixed_workspace_root_specifier() { + let preferred = preferred(&[( + "postcss", + &[ + ("8.5.10", plain(VersionSelectorType::Version)), + ("9.0.0", plain(VersionSelectorType::Version)), + ], + )]); + let mut missing = BTreeMap::new(); + missing.insert("postcss".to_string(), vec!["*".to_string()]); + for spec in ["workspace:^8.5.10", "npm:postcss@^8.5.10", "work:^8.5.10"] { + let root_deps = [WorkspaceRootDep { + alias: "postcss".to_string(), + pkg_name: "postcss".to_string(), + normalized_bare_specifier: Some(spec.to_string()), + }]; + let result = get_hoistable_optional_peers(&missing, &preferred, &root_deps); + let mut expected = BTreeMap::new(); + expected.insert("postcss".to_string(), "8.5.10".to_string()); + dbg!(spec); + assert_eq!(result, expected); + } +} + +#[test] +fn get_hoistable_optional_peers_stays_unbounded_when_the_root_specifier_has_no_version() { + let preferred = preferred(&[( + "postcss", + &[ + ("8.5.10", plain(VersionSelectorType::Version)), + ("9.0.0", plain(VersionSelectorType::Version)), + ], + )]); + let mut missing = BTreeMap::new(); + missing.insert("postcss".to_string(), vec!["*".to_string()]); + let root_deps = [WorkspaceRootDep { + alias: "postcss".to_string(), + pkg_name: "postcss".to_string(), + normalized_bare_specifier: Some("file:../postcss".to_string()), + }]; + + let result = get_hoistable_optional_peers(&missing, &preferred, &root_deps); + let mut expected = BTreeMap::new(); + expected.insert("postcss".to_string(), "9.0.0".to_string()); + assert_eq!(result, expected); +} diff --git a/pnpm/crates/resolving-deps-resolver/src/lib.rs b/pnpm/crates/resolving-deps-resolver/src/lib.rs index 32c36552cb..cb76e56520 100644 --- a/pnpm/crates/resolving-deps-resolver/src/lib.rs +++ b/pnpm/crates/resolving-deps-resolver/src/lib.rs @@ -80,7 +80,8 @@ pub use dependencies_graph::{ PeerDependencyIssues, }; pub use hoist_peers::{ - HoistPeersOptions, MissingPeerInfo, WorkspaceRootDep, get_hoistable_optional_peers, hoist_peers, + DependencyOverrider, HoistPeersOptions, MissingPeerInfo, WorkspaceRootDep, + get_hoistable_optional_peers, hoist_peers, }; pub use node_id::NodeId; pub use pacquet_deps_path::DepPath; diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs index 650d123929..5b5e5186a3 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -25,8 +25,8 @@ use crate::{ DirectDep, dependencies_graph::MissingPeer, hoist_peers::{ - HoistPeersOptions, MissingPeerInfo, WorkspaceRootDep, get_hoistable_optional_peers, - hoist_peers, + DependencyOverrider, HoistPeersOptions, MissingPeerInfo, WorkspaceRootDep, + get_hoistable_optional_peers, hoist_peers, }, resolve_dependency_tree::{ ResolveDependencyTreeError, TreeCtx, WantedSpec, WorkspaceTreeCtx, extend_tree, @@ -88,6 +88,10 @@ pub struct ResolveImporterOptions { /// lockfile + manifest seeding is available. pub all_preferred_versions: PreferredVersions, + /// Applies `overrides` to auto-installed peers. See + /// [`crate::DependencyOverrider`]. + pub override_bare_specifier: Option>, + /// Configured `patchedDependencies`, grouped by package name. The /// tree walker appends `(patch_hash=)` to each matched /// package's `pkgIdWithPatchHash` and records the matched key on @@ -164,6 +168,10 @@ impl std::fmt::Debug for ResolveImporterOptions { .field("resolve_peers_from_workspace_root", &self.resolve_peers_from_workspace_root) .field("dedupe_peers", &self.dedupe_peers) .field("all_preferred_versions", &self.all_preferred_versions) + .field( + "override_bare_specifier", + &self.override_bare_specifier.as_ref().map(|_| ""), + ) .field("patched_dependencies", &self.patched_dependencies) .field("base_opts", &self.base_opts) .field("pick_lowest_direct", &self.pick_lowest_direct) @@ -302,6 +310,7 @@ pub(crate) struct ImporterHoistState { parent_pkg_aliases: HashSet, all_missing_optional_peers: BTreeMap>, all_preferred_versions: PreferredVersions, + override_bare_specifier: Option>, auto_install_peers: bool, auto_install_peers_from_highest_match: bool, resolve_peers_from_workspace_root: bool, @@ -335,6 +344,7 @@ impl ImporterHoistState { resolve_peers_from_workspace_root, dedupe_peers, all_preferred_versions, + override_bare_specifier, patched_dependencies, base_opts, pick_lowest_direct, @@ -386,6 +396,7 @@ impl ImporterHoistState { parent_pkg_aliases, all_missing_optional_peers: BTreeMap::new(), all_preferred_versions, + override_bare_specifier, auto_install_peers, auto_install_peers_from_highest_match, resolve_peers_from_workspace_root, @@ -509,6 +520,8 @@ impl ImporterHoistState { auto_install_peers: self.auto_install_peers, all_preferred_versions: &self.all_preferred_versions, workspace_root_deps, + override_bare_specifier: self.override_bare_specifier.as_deref(), + project_dir: &self.project_dir, }, &missing_as_pairs, ); @@ -581,9 +594,12 @@ impl ImporterHoistState { if self.all_missing_optional_peers.is_empty() { return Ok(false); } + let workspace_root_deps: &[WorkspaceRootDep] = + if self.resolve_peers_from_workspace_root { &self.workspace_root_deps } else { &[] }; let hoisted_optional = get_hoistable_optional_peers( &self.all_missing_optional_peers, &self.all_preferred_versions, + workspace_root_deps, ); if hoisted_optional.is_empty() { return Ok(false); diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs index afda308ca4..9956c071ad 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_importer/tests.rs @@ -98,6 +98,7 @@ fn default_opts() -> ResolveImporterOptions { resolve_peers_from_workspace_root: false, dedupe_peers: false, all_preferred_versions: PreferredVersions::new(), + override_bare_specifier: None, patched_dependencies: None, base_opts: ResolveOptions::default(), pick_lowest_direct: false, diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs index 0a694ed041..30e8625235 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs @@ -174,6 +174,7 @@ fn importer_opts( resolve_peers_from_workspace_root: false, dedupe_peers: false, all_preferred_versions: PreferredVersions::new(), + override_bare_specifier: None, patched_dependencies: None, base_opts: ResolveOptions { published_by, project_dir, ..ResolveOptions::default() }, pick_lowest_direct: false, diff --git a/pnpm11/hooks/read-package-hook/src/createVersionsOverrider.ts b/pnpm11/hooks/read-package-hook/src/createVersionsOverrider.ts index f6254bdc64..bc50429ad5 100644 --- a/pnpm11/hooks/read-package-hook/src/createVersionsOverrider.ts +++ b/pnpm11/hooks/read-package-hook/src/createVersionsOverrider.ts @@ -22,19 +22,44 @@ export interface CreateVersionsOverriderOptions { convergeDeclaredRanges?: Map> } +/** + * Resolves the specifier an override imposes on a single dependency edge, or + * `undefined` when no override claims it. `'-'` means the edge is removed. + * + * Edges that have no declaring manifest — a peer pnpm auto-installs — reach + * the overrides through this function instead of through the read-package + * hook, so parent-scoped overrides (`parent>child`) never apply to them. + */ +export type DependencyOverrider = (name: string, bareSpecifier: string, dir?: string) => string | undefined + +/** + * `undefined` when no override in the set could ever claim an undeclared + * dependency, so the resolver skips the per-peer call in the common case of a + * project with no overrides — or with parent-scoped ones only. + */ +export function createDependencyOverrider ( + overrides: VersionOverrideWithoutRawSelector[], + rootDir: string +): DependencyOverrider | undefined { + const { genericVersionOverrides, convergeVersions } = splitOverrides(overrides, rootDir) + if (genericVersionOverrides.length === 0 && convergeVersions.size === 0) return undefined + return (name, bareSpecifier, dir) => { + const versionOverride = pickVersionOverride({ versionOverrides: [], genericVersionOverrides }, name, bareSpecifier) + if (versionOverride) { + return versionOverride.newBareSpecifier === '-' + ? '-' + : resolveOverriddenBareSpecifier(versionOverride, dir) + } + return convergeBareSpecifier(convergeVersions, name, bareSpecifier) + } +} + export function createVersionsOverrider ( overrides: VersionOverrideWithoutRawSelector[], rootDir: string, opts?: CreateVersionsOverriderOptions ): ReadPackageHook { - const [convergeOverrides, explicitOverrides] = partition(({ converge }) => converge === true, overrides) - const [versionOverrides, genericVersionOverrides] = partition(({ parentPkg }) => parentPkg != null, - explicitOverrides.map((override) => ({ - ...override, - localTarget: createLocalTarget(override, rootDir), - })) - ) as [VersionOverrideWithParent[], VersionOverride[]] - const convergeVersions = new Map(convergeOverrides.map((override) => [override.targetPkg.name, override.newBareSpecifier])) + const { versionOverrides, genericVersionOverrides, convergeVersions } = splitOverrides(overrides, rootDir) return ((manifest: PackageManifest, dir?: string) => { const versionOverridesWithParent = versionOverrides.filter(({ parentPkg }) => { return ( @@ -51,6 +76,25 @@ export function createVersionsOverrider ( }) as ReadPackageHook } +function splitOverrides (overrides: VersionOverrideWithoutRawSelector[], rootDir: string): { + versionOverrides: VersionOverrideWithParent[] + genericVersionOverrides: VersionOverride[] + convergeVersions: Map +} { + const [convergeOverrides, explicitOverrides] = partition(({ converge }) => converge === true, overrides) + const [versionOverrides, genericVersionOverrides] = partition(({ parentPkg }) => parentPkg != null, + explicitOverrides.map((override) => ({ + ...override, + localTarget: createLocalTarget(override, rootDir), + })) + ) as [VersionOverrideWithParent[], VersionOverride[]] + return { + versionOverrides, + genericVersionOverrides, + convergeVersions: new Map(convergeOverrides.map((override) => [override.targetPkg.name, override.newBareSpecifier])), + } +} + interface LocalTarget { protocol: LocalProtocol absolutePath: string @@ -117,19 +161,7 @@ function overrideDeps ( peerDeps: Dependencies | undefined ): void { for (const [name, bareSpecifier] of Object.entries(peerDeps ?? deps)) { - const versionOverride = - pickMostSpecificVersionOverride( - versionOverrides.filter( - ({ targetPkg }) => - targetPkg.name === name && isIntersectingRange(targetPkg.bareSpecifier, bareSpecifier) - ) - ) ?? - pickMostSpecificVersionOverride( - genericVersionOverrides.filter( - ({ targetPkg }) => - targetPkg.name === name && isIntersectingRange(targetPkg.bareSpecifier, bareSpecifier) - ) - ) + const versionOverride = pickVersionOverride({ versionOverrides, genericVersionOverrides }, name, bareSpecifier) if (!versionOverride) { convergeDep(convergeOpts, { deps, peerDeps }, name, bareSpecifier) continue @@ -144,9 +176,7 @@ function overrideDeps ( continue } - const newBareSpecifier = versionOverride.localTarget - ? `${versionOverride.localTarget.protocol}${resolveLocalOverride(versionOverride.localTarget, dir)}` - : versionOverride.newBareSpecifier + const newBareSpecifier = resolveOverriddenBareSpecifier(versionOverride, dir) if (peerDeps == null || !isValidPeerRange(newBareSpecifier)) { deps[versionOverride.targetPkg.name] = newBareSpecifier } else if (isValidPeerRange(newBareSpecifier)) { @@ -155,6 +185,37 @@ function overrideDeps ( } } +function convergeDep ( + convergeOpts: ConvergeOptions, + { deps, peerDeps }: { deps: Dependencies, peerDeps: Dependencies | undefined }, + name: string, + bareSpecifier: string +): void { + recordConvergeDeclaredRange(convergeOpts, name, bareSpecifier) + const convergeVersion = convergeBareSpecifier(convergeOpts.convergeVersions, name, bareSpecifier) + if (convergeVersion == null) return + if (peerDeps == null) { + deps[name] = convergeVersion + } else { + peerDeps[name] = convergeVersion + } +} + +function recordConvergeDeclaredRange ( + { convergeVersions, convergeDeclaredRanges }: ConvergeOptions, + name: string, + bareSpecifier: string +): void { + if (convergeDeclaredRanges == null) return + if (!convergeVersions.has(name) || semver.validRange(bareSpecifier, true) == null) return + let ranges = convergeDeclaredRanges.get(name) + if (ranges == null) { + ranges = new Set() + convergeDeclaredRanges.set(name, ranges) + } + ranges.add(bareSpecifier) +} + /** * A convergence override (`"pkg@": ""`) rewrites a dependency edge * only when its version satisfies the edge's declared range, so incompatible @@ -162,28 +223,34 @@ function overrideDeps ( * `workspace:`, `catalog:`, `npm:`, git/URL, and dist-tag specifiers have no * defined "satisfies" relation and are left untouched. */ -function convergeDep ( - { convergeVersions, convergeDeclaredRanges }: ConvergeOptions, - { deps, peerDeps }: { deps: Dependencies, peerDeps: Dependencies | undefined }, +function convergeBareSpecifier ( + convergeVersions: Map, name: string, bareSpecifier: string -): void { +): string | undefined { const convergeVersion = convergeVersions.get(name) - if (convergeVersion == null || semver.validRange(bareSpecifier, true) == null) return - if (convergeDeclaredRanges != null) { - let ranges = convergeDeclaredRanges.get(name) - if (ranges == null) { - ranges = new Set() - convergeDeclaredRanges.set(name, ranges) - } - ranges.add(bareSpecifier) - } - if (!semver.satisfies(convergeVersion, bareSpecifier, true)) return - if (peerDeps == null) { - deps[name] = convergeVersion - } else { - peerDeps[name] = convergeVersion - } + if (convergeVersion == null || semver.validRange(bareSpecifier, true) == null) return undefined + return semver.satisfies(convergeVersion, bareSpecifier, true) ? convergeVersion : undefined +} + +function pickVersionOverride ( + { versionOverrides, genericVersionOverrides }: { + versionOverrides: VersionOverrideWithParent[] + genericVersionOverrides: VersionOverride[] + }, + name: string, + bareSpecifier: string +): VersionOverride | undefined { + const matches = (override: VersionOverride): boolean => + override.targetPkg.name === name && isIntersectingRange(override.targetPkg.bareSpecifier, bareSpecifier) + return pickMostSpecificVersionOverride(versionOverrides.filter(matches)) ?? + pickMostSpecificVersionOverride(genericVersionOverrides.filter(matches)) +} + +function resolveOverriddenBareSpecifier (versionOverride: VersionOverride, dir: string | undefined): string { + return versionOverride.localTarget + ? `${versionOverride.localTarget.protocol}${resolveLocalOverride(versionOverride.localTarget, dir)}` + : versionOverride.newBareSpecifier } function resolveLocalOverride ({ specifiedViaRelativePath, absolutePath }: LocalTarget, pkgDir?: string): string { diff --git a/pnpm11/hooks/read-package-hook/src/index.ts b/pnpm11/hooks/read-package-hook/src/index.ts index 52c9fdb7d2..d7367c342b 100644 --- a/pnpm11/hooks/read-package-hook/src/index.ts +++ b/pnpm11/hooks/read-package-hook/src/index.ts @@ -1 +1,2 @@ export { createReadPackageHook, getEffectivePackageExtensions } from './createReadPackageHook.js' +export { createDependencyOverrider, type DependencyOverrider } from './createVersionsOverrider.js' diff --git a/pnpm11/hooks/read-package-hook/test/createVersionOverrider.test.ts b/pnpm11/hooks/read-package-hook/test/createVersionOverrider.test.ts index eea070f9ba..c12fb6794b 100644 --- a/pnpm11/hooks/read-package-hook/test/createVersionOverrider.test.ts +++ b/pnpm11/hooks/read-package-hook/test/createVersionOverrider.test.ts @@ -1,8 +1,9 @@ import path from 'node:path' -import { expect, test } from '@jest/globals' +import { describe, expect, test } from '@jest/globals' +import { parseOverrides } from '@pnpm/config.parse-overrides' -import { createVersionsOverrider } from '../src/createVersionsOverrider.js' +import { createDependencyOverrider, createVersionsOverrider } from '../src/createVersionsOverrider.js' test('createVersionsOverrider() matches sub-ranges', () => { const overrider = createVersionsOverrider([ @@ -905,3 +906,38 @@ test('createVersionsOverrider() collects declared ranges of convergence-governed ['foo', new Set(['^4.0.5', '^3.0.0'])], ])) }) + +describe('createDependencyOverrider()', () => { + test('resolves a generic override for a dependency that has no manifest', () => { + const overrideDependency = createDependencyOverrider(parseOverrides({ + react: 'npm:react@19.2.0', + 'zoo@^1': '1.0.0', + }), process.cwd())! + expect(overrideDependency('react', '^18.0.0')).toBe('npm:react@19.2.0') + expect(overrideDependency('zoo', '^1.5.0')).toBe('1.0.0') + expect(overrideDependency('zoo', '^2.0.0')).toBeUndefined() + expect(overrideDependency('qar', '^1.0.0')).toBeUndefined() + }) + + test('is not created for a set that cannot claim an undeclared dependency', () => { + expect(createDependencyOverrider([], process.cwd())).toBeUndefined() + expect(createDependencyOverrider(parseOverrides({ + 'foo>react': '19.2.0', + }), process.cwd())).toBeUndefined() + }) + + test('resolves a local override relative to the directory of the package that gets the dependency', () => { + const overrideDependency = createDependencyOverrider(parseOverrides({ + qar: 'link:../qar', + }), process.cwd())! + expect(overrideDependency('qar', '^1.0.0', path.resolve('pkg'))).toBe('link:../../qar') + }) + + test('applies a convergence override only when it satisfies the range', () => { + const overrideDependency = createDependencyOverrider(parseOverrides({ + 'react@': '18.3.1', + }), process.cwd())! + expect(overrideDependency('react', '^18.0.0')).toBe('18.3.1') + expect(overrideDependency('react', '^19.0.0')).toBeUndefined() + }) +}) diff --git a/pnpm11/installing/deps-installer/src/getPeerDependencyIssues.ts b/pnpm11/installing/deps-installer/src/getPeerDependencyIssues.ts index 92ab25629e..278a25c80e 100644 --- a/pnpm11/installing/deps-installer/src/getPeerDependencyIssues.ts +++ b/pnpm11/installing/deps-installer/src/getPeerDependencyIssues.ts @@ -1,6 +1,6 @@ import { DEFAULT_REGISTRIES } from '@pnpm/config.normalize-registries' import { parseOverrides } from '@pnpm/config.parse-overrides' -import { createReadPackageHook } from '@pnpm/hooks.read-package-hook' +import { createDependencyOverrider, createReadPackageHook } from '@pnpm/hooks.read-package-hook' import { getContext, type GetContextOptions, type ProjectOptions } from '@pnpm/installing.context' import { getWantedDependencies, resolveDependencies } from '@pnpm/installing.deps-resolver' import { getPreferredVersionsFromLockfileAndManifests } from '@pnpm/lockfile.preferred-versions' @@ -81,6 +81,7 @@ export async function getPeerDependencyIssues ( ignoredOptionalDependencies: opts.ignoredOptionalDependencies, }), }, + overrideBareSpecifier: createDependencyOverrider(overrides, lockfileDir), linkWorkspacePackagesDepth: opts.linkWorkspacePackagesDepth ?? (opts.saveWorkspaceProtocol ? 0 : -1), lockfileDir, nodeVersion: opts.nodeVersion ?? process.version, diff --git a/pnpm11/installing/deps-installer/src/install/index.ts b/pnpm11/installing/deps-installer/src/install/index.ts index f29f5e3c97..6f9856be0d 100644 --- a/pnpm11/installing/deps-installer/src/install/index.ts +++ b/pnpm11/installing/deps-installer/src/install/index.ts @@ -30,6 +30,7 @@ import { runLifecycleHooksConcurrently, type RunLifecycleHooksConcurrentlyOptions, } from '@pnpm/exec.lifecycle' +import { createDependencyOverrider } from '@pnpm/hooks.read-package-hook' import { getContext, type PnpmContext } from '@pnpm/installing.context' import { type DependenciesGraph, @@ -1585,6 +1586,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => { hooks: { readPackage: opts.readPackageHook, }, + overrideBareSpecifier: createDependencyOverrider(opts.parsedOverrides, opts.lockfileDir), linkWorkspacePackagesDepth: opts.linkWorkspacePackagesDepth ?? (opts.saveWorkspaceProtocol ? 0 : -1), lockfileDir: opts.lockfileDir, nodeVersion: opts.nodeVersion, diff --git a/pnpm11/installing/deps-resolver/src/hoistPeers.ts b/pnpm11/installing/deps-resolver/src/hoistPeers.ts index e582b879b3..5767170528 100644 --- a/pnpm11/installing/deps-resolver/src/hoistPeers.ts +++ b/pnpm11/installing/deps-resolver/src/hoistPeers.ts @@ -15,21 +15,35 @@ export function hoistPeers ( autoInstallPeers: boolean allPreferredVersions?: PreferredVersions workspaceRootDeps: HoistableRootDep[] + /** + * Applies `overrides` to a peer nobody declares as a dependency. Such a + * peer has no manifest for the read-package hook to rewrite, so without + * this it would resolve against its declared peer range and silently + * produce the second copy the override exists to prevent. + */ + overrideBareSpecifier?: (name: string, range: string) => string | undefined }, missingRequiredPeers: Array<[string, { range: string }]> ): Record { const dependencies: Record = {} for (const [peerName, { range }] of missingRequiredPeers) { - const rootDepByAlias = opts.workspaceRootDeps.find((rootDep) => rootDep.alias === peerName) - if (rootDepByAlias?.normalizedBareSpecifier) { - dependencies[peerName] = rootDepByAlias.normalizedBareSpecifier + const rootBareSpecifier = findWorkspaceRootDep(opts.workspaceRootDeps, peerName)?.normalizedBareSpecifier + // An override redirects a hoist; it must never create one, or disabling + // autoInstallPeers would still install a peer nobody depends on. Only the + // workspace root's own dependency hoists a peer that autoInstallPeers is + // not asking for, so that is the one hoist an override still governs here; + // the deduplication below installs nothing new either way. + const overridden = opts.autoInstallPeers || rootBareSpecifier + ? opts.overrideBareSpecifier?.(peerName, range) + : undefined + if (overridden != null) { + if (overridden !== '-') { + dependencies[peerName] = overridden + } continue } - const rootDep = opts.workspaceRootDeps - .filter((rootDep) => rootDep.pkgName === peerName) - .sort((rootDep1, rootDep2) => lexCompare(rootDep1.alias, rootDep2.alias))[0] - if (rootDep?.normalizedBareSpecifier) { - dependencies[peerName] = rootDep.normalizedBareSpecifier + if (rootBareSpecifier) { + dependencies[peerName] = rootBareSpecifier continue } if (opts.allPreferredVersions![peerName]) { @@ -83,17 +97,29 @@ export function hoistPeers ( export function getHoistableOptionalPeers ( allMissingOptionalPeers: Record, - allPreferredVersions: PreferredVersions + allPreferredVersions: PreferredVersions, + workspaceRootDeps: HoistableRootDep[] = [] ): Record { const optionalDependencies: Record = {} for (const [missingOptionalPeerName, ranges] of Object.entries(allMissingOptionalPeers)) { if (!allPreferredVersions[missingOptionalPeerName]) continue + // The workspace root's own specifier bounds the candidates the same way + // it short-circuits `hoistPeers` above. Maximizing over every version in + // the graph instead lets one importer's newer resolution be hoisted into + // a sibling that declares nothing, adding a second instance of a package + // the root already pins. A scheme specifier bounds them through the + // version body getPeerVersionRange extracts; one with no version body + // yields `*` and leaves them unbounded. + const rootBareSpecifier = findWorkspaceRootDep(workspaceRootDeps, missingOptionalPeerName)?.normalizedBareSpecifier + const rootRange = rootBareSpecifier != null ? semver.validRange(getPeerVersionRange(rootBareSpecifier)) : null + let maxSatisfyingVersion: string | undefined for (const [version, selector] of Object.entries(allPreferredVersions[missingOptionalPeerName])) { const specType = typeof selector === 'string' ? selector : selector.selectorType if ( specType === 'version' && + (rootRange == null || semver.satisfies(version, rootRange)) && ranges.every(range => semver.satisfies(version, range)) && (!maxSatisfyingVersion || semver.gt(version, maxSatisfyingVersion)) ) { @@ -106,3 +132,31 @@ export function getHoistableOptionalPeers ( } return optionalDependencies } + +/** + * The root dependency that provides `peerName`: an alias match wins over a + * package-name match (an `npm:` alias can install the same package under a + * different slot), and among package-name matches the lexicographically + * first alias wins so the pick is stable. Only a dependency that has a + * normalized specifier is a candidate — the callers have nothing to install + * or bound the peer with otherwise. + */ +function findWorkspaceRootDep ( + workspaceRootDeps: HoistableRootDep[], + peerName: string +): HoistableRootDep | undefined { + // One allocation-free pass: this runs for every missing peer of every + // importer on each hoist round. + let rootDepByPkgName: HoistableRootDep | undefined + for (const rootDep of workspaceRootDeps) { + if (!rootDep.normalizedBareSpecifier) continue + if (rootDep.alias === peerName) return rootDep + if ( + rootDep.pkgName === peerName && + (rootDepByPkgName == null || lexCompare(rootDep.alias, rootDepByPkgName.alias) < 0) + ) { + rootDepByPkgName = rootDep + } + } + return rootDepByPkgName +} diff --git a/pnpm11/installing/deps-resolver/src/resolveDependencies.ts b/pnpm11/installing/deps-resolver/src/resolveDependencies.ts index 621273b798..746eebde2e 100644 --- a/pnpm11/installing/deps-resolver/src/resolveDependencies.ts +++ b/pnpm11/installing/deps-resolver/src/resolveDependencies.ts @@ -195,6 +195,7 @@ export interface ResolutionContext { force: boolean preferWorkspacePackages?: boolean readPackageHook?: ReadPackageHook + overrideBareSpecifier?: (name: string, bareSpecifier: string, dir?: string) => string | undefined engineStrict: boolean nodeVersion?: string pnpmVersion: string @@ -411,15 +412,21 @@ export async function resolveRootDependencies ( } else { workspaceRootDeps = [] } - const _hoistPeers = hoistPeers.bind(null, { - autoInstallPeers: ctx.autoInstallPeers, - allPreferredVersions: ctx.allPreferredVersions, - workspaceRootDeps, - }) /* eslint-disable no-await-in-loop */ while (true) { const allMissingOptionalPeersByImporters = await Promise.all(pkgAddressesByImportersWithoutPeers.map(async (importerResolutionResult, index) => { const { parentPkgAliases, preferredVersions, options } = importers[index] + // The importer is the manifest the hoisted peer is added to, so a local + // override's `link:`/`file:` target is made relative to its directory, + // exactly as it would be for a dependency the importer declares. + const _hoistPeers = hoistPeers.bind(null, { + autoInstallPeers: ctx.autoInstallPeers, + allPreferredVersions: ctx.allPreferredVersions, + workspaceRootDeps, + overrideBareSpecifier: ctx.overrideBareSpecifier == null + ? undefined + : (name, range) => ctx.overrideBareSpecifier!(name, range, options.prefix), + }) const allMissingOptionalPeers: Record = {} while (true) { for (const pkgAddress of importerResolutionResult.pkgAddresses) { @@ -477,7 +484,7 @@ export async function resolveRootDependencies ( await Promise.all(allMissingOptionalPeersByImporters.map(async (allMissingOptionalPeers, index) => { const { preferredVersions, parentPkgAliases, options } = importers[index] if (Object.keys(allMissingOptionalPeers).length && ctx.allPreferredVersions) { - const optionalDependencies = getHoistableOptionalPeers(allMissingOptionalPeers, ctx.allPreferredVersions) + const optionalDependencies = getHoistableOptionalPeers(allMissingOptionalPeers, ctx.allPreferredVersions, workspaceRootDeps) if (Object.keys(optionalDependencies).length) { hasNewMissingPeers = true const wantedDependencies = getNonDevWantedDependencies({ optionalDependencies }) diff --git a/pnpm11/installing/deps-resolver/src/resolveDependencyTree.ts b/pnpm11/installing/deps-resolver/src/resolveDependencyTree.ts index d78aeefe6b..abc4e0b001 100644 --- a/pnpm11/installing/deps-resolver/src/resolveDependencyTree.ts +++ b/pnpm11/installing/deps-resolver/src/resolveDependencyTree.ts @@ -126,6 +126,7 @@ export interface ResolveDependenciesOptions { hooks: { readPackage?: ReadPackageHook } + overrideBareSpecifier?: (name: string, bareSpecifier: string, dir?: string) => string | undefined nodeVersion?: string registries: Registries namedRegistries?: Record @@ -209,6 +210,7 @@ export async function resolveDependencyTree ( pnpmVersion: opts.pnpmVersion, preferWorkspacePackages: opts.preferWorkspacePackages, readPackageHook: opts.hooks.readPackage, + overrideBareSpecifier: opts.overrideBareSpecifier, registries: opts.registries, namedRegistryPrefixes: Array.from( new Set([ diff --git a/pnpm11/installing/deps-resolver/test/hoistPeers.test.ts b/pnpm11/installing/deps-resolver/test/hoistPeers.test.ts index a375953b58..c716cf67e1 100644 --- a/pnpm11/installing/deps-resolver/test/hoistPeers.test.ts +++ b/pnpm11/installing/deps-resolver/test/hoistPeers.test.ts @@ -269,3 +269,136 @@ test('getHoistableOptionalPeers handles version selector with weight', () => { jsdom: '27.4.0', }) }) + +test('hoistPeers installs an auto-installed peer at the overridden specifier', () => { + expect(hoistPeers({ + autoInstallPeers: true, + allPreferredVersions: { + react: { + '18.3.1': 'version', + }, + }, + workspaceRootDeps: [{ alias: 'react', pkgName: 'react', normalizedBareSpecifier: '18.3.1' }], + overrideBareSpecifier: (name) => name === 'react' ? 'npm:react@19.2.0' : undefined, + }, [['react', { range: '^16.5.1 || ^17.0.0 || ^18.0.0' }]])).toStrictEqual({ + react: 'npm:react@19.2.0', + }) +}) + +test('hoistPeers does not let an override install a peer that nothing provides when peers are not auto-installed', () => { + expect(hoistPeers({ + autoInstallPeers: false, + allPreferredVersions: {}, + workspaceRootDeps: [], + overrideBareSpecifier: () => 'npm:react@19.2.0', + }, [['react', { range: '^18.0.0' }]])).toStrictEqual({}) +}) + +test('hoistPeers leaves a deduplicating hoist to the graph when peers are not auto-installed', () => { + expect(hoistPeers({ + autoInstallPeers: false, + allPreferredVersions: { + react: { + '18.3.1': 'version', + }, + }, + workspaceRootDeps: [], + overrideBareSpecifier: () => 'npm:react@19.2.0', + }, [['react', { range: '^18.0.0' }]])).toStrictEqual({ + react: '18.3.1', + }) +}) + +test('hoistPeers redirects the workspace root\'s hoist through an override when peers are not auto-installed', () => { + expect(hoistPeers({ + autoInstallPeers: false, + allPreferredVersions: {}, + workspaceRootDeps: [{ alias: 'react', pkgName: 'react', normalizedBareSpecifier: '18.3.1' }], + overrideBareSpecifier: () => 'npm:react@19.2.0', + }, [['react', { range: '^18.0.0' }]])).toStrictEqual({ + react: 'npm:react@19.2.0', + }) +}) + +test('hoistPeers leaves a peer removed by an override uninstalled', () => { + expect(hoistPeers({ + autoInstallPeers: true, + allPreferredVersions: { + react: { + '18.3.1': 'version', + }, + }, + workspaceRootDeps: [], + overrideBareSpecifier: () => '-', + }, [['react', { range: '^18.0.0' }]])).toStrictEqual({}) +}) + +test('getHoistableOptionalPeers stays within the workspace root\'s range', () => { + const allMissingOptionalPeers = { postcss: ['*'] } + const allPreferredVersions = { + postcss: { + '8.5.10': 'version' as const, + '8.5.22': 'version' as const, + }, + } + expect(getHoistableOptionalPeers(allMissingOptionalPeers, allPreferredVersions, [ + { alias: 'postcss', pkgName: 'postcss', normalizedBareSpecifier: '8.5.10' }, + ])).toStrictEqual({ + postcss: '8.5.10', + }) + expect(getHoistableOptionalPeers(allMissingOptionalPeers, allPreferredVersions)).toStrictEqual({ + postcss: '8.5.22', + }) +}) + +test('hoistPeers skips a workspace root dependency that has no specifier in favor of one that has', () => { + const workspaceRootDeps = [ + { alias: 'postcss', pkgName: 'postcss' }, + { alias: 'zz-postcss', pkgName: 'postcss', normalizedBareSpecifier: '8.5.10' }, + ] + expect(hoistPeers({ + autoInstallPeers: true, + allPreferredVersions: {}, + workspaceRootDeps, + }, [['postcss', { range: '^8.0.0' }]])).toStrictEqual({ + postcss: '8.5.10', + }) + expect(getHoistableOptionalPeers({ postcss: ['*'] }, { + postcss: { + '8.5.10': 'version', + '9.0.0': 'version', + }, + }, workspaceRootDeps)).toStrictEqual({ + postcss: '8.5.10', + }) +}) + +test('getHoistableOptionalPeers stays within the version range of a scheme-prefixed workspace root specifier', () => { + const allMissingOptionalPeers = { postcss: ['*'] } + const allPreferredVersions = { + postcss: { + '8.5.10': 'version' as const, + '9.0.0': 'version' as const, + }, + } + for (const normalizedBareSpecifier of ['workspace:^8.5.10', 'npm:postcss@^8.5.10', 'work:^8.5.10']) { + expect(getHoistableOptionalPeers(allMissingOptionalPeers, allPreferredVersions, [ + { alias: 'postcss', pkgName: 'postcss', normalizedBareSpecifier }, + ])).toStrictEqual({ + postcss: '8.5.10', + }) + } +}) + +test('getHoistableOptionalPeers keeps candidates unbounded when the workspace root specifier has no version', () => { + expect(getHoistableOptionalPeers({ postcss: ['*'] }, { + postcss: { + '8.5.10': 'version', + '9.0.0': 'version', + }, + }, [ + { alias: 'postcss', pkgName: 'postcss', normalizedBareSpecifier: 'file:../postcss' }, + ])).toStrictEqual({ + postcss: '9.0.0', + }) +})