perf: fix hoisting peers (#8144)

Reverts 4b65113

ref #8072

This change makes install faster at least 3 times in some Bit workspaces.

.changeset/perfect-needles-collect.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/resolve-dependencies": patch
+"pnpm": patch
+---
+
+Improved the performance of the resolution stage by changing how missing peer dependencies are detected [#8144](https://github.com/pnpm/pnpm/pull/8144).

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

@@ -323,15 +323,16 @@ test('automatically install peer dependency when it is a dev dependency in anoth
 
 // Covers https://github.com/pnpm/pnpm/issues/4820
 test('auto install peer deps in a workspace. test #1', async () => {
-  prepareEmpty()
+  await addDistTag({ package: '@pnpm.e2e/peer-c', version: '1.0.0', distTag: 'latest' })
+  const project = prepareEmpty()
   await mutateModules([
     {
       mutation: 'install',
-      rootDir: process.cwd(),
+      rootDir: path.resolve('project1'),
     },
     {
       mutation: 'install',
-      rootDir: path.resolve('project'),
+      rootDir: path.resolve('project2'),
     },
   ], testDefaults({
     autoInstallPeers: true,
@@ -344,7 +345,7 @@ test('auto install peer deps in a workspace. test #1', async () => {
             '@pnpm.e2e/abc-parent-with-ab': '1.0.0',
           },
         },
-        rootDir: process.cwd(),
+        rootDir: path.resolve('project1'),
       },
       {
         buildIndex: 0,
@@ -354,10 +355,14 @@ test('auto install peer deps in a workspace. test #1', async () => {
             '@pnpm.e2e/abc-parent-with-ab': '1.0.0',
           },
         },
-        rootDir: path.resolve('project'),
+        rootDir: path.resolve('project2'),
       },
     ],
+    dedupePeerDependents: false,
   }))
+  const lockfile = project.readLockfile()
+  expect(lockfile.importers['project1'].devDependencies?.['@pnpm.e2e/abc-parent-with-ab']?.version).toBe('1.0.0(@pnpm.e2e/peer-c@1.0.0)')
+  expect(lockfile.importers['project2'].dependencies?.['@pnpm.e2e/abc-parent-with-ab']?.version).toBe('1.0.0(@pnpm.e2e/peer-c@1.0.0)')
 })
 
 test('auto install peer deps in a workspace. test #2', async () => {

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

@@ -1384,7 +1384,7 @@ test('deduplicate packages that have peers, when adding new dependency in a work
   expect(depPaths).toContain(`@pnpm.e2e/abc-parent-with-ab@1.0.0${createPeersDirSuffix([{ name: '@pnpm.e2e/peer-c', version: '1.0.0' }])}`)
 })
 
-test.skip('resolve peer dependencies from aliased subdependencies if they are dependencies of a parent package', async () => {
+test('resolve peer dependencies from aliased subdependencies if they are dependencies of a parent package', async () => {
   prepareEmpty()
   await addDistTag({ package: '@pnpm.e2e/peer-a', version: '1.0.0', distTag: 'latest' })
   await addDistTag({ package: '@pnpm.e2e/peer-c', version: '1.0.0', distTag: 'latest' })
@@ -1452,7 +1452,7 @@ test('when there is an aliases dependency and a non-aliased one, prefer the non-
   expect(lockfile.snapshots['@pnpm.e2e/abc@1.0.0(@pnpm.e2e/peer-c@1.0.0)']).toBeTruthy()
 })
 
-test.skip('in a subdependency, when there are several aliased dependencies of the same package, pick the one with the highest version to resolve peers', async () => {
+test('in a subdependency, when there are several aliased dependencies of the same package, pick the one with the highest version to resolve peers', async () => {
   prepareEmpty()
 
   await addDependenciesToPackage({}, ['@pnpm.e2e/abc-parent-with-aliases-of-same-pkg@1.0.0'], testDefaults({ autoInstallPeers: false, strictPeerDependencies: false }))
@@ -1703,7 +1703,7 @@ test('3 circular peers', async () => {
   expect(lockfile.importers?.['.'].dependencies?.['@pnpm.e2e/circular-peers-3-of-3'].version).toBe('1.0.0(@pnpm.e2e/circular-peers-1-of-3@1.0.0)')
 })
 
-test.skip('3 circular peers in workspace root', async () => {
+test('3 circular peers in workspace root', async () => {
   const projects = preparePackages([
     {
       location: '.',
@@ -1798,7 +1798,7 @@ 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.skip('optional peer dependency is resolved if it is installed anywhere in the dependency graph and auto install peers is false', async () => {
+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()
 

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

@@ -165,7 +165,7 @@ export interface ResolutionContext {
   virtualStoreDir: string
   virtualStoreDirMaxLength: number
   workspacePackages?: WorkspacePackages
-  missingPeersOfChildrenByPkgId: Record<PkgResolutionId, { parentImporterId: string, missingPeersOfChildren: MissingPeersOfChildren }>
+  missingPeersOfChildrenByPkgId: Record<PkgResolutionId, { depth: number, missingPeersOfChildren: MissingPeersOfChildren }>
   hoistPeers?: boolean
 }
 
@@ -241,7 +241,6 @@ export interface ResolvedPackage {
     os?: string[]
     libc?: string[]
   }
-  parentImporterIds: Set<string>
 }
 
 type ParentPkg = Pick<PkgAddress, 'nodeId' | 'installable' | 'rootDir' | 'optional' | 'pkgId'>
@@ -519,7 +518,7 @@ function filterMissingPeersFromPkgAddresses (
 ): PkgAddress[] {
   return pkgAddresses.map((pkgAddress) => ({
     ...pkgAddress,
-    missingPeers: pickBy((peer, peerName) => {
+    missingPeers: pickBy((_, peerName) => {
       if (!currentParentPkgAliases[peerName]) return true
       if (currentParentPkgAliases[peerName] !== true) {
         resolvedPeers[peerName] = currentParentPkgAliases[peerName] as PkgAddress
@@ -561,23 +560,28 @@ export async function resolveDependencies (
   const postponedPeersResolutionQueue: PostponedPeersResolutionFunction[] = []
   const pkgAddresses: PkgAddress[] = []
   ;(await Promise.all(
-    extendedWantedDeps.map((extendedWantedDep) => resolveDependenciesOfDependency(
-      ctx,
-      preferredVersions,
-      options,
-      extendedWantedDep
-    ))
-  )).forEach(({ resolveDependencyResult, postponedResolution, postponedPeersResolution }) => {
-    if (resolveDependencyResult) {
-      pkgAddresses.push(resolveDependencyResult as PkgAddress)
-    }
-    if (postponedResolution) {
-      postponedResolutionsQueue.push(postponedResolution)
-    }
-    if (postponedPeersResolution) {
-      postponedPeersResolutionQueue.push(postponedPeersResolution)
-    }
-  })
+    extendedWantedDeps.map(async (extendedWantedDep) => {
+      const {
+        resolveDependencyResult,
+        postponedResolution,
+        postponedPeersResolution,
+      } = await resolveDependenciesOfDependency(
+        ctx,
+        preferredVersions,
+        options,
+        extendedWantedDep
+      )
+      if (resolveDependencyResult) {
+        pkgAddresses.push(resolveDependencyResult as PkgAddress)
+      }
+      if (postponedResolution) {
+        postponedResolutionsQueue.push(postponedResolution)
+      }
+      if (postponedPeersResolution) {
+        postponedPeersResolutionQueue.push(postponedPeersResolution)
+      }
+    })
+  ))
   const newPreferredVersions = Object.create(preferredVersions) as PreferredVersions
   const currentParentPkgAliases: Record<string, PkgAddress | true> = {}
   for (const pkgAddress of pkgAddresses) {
@@ -1349,7 +1353,6 @@ async function resolveDependency (
   const installable = parentIsInstallable && pkgResponse.body.isInstallable !== false
   const isNew = !ctx.resolvedPkgsById[pkgResponse.body.id]
   const parentImporterId = options.parentIds[0]
-  let resolveChildren = false
   const currentIsOptional = wantedDependency.optional || options.parentPkg.optional
 
   if (isNew) {
@@ -1396,11 +1399,6 @@ async function resolveDependency (
     ctx.resolvedPkgsById[pkgResponse.body.id].prod = ctx.resolvedPkgsById[pkgResponse.body.id].prod || !wantedDependency.dev && !wantedDependency.optional
     ctx.resolvedPkgsById[pkgResponse.body.id].dev = ctx.resolvedPkgsById[pkgResponse.body.id].dev || wantedDependency.dev
     ctx.resolvedPkgsById[pkgResponse.body.id].optional = ctx.resolvedPkgsById[pkgResponse.body.id].optional && currentIsOptional
-    if (ctx.hoistPeers) {
-      resolveChildren = !ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved &&
-        !ctx.resolvedPkgsById[pkgResponse.body.id].parentImporterIds.has(parentImporterId)
-      ctx.resolvedPkgsById[pkgResponse.body.id].parentImporterIds.add(parentImporterId)
-    }
     if (ctx.resolvedPkgsById[pkgResponse.body.id].fetching == null && pkgResponse.fetching != null) {
       ctx.resolvedPkgsById[pkgResponse.body.id].fetching = pkgResponse.fetching
       ctx.resolvedPkgsById[pkgResponse.body.id].filesIndexFile = pkgResponse.filesIndexFile!
@@ -1426,7 +1424,13 @@ async function resolveDependency (
   let missingPeersOfChildren!: MissingPeersOfChildren | undefined
   if (ctx.hoistPeers && !options.parentIds.includes(pkgResponse.body.id)) {
     if (ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id]) {
-      if (options.parentIds[0] !== ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].parentImporterId) {
+      // This if condition is used to avoid a dead lock.
+      // There might be a better way to hoist peer dependencies during resolution
+      // but it would probably require a big rewrite of the resolution algorithm.
+      if (
+        ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].depth >= options.currentDepth ||
+        ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved
+      ) {
         missingPeersOfChildren = ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren
       }
     } else {
@@ -1437,7 +1441,7 @@ async function resolveDependency (
         get: pShare(p.promise),
       }
       ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = {
-        parentImporterId,
+        depth: options.currentDepth,
         missingPeersOfChildren,
       }
     }
@@ -1445,7 +1449,7 @@ async function resolveDependency (
   return {
     alias: wantedDependency.alias || pkg.name,
     depIsLinked,
-    isNew: isNew || resolveChildren,
+    isNew,
     nodeId,
     normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined,
     missingPeersOfChildren,
@@ -1522,7 +1526,6 @@ function getResolvedPackage (
       os: options.pkg.os,
       libc: options.pkg.libc,
     },
-    parentImporterIds: new Set([options.parentImporterId]),
     pkgIdWithPatchHash: options.pkgIdWithPatchHash,
     dev: options.wantedDependency.dev,
     fetching: options.pkgResponse.fetching!,

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

@@ -157,7 +157,7 @@ export async function resolveDependencyTree<T> (
     updatedSet: new Set<string>(),
     workspacePackages: opts.workspacePackages,
     missingPeersOfChildrenByPkgId: {},
-    hoistPeers: autoInstallPeers,
+    hoistPeers: autoInstallPeers || opts.dedupePeerDependents,
     allPeerDepNames: new Set(),
   }