fix: hoist peer dependencies (#7841)

pkg-manager/core/src/getPeerDependencyIssues.ts

@@ -8,6 +8,7 @@ import { DEFAULT_REGISTRIES } from '@pnpm/normalize-registries'
 
 export type ListMissingPeersOptions = Partial<GetContextOptions>
 & Pick<InstallOptions, 'hooks'
+| 'dedupePeerDependents'
 | 'ignoreCompatibilityDb'
 | 'linkWorkspacePackagesDepth'
 | 'nodeVersion'
@@ -58,6 +59,7 @@ export async function getPeerDependencyIssues (
       allowedDeprecatedVersions: {},
       allowNonAppliedPatches: false,
       defaultUpdateDepth: -1,
+      dedupePeerDependents: opts.dedupePeerDependents,
       dryRun: true,
       engineStrict: false,
       force: false,

pkg-manager/core/test/install/peerDependencies.ts

@@ -1435,6 +1435,7 @@ test('when there are several aliased dependencies of the same package, pick the
   await addDependenciesToPackage(manifest, ['@pnpm.e2e/abc@1.0.0'], opts)
 
   const lockfile = readYamlFile<any>(path.resolve(WANTED_LOCKFILE)) // eslint-disable-line
+  console.log(JSON.stringify(lockfile, null, 2))
   expect(lockfile.snapshots['@pnpm.e2e/abc@1.0.0(@pnpm.e2e/peer-c@2.0.0)']).toBeTruthy()
 })
 
@@ -1756,7 +1757,8 @@ test('3 circular peers in workspace root', async () => {
   ], testDefaults({ allProjects, reporter, autoInstallPeers: false, resolvePeersFromWorkspaceRoot: true, strictPeerDependencies: false }))
 
   const lockfile = projects.root.readLockfile()
-  expect(lockfile.importers.pkg?.dependencies?.['@pnpm.e2e/circular-peers-1-of-3'].version).toBe('1.0.0(@pnpm.e2e/circular-peers-2-of-3@1.0.0(@pnpm.e2e/circular-peers-3-of-3@1.0.0)(@pnpm.e2e/peer-a@1.0.0))(@pnpm.e2e/peer-a@1.0.0)')
+  expect(Object.keys(lockfile.snapshots).length).toBe(4)
+  expect(lockfile.importers.pkg?.dependencies?.['@pnpm.e2e/circular-peers-1-of-3'].version).toBe('1.0.0(@pnpm.e2e/circular-peers-2-of-3@1.0.0)(@pnpm.e2e/peer-a@1.0.0)')
 })
 
 test('resolves complex circular deps', async () => {
@@ -1797,6 +1799,20 @@ test('optional peer dependency is resolved if it is installed anywhere in the de
   expect(lockfile.snapshots['@pnpm.e2e/abc-optional-peers@1.0.0(@pnpm.e2e/peer-a@1.0.0)(@pnpm.e2e/peer-b@1.0.0)(@pnpm.e2e/peer-c@1.0.0)']).toBeDefined()
 })
 
+test('optional peer dependency is resolved if it is installed anywhere in the dependency graph and auto install peers is false', async () => {
+  await addDistTag({ package: '@pnpm.e2e/abc-parent-with-ab', version: '1.0.0', distTag: 'latest' })
+  const project = prepareEmpty()
+
+  await addDependenciesToPackage(
+    {},
+    ['@pnpm.e2e/abc-regular-deps@1.0.0', '@pnpm.e2e/abc-optional-peers@1.0.0'],
+    testDefaults({ autoInstallPeers: false })
+  )
+
+  const lockfile = project.readLockfile()
+  expect(lockfile.snapshots['@pnpm.e2e/abc-optional-peers@1.0.0(@pnpm.e2e/peer-a@1.0.0)(@pnpm.e2e/peer-b@1.0.0)(@pnpm.e2e/peer-c@1.0.0)']).toBeDefined()
+})
+
 // It is resolved on the second iteration only
 test('optional peer dependency is resolved if it is installed anywhere in the dependency graph and auto install peers is true #2', async () => {
   await addDistTag({ package: '@pnpm.e2e/abc-parent-with-ab', version: '1.0.0', distTag: 'latest' })

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

@@ -164,6 +164,7 @@ export interface ResolutionContext {
   virtualStoreDir: string
   workspacePackages?: WorkspacePackages
   missingPeersOfChildrenByPkgId: Record<string, { parentImporterId: string, missingPeersOfChildren: MissingPeersOfChildren }>
+  hoistPeers?: boolean
 }
 
 export type MissingPeers = Record<string, { range: string, optional: boolean }>
@@ -292,11 +293,13 @@ export async function resolveRootDependencies (
 ): Promise<ResolvedRootDependenciesResult> {
   if (ctx.autoInstallPeers) {
     ctx.allPreferredVersions = getPreferredVersionsFromLockfileAndManifests(ctx.wantedLockfile.packages, [])
+  } else if (ctx.hoistPeers) {
+    ctx.allPreferredVersions = {}
   }
   const { pkgAddressesByImportersWithoutPeers, publishedBy, time } = await resolveDependenciesOfImporters(ctx, importers)
   const pkgAddressesByImporters = await Promise.all(zipWith(async (importerResolutionResult, { parentPkgAliases, preferredVersions, options }) => {
     const pkgAddresses = importerResolutionResult.pkgAddresses
-    if (!ctx.autoInstallPeers) return pkgAddresses
+    if (!ctx.hoistPeers) return pkgAddresses
     let prevMissingOptionalPeers: string[] = []
     while (true) {
       for (const pkgAddress of importerResolutionResult.pkgAddresses) {
@@ -306,12 +309,14 @@ export async function resolveRootDependencies (
       for (const missingPeerName of Object.keys(missingRequiredPeers)) {
         parentPkgAliases[missingPeerName] = true
       }
-      // All the missing peers should get installed in the root.
-      // Otherwise, pending nodes will not work.
-      // even those peers should be hoisted that are not autoinstalled
-      for (const [resolvedPeerName, resolvedPeerAddress] of Object.entries(importerResolutionResult.resolvedPeers ?? {})) {
-        if (!parentPkgAliases[resolvedPeerName]) {
-          pkgAddresses.push(resolvedPeerAddress)
+      if (ctx.autoInstallPeers) {
+        // All the missing peers should get installed in the root.
+        // Otherwise, pending nodes will not work.
+        // even those peers should be hoisted that are not autoinstalled
+        for (const [resolvedPeerName, resolvedPeerAddress] of Object.entries(importerResolutionResult.resolvedPeers ?? {})) {
+          if (!parentPkgAliases[resolvedPeerName]) {
+            pkgAddresses.push(resolvedPeerAddress)
+          }
         }
       }
       const missingOptionalPeerNames = Array.from(
@@ -323,13 +328,23 @@ export async function resolveRootDependencies (
         )
       )
       if (!missingRequiredPeers.length && !missingOptionalPeerNames.length) break
-      const dependencies = Object.fromEntries(
-        missingRequiredPeers
-          .map(([peerName, { range }]) => {
-            if (!ctx.allPreferredVersions![peerName]) return [peerName, range]
-            return [peerName, Object.keys(ctx.allPreferredVersions![peerName]).join(' || ')]
-          })
-      )
+      const dependencies: Record<string, string> = {}
+      for (const [peerName, { range }] of missingRequiredPeers) {
+        if (ctx.allPreferredVersions![peerName]) {
+          const versions: string[] = []
+          const nonVersions: string[] = []
+          for (const [spec, specType] of Object.entries(ctx.allPreferredVersions![peerName])) {
+            if (specType === 'version') {
+              versions.push(spec)
+            } else {
+              nonVersions.push(spec)
+            }
+          }
+          dependencies[peerName] = [semver.maxSatisfying(versions, '*'), ...nonVersions].join(' || ')
+        } else if (ctx.autoInstallPeers) {
+          dependencies[peerName] = range
+        }
+      }
       const nextMissingOptionalPeers: string[] = []
       for (const missingOptionalPeerName of missingOptionalPeerNames) {
         if (ctx.allPreferredVersions![missingOptionalPeerName]) {
@@ -465,7 +480,7 @@ async function resolveDependenciesOfImporters (
     const childrenResults = await Promise.all(
       postponedResolutionsQueue.map((postponedResolution) => postponedResolution(postponedResolutionOpts))
     )
-    if (!ctx.autoInstallPeers) {
+    if (!ctx.hoistPeers) {
       return {
         missingPeers: {},
         pkgAddresses,
@@ -595,7 +610,7 @@ export async function resolveDependencies (
   const childrenResults = await Promise.all(
     postponedResolutionsQueue.map((postponedResolution) => postponedResolution(postponedResolutionOpts))
   )
-  if (!ctx.autoInstallPeers) {
+  if (!ctx.hoistPeers) {
     return {
       resolvingPeers: Promise.resolve({
         missingPeers: {},
@@ -1372,7 +1387,7 @@ async function resolveDependency (
     ctx.resolvedPackagesByDepPath[depPath].prod = ctx.resolvedPackagesByDepPath[depPath].prod || !wantedDependency.dev && !wantedDependency.optional
     ctx.resolvedPackagesByDepPath[depPath].dev = ctx.resolvedPackagesByDepPath[depPath].dev || wantedDependency.dev
     ctx.resolvedPackagesByDepPath[depPath].optional = ctx.resolvedPackagesByDepPath[depPath].optional && currentIsOptional
-    if (ctx.autoInstallPeers) {
+    if (ctx.hoistPeers) {
       resolveChildren = !ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved &&
         !ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.has(parentImporterId)
       ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.add(parentImporterId)
@@ -1399,7 +1414,7 @@ async function resolveDependency (
     ? path.resolve(ctx.lockfileDir, (pkgResponse.body.resolution as DirectoryResolution).directory)
     : options.prefix
   let missingPeersOfChildren!: MissingPeersOfChildren | undefined
-  if (ctx.autoInstallPeers && !nodeIdContains(options.parentPkg.nodeId, depPath)) {
+  if (ctx.hoistPeers && !nodeIdContains(options.parentPkg.nodeId, depPath)) {
     if (ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id]) {
       if (!options.parentPkg.nodeId.startsWith(ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].parentImporterId)) {
         missingPeersOfChildren = ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren

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

@@ -26,6 +26,7 @@ import {
   resolveRootDependencies,
   type ResolvedPackage,
   type ResolvedPackagesByDepPath,
+  type ResolutionContext,
 } from './resolveDependencies'
 
 export * from './nodeIdUtils'
@@ -76,6 +77,7 @@ export interface ResolveDependenciesOptions {
   allowedDeprecatedVersions: AllowedDeprecatedVersions
   allowNonAppliedPatches: boolean
   currentLockfile: Lockfile
+  dedupePeerDependents?: boolean
   dryRun: boolean
   engineStrict: boolean
   force: boolean
@@ -108,8 +110,9 @@ export async function resolveDependencyTree<T> (
   opts: ResolveDependenciesOptions
 ) {
   const wantedToBeSkippedPackageIds = new Set<string>()
-  const ctx = {
-    autoInstallPeers: opts.autoInstallPeers === true,
+  const autoInstallPeers = opts.autoInstallPeers === true
+  const ctx: ResolutionContext = {
+    autoInstallPeers,
     autoInstallPeersFromHighestMatch: opts.autoInstallPeersFromHighestMatch === true,
     allowBuild: opts.allowBuild,
     allowedDeprecatedVersions: opts.allowedDeprecatedVersions,
@@ -142,6 +145,7 @@ export async function resolveDependencyTree<T> (
     updatedSet: new Set<string>(),
     workspacePackages: opts.workspacePackages,
     missingPeersOfChildrenByPkgId: {},
+    hoistPeers: autoInstallPeers || opts.dedupePeerDependents,
   }
 
   const resolveArgs: ImporterToResolve[] = importers.map((importer) => {