fix: don't crash when auto-install-peers is true in a workspace (#5467)

close #5454

.changeset/eight-waves-type.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/resolve-dependencies": patch
+"pnpm": patch
+---
+
+Don't crash when `auto-install-peers` is set to `true` and installation is done on a workspace with that has the same dependencies in multiple projects [#5454](https://github.com/pnpm/pnpm/issues/5454).

packages/core/test/install/autoInstallPeers.ts

@@ -348,6 +348,147 @@ test('installation on a package with many complex circular dependencies does not
   await addDependenciesToPackage({}, ['webpack@4.46.0'], await testDefaults({ autoInstallPeers: true }))
 })
 
+// This test may be removed if autoInstallPeers will become true by default
+test('installation on a workspace with many complex circular dependencies does not fail when auto install peers is on', async () => {
+  prepareEmpty()
+  await mutateModules([
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project1',
+        dependencies: {
+          '@angular/common': '14.2.4',
+          '@angular/core': '14.2.4',
+          '@angular/forms': '14.2.4',
+          '@angular/platform-browser': '14.2.4',
+          '@angular/platform-browser-dynamic': '14.2.4',
+          '@angular/router': '14.2.4',
+          '@capacitor/app': '4.0.1',
+          '@capacitor/core': '4.3.0',
+          '@capacitor/haptics': '4.0.1',
+          '@capacitor/keyboard': '4.0.1',
+          '@capacitor/status-bar': '4.0.1',
+          '@ionic/angular': '6.2.9',
+          '@ionic/core': '6.2.9',
+          ionicons: '6.0.3',
+          'ng-particles': '3.3.3',
+          rxjs: '7.5.7',
+          tslib: '2.4.0',
+          tsparticles: '2.3.4',
+          'tsparticles-engine': '2.3.3',
+          'tsparticles-interaction-external-attract': '2.3.3',
+          'tsparticles-interaction-external-bounce': '2.3.3',
+          'tsparticles-interaction-external-bubble': '2.3.3',
+          'tsparticles-interaction-external-connect': '2.3.3',
+          'tsparticles-interaction-external-grab': '2.3.3',
+          'tsparticles-interaction-external-pause': '2.3.3',
+          'tsparticles-interaction-external-push': '2.3.3',
+          'tsparticles-interaction-external-remove': '2.3.3',
+          'tsparticles-interaction-external-repulse': '2.3.4',
+          'tsparticles-interaction-external-slow': '2.3.3',
+          'tsparticles-interaction-external-trail': '2.3.3',
+          'tsparticles-interaction-particles-attract': '2.3.3',
+          'tsparticles-interaction-particles-collisions': '2.3.3',
+          'tsparticles-interaction-particles-links': '2.3.3',
+          'tsparticles-move-base': '2.3.3',
+          'tsparticles-move-parallax': '2.3.3',
+          'tsparticles-particles.js': '2.3.3',
+          'tsparticles-plugin-absorbers': '2.3.4',
+          'tsparticles-plugin-emitters': '2.3.4',
+          'tsparticles-plugin-polygon-mask': '2.3.3',
+          'tsparticles-shape-circle': '2.3.3',
+          'tsparticles-shape-image': '2.3.3',
+          'tsparticles-shape-line': '2.3.3',
+          'tsparticles-shape-polygon': '2.3.3',
+          'tsparticles-shape-square': '2.3.3',
+          'tsparticles-shape-star': '2.3.3',
+          'tsparticles-shape-text': '2.3.3',
+          'tsparticles-slim': '2.3.4',
+          'tsparticles-updater-angle': '2.3.3',
+          'tsparticles-updater-color': '2.3.3',
+          'tsparticles-updater-destroy': '2.3.3',
+          'tsparticles-updater-life': '2.3.3',
+          'tsparticles-updater-opacity': '2.3.3',
+          'tsparticles-updater-out-modes': '2.3.3',
+          'tsparticles-updater-roll': '2.3.3',
+          'tsparticles-updater-size': '2.3.3',
+          'tsparticles-updater-stroke-color': '2.3.3',
+          'tsparticles-updater-tilt': '2.3.3',
+          'tsparticles-updater-twinkle': '2.3.3',
+          'tsparticles-updater-wobble': '2.3.3',
+          'zone.js': '0.11.8',
+        },
+        devDependencies: {
+          '@angular-devkit/build-angular': '14.2.4',
+          '@angular-eslint/builder': '14.1.2',
+          '@angular-eslint/eslint-plugin': '14.1.2',
+          '@angular-eslint/eslint-plugin-template': '14.1.2',
+          '@angular-eslint/template-parser': '14.1.2',
+          '@angular/cli': '14.2.4',
+          '@angular/compiler': '14.2.4',
+          '@angular/compiler-cli': '14.2.4',
+          '@angular/language-service': '14.2.4',
+          '@capacitor/cli': '4.3.0',
+          '@ionic/angular-toolkit': '7.0.0',
+          '@types/jasmine': '4.3.0',
+          '@types/jasminewd2': '2.0.10',
+          '@types/node': '18.7.23',
+          '@typescript-eslint/eslint-plugin': '5.38.1',
+          '@typescript-eslint/parser': '5.38.1',
+          eslint: '8.24.0',
+          'eslint-plugin-import': '2.26.0',
+          'eslint-plugin-jsdoc': '39.3.6',
+          'eslint-plugin-prefer-arrow': '1.2.3',
+          'jasmine-core': '4.4.0',
+          'jasmine-spec-reporter': '7.0.0',
+          karma: '6.4.1',
+          'karma-chrome-launcher': '3.1.1',
+          'karma-coverage': '2.2.0',
+          'karma-coverage-istanbul-reporter': '3.0.3',
+          'karma-jasmine': '5.1.0',
+          'karma-jasmine-html-reporter': '2.0.0',
+          protractor: '7.0.0',
+          'ts-node': '10.9.1',
+          typescript: '4.8.4',
+        },
+      },
+      mutation: 'install',
+      rootDir: path.resolve('project1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project2',
+        devDependencies: {
+          '@typescript-eslint/eslint-plugin': '5.38.1',
+          '@typescript-eslint/parser': '5.38.1',
+          copyfiles: '2.4.1',
+          enzyme: '3.11.0',
+          'enzyme-adapter-preact-pure': '4.0.1',
+          eslint: '8.24.0',
+          'eslint-config-preact': '1.3.0',
+          'eslint-config-prettier': '8.5.0',
+          'eslint-plugin-react-hooks': '4.6.0',
+          'identity-obj-proxy': '3.0.0',
+          'preact-cli': '3.4.1',
+          prettier: '2.7.1',
+          'sirv-cli': '2.0.2',
+        },
+        dependencies: {
+          preact: '10.11.0',
+          'preact-particles': '2.3.3',
+          'preact-render-to-string': '5.2.4',
+          'preact-router': '4.1.0',
+          tsparticles: '2.3.4',
+          'tsparticles-engine': '2.3.3',
+        },
+      },
+      mutation: 'install',
+      rootDir: path.resolve('project2'),
+    },
+  ], await testDefaults({ autoInstallPeers: true, ignoreScripts: true, lockfileOnly: true }))
+})
+
 test('do not override the direct dependency with an auto installed peer dependency', async () => {
   const includedDeps = new Set([
     '@angular-devkit/build-angular',

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -169,6 +169,7 @@ interface MissingPeersOfChildren {
   resolve: (missingPeers: MissingPeers) => void
   reject: (err: Error) => void
   get: () => Promise<MissingPeers>
+  resolved?: boolean
 }
 
 export type PkgAddress = {
@@ -228,6 +229,7 @@ export interface ResolvedPackage {
     os?: string[]
     libc?: string[]
   }
+  parentImporterIds: Set<string>
 }
 
 type ParentPkg = Pick<PkgAddress, 'nodeId' | 'installable' | 'depPath' | 'rootDir'>
@@ -697,7 +699,10 @@ async function resolveDependenciesOfDependency (
     resolveDependencyResult,
     postponedResolution: async (postponedResolutionOpts) => {
       const { missingPeers, resolvedPeers } = await postponedResolution(postponedResolutionOpts)
-      resolveDependencyResult.missingPeersOfChildren!.resolve(missingPeers)
+      if (resolveDependencyResult.missingPeersOfChildren) {
+        resolveDependencyResult.missingPeersOfChildren.resolved = true
+        resolveDependencyResult.missingPeersOfChildren.resolve(missingPeers)
+      }
       return filterMissingPeers({ missingPeers, resolvedPeers }, postponedResolutionOpts.parentPkgAliases)
     },
   }
@@ -1213,6 +1218,8 @@ async function resolveDependency (
   const parentIsInstallable = options.parentPkg.installable === undefined || options.parentPkg.installable
   const installable = parentIsInstallable && pkgResponse.body.isInstallable !== false
   const isNew = !ctx.resolvedPackagesByDepPath[depPath]
+  const parentImporterId = options.parentPkg.nodeId.substring(0, options.parentPkg.nodeId.indexOf('>', 1) + 1)
+  let resolveChildren = false
 
   if (isNew) {
     if (
@@ -1251,11 +1258,17 @@ async function resolveDependency (
       pkgResponse,
       prepare,
       wantedDependency,
+      parentImporterId,
     })
   } else {
     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 && wantedDependency.optional
+    if (ctx.autoInstallPeers) {
+      resolveChildren = !ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved &&
+        !ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.has(parentImporterId)
+      ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.add(parentImporterId)
+    }
     if (ctx.resolvedPackagesByDepPath[depPath].fetchingFiles == null && pkgResponse.files != null) {
       ctx.resolvedPackagesByDepPath[depPath].fetchingFiles = pkgResponse.files
       ctx.resolvedPackagesByDepPath[depPath].filesIndexFile = pkgResponse.filesIndexFile!
@@ -1280,7 +1293,7 @@ async function resolveDependency (
     ? path.resolve(ctx.lockfileDir, pkgResponse.body.resolution['directory'])
     : options.prefix
   let missingPeersOfChildren!: MissingPeersOfChildren | undefined
-  if (!nodeIdContains(options.parentPkg.nodeId, depPath)) {
+  if (ctx.autoInstallPeers && !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
@@ -1293,7 +1306,7 @@ async function resolveDependency (
         get: pShare(p.promise),
       }
       ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = {
-        parentImporterId: options.parentPkg.nodeId.substring(0, options.parentPkg.nodeId.indexOf('>', 1) + 1),
+        parentImporterId,
         missingPeersOfChildren,
       }
     }
@@ -1302,7 +1315,7 @@ async function resolveDependency (
     alias: wantedDependency.alias || pkg.name,
     depIsLinked,
     depPath,
-    isNew,
+    isNew: isNew || resolveChildren,
     nodeId,
     normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined,
     missingPeersOfChildren,
@@ -1354,6 +1367,7 @@ function getResolvedPackage (
     depPath: string
     force: boolean
     hasBin: boolean
+    parentImporterId: string
     patchFile?: PatchFile
     pkg: PackageManifest
     pkgResponse: PackageResponse
@@ -1377,6 +1391,7 @@ function getResolvedPackage (
       os: options.pkg.os,
       libc: options.pkg.libc,
     },
+    parentImporterIds: new Set([options.parentImporterId]),
     depPath: options.depPath,
     dev: options.wantedDependency.dev,
     fetchingBundledManifest: options.pkgResponse.bundledManifest,