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', + }) +})