fix: out-of-memory on peers resolution (#8457)

.changeset/silver-eels-switch.md

@@ -0,0 +1,62 @@
+---
+"@pnpm/resolve-dependencies": major
+"@pnpm/core": minor
+"pnpm": minor
+---
+
+**Minor breaking change.** This change might result in resolving your peer dependencies slightly differently but we don't expect it to introduce issues.
+
+We had to optimize how we resolve peer dependencies in order to fix some [infinite loops and out-of-memory errors during peer dependencies resolution](https://github.com/pnpm/pnpm/issues/8370).
+
+When a peer dependency is a prod dependency somewhere in the dependency graph (with the same version), pnpm will resolve the peers of that peer dependency in the same way across the subgraph.
+
+For example, we have `react-dom` in the peer deps of the `form` and `button` packages. `card` has `react-dom` and `react` as regular dependencies and `card` is a dependency of `form`.
+
+These are the direct dependencies of our example project:
+
+```
+form
+react@16
+react-dom@16
+```
+
+These are the dependencies of card:
+
+```
+button
+react@17
+react-dom@16
+```
+
+When resolving peers, pnpm will not re-resolve `react-dom` for `card`, even though `card` shadows `react@16` from the root with `react@17`. So, all 3 packages (`form`, `card`, and `button`) will use `react-dom@16`, which in turn uses `react@16`. `form` will use `react@16`, while `card` and `button` will use `react@17`.
+
+Before this optimization `react-dom@16` was duplicated for the `card`, so that `card` and `button` would use a `react-dom@16` instance that uses `react@17`.
+
+Before the change:
+
+```
+form
+-> react-dom@16(react@16)
+-> react@16
+card
+-> react-dom@16(react@17)
+-> react@17
+button
+-> react-dom@16(react@17)
+-> react@17
+```
+
+After the change
+
+```
+form
+-> react-dom@16(react@16)
+-> react@16
+card
+-> react-dom@16(react@16)
+-> react@17
+button
+-> react-dom@16(react@16)
+-> react@17
+```
+

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

@@ -1829,7 +1829,8 @@ 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('peer dependency cache is invalidated correctly when the peer of a peer mismatch', async () => {
+// We need to hoist peer dependencies. Otherwise the peers resolution stage would consume too much memory.
+test('peer dependency is hoisted', async () => {
   const project = prepareEmpty()
 
   await addDependenciesToPackage(
@@ -1838,6 +1839,55 @@ test('peer dependency cache is invalidated correctly when the peer of a peer mis
     testDefaults({ autoInstallPeers: true })
   )
 
+  const lockfile = project.readLockfile()
+  expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@1.0.0))']).toBeTruthy()
+  expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@2.0.0))']).toBeFalsy()
+})
+
+test('peer dependency cache is invalidated correctly when the peer of a peer mismatch', async () => {
+  const project = prepareEmpty()
+
+  const allProjects: ProjectOptions[] = [
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-1',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/repeat-peers.a': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-1') as ProjectRootDir,
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-2',
+        version: '1.0.0',
+
+        dependencies: {
+          '@pnpm.e2e/repeat-peers.x': '1.0.0',
+        },
+      },
+      rootDir: path.resolve('project-2') as ProjectRootDir,
+    },
+  ]
+  const importers: MutatedProject[] = [
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-1') as ProjectRootDir,
+    },
+    {
+      mutation: 'install',
+      rootDir: path.resolve('project-2') as ProjectRootDir,
+    },
+  ]
+  await mutateModules(importers, testDefaults({
+    allProjects,
+    autoInstallPeers: true,
+  }))
+
   const lockfile = project.readLockfile()
   expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@1.0.0))']).toBeTruthy()
   expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@2.0.0))']).toBeTruthy()

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

@@ -410,12 +410,13 @@ async function resolvePeersOfNode<T extends PartialResolvedPackage> (
       }
     }
     const newParentPkgs = toPkgByName(parentPkgNodes)
+    const _parentPkgsMatch = parentPkgsMatch.bind(null, ctx.dependenciesTree)
     for (const [newParentPkgName, newParentPkg] of Object.entries(newParentPkgs)) {
       if (parentPkgs[newParentPkgName]) {
-        if (parentPkgs[newParentPkgName].version !== newParentPkg.version) {
+        if (!_parentPkgsMatch(parentPkgs[newParentPkgName], newParentPkg)) {
           newParentPkg.occurrence = parentPkgs[newParentPkgName].occurrence + 1
+          parentPkgs[newParentPkgName] = newParentPkg
         }
-        parentPkgs[newParentPkgName] = newParentPkg
       } else {
         parentPkgs[newParentPkgName] = newParentPkg
       }
@@ -603,6 +604,24 @@ async function resolvePeersOfNode<T extends PartialResolvedPackage> (
   }
 }
 
+function parentPkgsMatch<T> (
+  dependenciesTree: DependenciesTree<T>,
+  currentParentPkg: ParentRef,
+  newParentPkg: ParentRef
+) {
+  if (
+    currentParentPkg.version !== newParentPkg.version ||
+    currentParentPkg.alias !== newParentPkg.alias
+  ) {
+    return false
+  }
+  const currentParentResolvedPkg = currentParentPkg.nodeId && dependenciesTree.get(currentParentPkg.nodeId)?.resolvedPackage
+  if (currentParentResolvedPkg == null) return true
+  const newParentResolvedPkg = newParentPkg.nodeId && dependenciesTree.get(newParentPkg.nodeId)?.resolvedPackage
+  if (newParentResolvedPkg == null) return true
+  return currentParentResolvedPkg.name === newParentResolvedPkg.name
+}
+
 function findHit<T extends PartialResolvedPackage> (ctx: {
   parentPkgsOfNode: ParentPkgsOfNode
   peersCache: PeersCache

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

@@ -118,7 +118,7 @@ test('resolve peer dependencies of cyclic dependencies', async () => {
     'qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0)',
     'zoo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0))',
     'foo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0))(zoo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0)))',
-    'bar/1.0.0(foo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0))(zoo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0))))(zoo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0)))',
+    'bar/1.0.0(foo/1.0.0)(zoo/1.0.0(qar/1.0.0(bar/1.0.0(foo/1.0.0))(foo/1.0.0)))',
   ])
 })