From 9f003e94fa9e4dcd113b944845260795e72916f6 Mon Sep 17 00:00:00 2001
From: Zoltan Kochan <zkochan@users.noreply.github.com>
Date: Wed, 14 Oct 2020 21:06:55 +0300
Subject: [PATCH] fix: caching during peer resolution

close #2919
PR #2927
---
 .changeset/late-items-fail.md                 |   5 +
 .../resolve-dependencies/src/resolvePeers.ts  |  11 +-
 .../resolve-dependencies/test/resolvePeers.ts | 101 ++++++++++++++++++
 3 files changed, 111 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/late-items-fail.md

diff --git a/.changeset/late-items-fail.md b/.changeset/late-items-fail.md
new file mode 100644
index 0000000000..b5554d83d7
--- /dev/null
+++ b/.changeset/late-items-fail.md
@@ -0,0 +1,5 @@
+---
+"@pnpm/resolve-dependencies": patch
+---
+
+Don't cache the peer resolution of packages that have missing peer dependencies.
diff --git a/packages/resolve-dependencies/src/resolvePeers.ts b/packages/resolve-dependencies/src/resolvePeers.ts
index a17a0929dd..fa07acaf18 100644
--- a/packages/resolve-dependencies/src/resolvePeers.ts
+++ b/packages/resolve-dependencies/src/resolvePeers.ts
@@ -218,13 +218,9 @@ function resolvePeersOfNode<T extends PartialResolvedPackage> (
   let modules: string
   let depPath: string
   const localLocation = path.join(ctx.virtualStoreDir, pkgIdToFilename(resolvedPackage.depPath, ctx.lockfileDir))
-  const isPure = R.isEmpty(allResolvedPeers)
-  if (isPure) {
+  if (R.isEmpty(allResolvedPeers)) {
     modules = path.join(localLocation, 'node_modules')
     depPath = resolvedPackage.depPath
-    if (R.isEmpty(resolvedPackage.peerDependencies)) {
-      ctx.purePkgs.add(resolvedPackage.depPath)
-    }
   } else {
     const peersFolderSuffix = createPeersFolderSuffix(
       Object.keys(allResolvedPeers).map((alias) => ({
@@ -234,7 +230,10 @@ function resolvePeersOfNode<T extends PartialResolvedPackage> (
     modules = path.join(`${localLocation}${peersFolderSuffix}`, 'node_modules')
     depPath = `${resolvedPackage.depPath}${peersFolderSuffix}`
   }
-  if (!isPure || !R.isEmpty(resolvedPackage.peerDependencies)) {
+  const isPure = R.isEmpty(allResolvedPeers) && allMissingPeers.length === 0
+  if (isPure) {
+    ctx.purePkgs.add(resolvedPackage.depPath)
+  } else {
     const cache = {
       missingPeers: allMissingPeers,
       depPath,
diff --git a/packages/resolve-dependencies/test/resolvePeers.ts b/packages/resolve-dependencies/test/resolvePeers.ts
index 98fd8b3589..b94ca5aaa7 100644
--- a/packages/resolve-dependencies/test/resolvePeers.ts
+++ b/packages/resolve-dependencies/test/resolvePeers.ts
@@ -107,3 +107,104 @@ test('resolve peer dependencies of cyclic dependencies', (t) => {
   ])
   t.end()
 })
+
+test('when a package is referenced twice in the dependencies graph and one of the times it cannot resolve its peers, still try to resolve it in the other occurence', (t) => {
+  const fooPkg = {
+    name: 'foo',
+    depPath: 'foo/1.0.0',
+    version: '1.0.0',
+    peerDependencies: {
+      qar: '1.0.0',
+    },
+  }
+  const barPkg = {
+    name: 'bar',
+    depPath: 'bar/1.0.0',
+    version: '1.0.0',
+    peerDependencies: {
+      foo: '1.0.0',
+      qar: '1.0.0',
+    } as Record<string, string>,
+  }
+  const zooPkg = {
+    name: 'zoo',
+    depPath: 'zoo/1.0.0',
+    version: '1.0.0',
+    peerDependencies: {} as Record<string, string>,
+  }
+  const { dependenciesGraph } = resolvePeers({
+    projects: [
+      {
+        directNodeIdsByAlias: {
+          zoo: 'zoo/1.0.0',
+          bar: 'bar/1.0.0',
+        },
+        topParents: [],
+        rootDir: '',
+        id: '',
+      },
+    ],
+    dependenciesTree: {
+      'zoo/1.0.0': {
+        children: {
+          foo: 'zoo/1.0.0>foo/1.0.0',
+        },
+        installable: true,
+        resolvedPackage: zooPkg,
+        depth: 0,
+      },
+      'zoo/1.0.0>foo/1.0.0': {
+        children: {},
+        installable: true,
+        resolvedPackage: fooPkg,
+        depth: 1,
+      },
+      'bar/1.0.0': {
+        children: {
+          zoo: 'bar/1.0.0>zoo/1.0.0',
+          qar: 'bar/1.0.0>qar/1.0.0',
+        },
+        installable: true,
+        resolvedPackage: barPkg,
+        depth: 0,
+      },
+      'bar/1.0.0>zoo/1.0.0': {
+        children: {
+          foo: 'bar/1.0.0>zoo/1.0.0>foo/1.0.0',
+        },
+        installable: true,
+        resolvedPackage: zooPkg,
+        depth: 1,
+      },
+      'bar/1.0.0>zoo/1.0.0>foo/1.0.0': {
+        children: {},
+        installable: true,
+        resolvedPackage: fooPkg,
+        depth: 2,
+      },
+      'bar/1.0.0>qar/1.0.0': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'qar',
+          depPath: 'qar/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+        },
+        depth: 1,
+      },
+    },
+    virtualStoreDir: '',
+    lockfileDir: '',
+    strictPeerDependencies: false,
+  })
+  t.deepEqual(Object.keys(dependenciesGraph), [
+    'foo/1.0.0',
+    'zoo/1.0.0',
+    'foo/1.0.0_qar@1.0.0',
+    'zoo/1.0.0_qar@1.0.0',
+    'qar/1.0.0',
+    'bar/1.0.0',
+  ])
+  t.end()
+})