fix: only automatically resolve optional peers with versions that sat…

…isfy the ranges (#8028)

close #7985

.changeset/curly-actors-repeat.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/resolve-dependencies": patch
+"pnpm": patch
+---
+
+A dependency is hoisted to resolve an optional peer dependency only if it satisfies the range provided for the optional peer dependency [#8028](https://github.com/pnpm/pnpm/pull/8028).

cspell.json

@@ -87,6 +87,7 @@
  "hashbang",
  "highmaps",
  "hikljmi",
+ "hoistable",
  "homepath",
  "hosters",
  "hyperdrive",

pkg-manager/resolve-dependencies/src/hoistPeers.ts

@@ -27,3 +27,28 @@ export function hoistPeers (
  }
  return dependencies
 }
+
+export function getHoistableOptionalPeers (
+ allMissingOptionalPeers: Record<string, string[]>,
+ allPreferredVersions: PreferredVersions
+): Record<string, string> {
+ const optionalDependencies: Record<string, string> = {}
+ for (const [missingOptionalPeerName, ranges] of Object.entries(allMissingOptionalPeers)) {
+ if (!allPreferredVersions[missingOptionalPeerName]) continue
+
+ let maxSatisfyingVersion: string | undefined
+ for (const [version, specType] of Object.entries(allPreferredVersions[missingOptionalPeerName])) {
+ if (
+ specType === 'version' &&
+ ranges.every(range => semver.satisfies(version, range)) &&
+ (!maxSatisfyingVersion || semver.gt(version, maxSatisfyingVersion))
+ ) {
+ maxSatisfyingVersion = version
+ }
+ }
+ if (maxSatisfyingVersion) {
+ optionalDependencies[missingOptionalPeerName] = maxSatisfyingVersion
+ }
+ }
+ return optionalDependencies
+}

pkg-manager/resolve-dependencies/src/resolveDependencies.ts

@@ -57,7 +57,7 @@ import {
  nodeIdContains,
  splitNodeId,
 } from './nodeIdUtils'
-import { hoistPeers } from './hoistPeers'
+import { hoistPeers, getHoistableOptionalPeers } from './hoistPeers'
 import { wantedDepIsLocallyAvailable } from './wantedDepIsLocallyAvailable'
 import { replaceVersionInPref } from './replaceVersionInPref'
 
@@ -303,7 +303,7 @@ export async function resolveRootDependencies (
  const pkgAddressesByImporters = await Promise.all(zipWith(async (importerResolutionResult, { parentPkgAliases, preferredVersions, options }) => {
  const pkgAddresses = importerResolutionResult.pkgAddresses
  if (!ctx.hoistPeers) return pkgAddresses
- let prevMissingOptionalPeers: string[] = []
+ const allMissingOptionalPeers: Record<string, string[]> = {}
  while (true) {
  for (const pkgAddress of importerResolutionResult.pkgAddresses) {
  parentPkgAliases[pkgAddress.alias] = true
@@ -322,28 +322,17 @@ export async function resolveRootDependencies (
  }
  }
  }
- const missingOptionalPeerNames = Array.from(
- new Set(
- [
- ...missingOptionalPeers.map(([peerName]) => peerName),
- ...prevMissingOptionalPeers,
- ]
- )
- )
- if (!missingRequiredPeers.length && !missingOptionalPeerNames.length) break
- const dependencies = hoistPeers(missingRequiredPeers, ctx)
- const nextMissingOptionalPeers: string[] = []
- const optionalDependencies: Record<string, string> = {}
- for (const missingOptionalPeerName of missingOptionalPeerNames) {
- if (ctx.allPreferredVersions![missingOptionalPeerName]) {
- optionalDependencies[missingOptionalPeerName] = Object.keys(ctx.allPreferredVersions![missingOptionalPeerName]).join(' || ')
- } else {
- nextMissingOptionalPeers.push(missingOptionalPeerName)
+ for (const [missingOptionalPeerName, { range: missingOptionalPeerRange }] of missingOptionalPeers) {
+ if (!allMissingOptionalPeers[missingOptionalPeerName]) {
+ allMissingOptionalPeers[missingOptionalPeerName] = [missingOptionalPeerRange]
+ } else if (!allMissingOptionalPeers[missingOptionalPeerName].includes(missingOptionalPeerRange)) {
+ allMissingOptionalPeers[missingOptionalPeerName].push(missingOptionalPeerRange)
  }
  }
- prevMissingOptionalPeers = nextMissingOptionalPeers
- if (!Object.keys(dependencies).length && !Object.keys(optionalDependencies).length) break
- const wantedDependencies = getNonDevWantedDependencies({ dependencies, optionalDependencies })
+ if (!missingRequiredPeers.length) break
+ const dependencies = hoistPeers(missingRequiredPeers, ctx)
+ if (!Object.keys(dependencies).length) break
+ const wantedDependencies = getNonDevWantedDependencies({ dependencies })
 
  // eslint-disable-next-line no-await-in-loop
  const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, {
@@ -358,6 +347,22 @@ export async function resolveRootDependencies (
  }
  pkgAddresses.push(...importerResolutionResult.pkgAddresses)
  }
+ if (Object.keys(allMissingOptionalPeers).length && ctx.allPreferredVersions) {
+ const optionalDependencies = getHoistableOptionalPeers(allMissingOptionalPeers, ctx.allPreferredVersions)
+ if (Object.keys(optionalDependencies).length) {
+ const wantedDependencies = getNonDevWantedDependencies({ optionalDependencies })
+ const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, {
+ ...options,
+ parentPkgAliases,
+ publishedBy,
+ })
+ importerResolutionResult = {
+ pkgAddresses: resolveDependenciesResult.pkgAddresses,
+ ...filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases),
+ }
+ pkgAddresses.push(...importerResolutionResult.pkgAddresses)
+ }
+ }
  return pkgAddresses
  }, pkgAddressesByImportersWithoutPeers, importers))
  return { pkgAddressesByImporters, time }

pkg-manager/resolve-dependencies/test/hoistPeers.test.ts

@@ -1,4 +1,4 @@
-import { hoistPeers } from '../lib/hoistPeers'
+import { hoistPeers, getHoistableOptionalPeers } from '../lib/hoistPeers'
 
 test('hoistPeers picks an already available prerelease version', () => {
  expect(hoistPeers([['foo', { range: '*' }]], {
@@ -12,3 +12,31 @@ test('hoistPeers picks an already available prerelease version', () => {
  foo: '1.0.0-beta.0',
  })
 })
+
+test('getHoistableOptionalPeers only picks a version that satisfies all optional ranges', () => {
+ expect(getHoistableOptionalPeers({
+ foo: ['2', '2.1'],
+ }, {
+ foo: {
+ '1.0.0': 'version',
+ '2.0.0': 'version',
+ '2.1.0': 'version',
+ '3.0.0': 'version',
+ },
+ })).toStrictEqual({
+ foo: '2.1.0',
+ })
+})
+
+test('getHoistableOptionalPeers picks the highest version that satisfies all the optional ranges', () => {
+ expect(getHoistableOptionalPeers({
+ foo: ['2', '2.1'],
+ }, {
+ foo: {
+ '2.1.0': 'version',
+ '2.1.1': 'version',
+ },
+ })).toStrictEqual({
+ foo: '2.1.1',
+ })
+})