fix: named update of package that is a resolved peer dependency

packages/headless/package.json

@@ -38,7 +38,7 @@
     "isexe": "2.0.0",
     "mz": "2.7.0",
     "npm-run-all": "4.1.3",
-    "pnpm-registry-mock": "2.2.0",
+    "pnpm-registry-mock": "2.4.0",
     "rimraf": "2.6.2",
     "rimraf-then": "1.0.1",
     "sinon": "7.1.1",

packages/pnpm/package.json

@@ -112,7 +112,7 @@
     "package-preview": "1.0.6",
     "path-exists": "3.0.0",
     "pnpm": "link:",
-    "pnpm-registry-mock": "2.2.0",
+    "pnpm-registry-mock": "2.4.0",
     "retry": "0.12.0",
     "rimraf": "2.6.2",
     "rimraf-then": "1.0.1",

packages/resolve-dependencies/src/index.ts

@@ -102,6 +102,7 @@ export default async function (
       hasManifestInShrinkwrap: opts.hasManifestInShrinkwrap,
       keypath: [],
       localPackages: opts.localPackages,
+      parentDependsOnPeers: true,
       parentNodeId: ROOT_NODE_ID,
       readPackageHook: opts.hooks.readPackage,
       resolvedDependencies: {

packages/resolve-dependencies/src/resolveDependencies.ts

@@ -169,6 +169,7 @@ export default async function resolveDependencies (
   wantedDependencies: WantedDependency[],
   options: {
     keypath: string[],
+    parentDependsOnPeers: boolean,
     parentNodeId: string,
     currentDepth: number,
     resolvedDependencies?: ResolvedDependencies,
@@ -189,10 +190,10 @@ export default async function resolveDependencies (
   const preferedDependencies = options.preferedDependencies || {}
   const update = options.update && options.currentDepth <= ctx.depth
   const extendedWantedDeps = []
-  const allPeerDependencies = new Set<string>()
+  let proceedAll = false
   for (const wantedDependency of wantedDependencies) {
     let reference = wantedDependency.alias && resolvedDependencies[wantedDependency.alias]
-    let proceed = false
+    let proceed = options.parentDependsOnPeers
 
     // If dependencies that were used by the previous version of the package
     // satisfy the newer version's requirements, then pnpm tries to keep
@@ -215,10 +216,8 @@ export default async function resolveDependencies (
       reference = preferedDependencies[wantedDependency.alias]
     }
     const infoFromShrinkwrap = getInfoFromShrinkwrap(ctx.wantedShrinkwrap, ctx.registries.default, reference, wantedDependency.alias)
-    if (infoFromShrinkwrap && infoFromShrinkwrap.dependencyShrinkwrap) {
-      for (const peer of Object.keys(infoFromShrinkwrap.dependencyShrinkwrap.peerDependencies || {})) {
-        allPeerDependencies.add(peer)
-      }
+    if (infoFromShrinkwrap && infoFromShrinkwrap.dependencyShrinkwrap && infoFromShrinkwrap.dependencyShrinkwrap.id) {
+      proceedAll = true
     }
     extendedWantedDeps.push({
       infoFromShrinkwrap,
@@ -227,23 +226,27 @@ export default async function resolveDependencies (
       wantedDependency,
     })
   }
+  const resolveDepOpts = {
+    currentDepth: options.currentDepth,
+    hasManifestInShrinkwrap: options.hasManifestInShrinkwrap,
+    keypath: options.keypath,
+    localPackages: options.localPackages,
+    parentDependsOnPeer: options.parentDependsOnPeers,
+    parentIsInstallable: options.parentIsInstallable,
+    parentNodeId: options.parentNodeId,
+    readPackageHook: options.readPackageHook,
+    shamefullyFlatten: options.shamefullyFlatten,
+    sideEffectsCache: options.sideEffectsCache,
+    update,
+  }
   const pkgAddresses = (
     await Promise.all(
       extendedWantedDeps
-        .map(async (extendedWantedDeps) => {
-          return install(extendedWantedDeps.wantedDependency, ctx, {
-            currentDepth: options.currentDepth,
-            hasManifestInShrinkwrap: options.hasManifestInShrinkwrap,
-            keypath: options.keypath,
-            localPackages: options.localPackages,
-            parentIsInstallable: options.parentIsInstallable,
-            parentNodeId: options.parentNodeId,
-            proceed: extendedWantedDeps.proceed || Boolean(extendedWantedDeps.wantedDependency.alias && allPeerDependencies.has(extendedWantedDeps.wantedDependency.alias)),
-            readPackageHook: options.readPackageHook,
-            shamefullyFlatten: options.shamefullyFlatten,
-            sideEffectsCache: options.sideEffectsCache,
-            update,
-            ...extendedWantedDeps.infoFromShrinkwrap,
+        .map(async (extendedWantedDep) => {
+          return resolveDependency(extendedWantedDep.wantedDependency, ctx, {
+            ...resolveDepOpts,
+            ...extendedWantedDep.infoFromShrinkwrap,
+            proceed: extendedWantedDep.proceed || proceedAll,
           })
         }),
     )
@@ -315,14 +318,15 @@ function getInfoFromShrinkwrap (
   }
 }
 
-async function install (
+async function resolveDependency (
   wantedDependency: WantedDependency,
   ctx: ResolutionContext,
   options: {
     keypath: string[], // TODO: remove. Currently used only for logging
     pkgId?: string,
     depPath?: string,
     relDepPath?: string,
+    parentDependsOnPeer: boolean,
     parentNodeId: string,
     currentDepth: number,
     dependencyShrinkwrap?: DependencyShrinkwrap,
@@ -412,7 +416,7 @@ async function install (
 
   pkgResponse.body.id = encodePkgId(pkgResponse.body.id)
 
-  if (!pkgResponse.body.updated && options.update && options.currentDepth >= ctx.depth && options.relDepPath &&
+  if (!options.parentDependsOnPeer && !pkgResponse.body.updated && options.update && options.currentDepth >= ctx.depth && options.relDepPath &&
     ctx.currentShrinkwrap.packages && ctx.currentShrinkwrap.packages[options.relDepPath] && !ctx.force) {
     return null
   }
@@ -606,6 +610,7 @@ async function install (
         hasManifestInShrinkwrap: options.hasManifestInShrinkwrap,
         keypath: options.keypath.concat([ pkgResponse.body.id ]),
         optionalDependencyNames: options.optionalDependencyNames,
+        parentDependsOnPeers: Boolean(options.dependencyShrinkwrap && options.dependencyShrinkwrap.id),
         parentIsInstallable: installable,
         parentNodeId: nodeId,
         preferedDependencies: pkgResponse.body.updated
@@ -696,6 +701,7 @@ async function resolveDependenciesOfPackage (
     resolvedDependencies?: ResolvedDependencies,
     preferedDependencies?: ResolvedDependencies,
     optionalDependencyNames?: string[],
+    parentDependsOnPeers: boolean,
     parentIsInstallable: boolean,
     update: boolean,
     readPackageHook?: ReadPackageHook,

packages/supi/package.json

@@ -103,7 +103,7 @@
     "package-preview": "1.0.6",
     "path-exists": "3.0.0",
     "path-name": "1.0.0",
-    "pnpm-registry-mock": "2.2.0",
+    "pnpm-registry-mock": "2.4.0",
     "read-pkg": "4.0.1",
     "rimraf": "2.6.2",
     "sepia": "2.0.2",

packages/supi/test/install/peerDependencies.ts

@@ -209,6 +209,7 @@ async function okFile (t: tape.Test, filename: string) {
 // This usecase was failing. See https://github.com/pnpm/supi/issues/15
 test('peer dependencies are linked when running one named installation', async (t: tape.Test) => {
   await addDistTag({ package: 'peer-a', version: '1.0.0', distTag: 'latest' })
+  await addDistTag({ package: 'peer-c', version: '1.0.0', distTag: 'latest' })
 
   const project = prepare(t)
 
@@ -239,6 +240,7 @@ test('peer dependencies are linked when running one named installation', async (
 
 test('peer dependencies are linked when running two separate named installations', async (t: tape.Test) => {
   await addDistTag({ package: 'peer-a', version: '1.0.0', distTag: 'latest' })
+  await addDistTag({ package: 'peer-c', version: '1.0.0', distTag: 'latest' })
   const project = prepare(t)
 
   await installPkgs(['abc-grand-parent-with-c', 'peer-c@2.0.0'], await testDefaults())
@@ -473,3 +475,59 @@ test('external shrinkwrap: peer dependency is grouped with dependent even after
     })
   }
 })
+
+test('external shrinkwrap: peer dependency is grouped with dependent even after a named update of the resolved package', async (t: tape.Test) => {
+  const project = prepare(t)
+  await mkdir('_')
+  process.chdir('_')
+  const shrinkwrapDirectory = path.resolve('..')
+
+  await installPkgs(['peer-c@1.0.0', 'abc-parent-with-ab@1.0.0'], await testDefaults({ shrinkwrapDirectory }))
+
+  {
+    const shr = await loadYamlFile(path.resolve('..', 'shrinkwrap.yaml'))
+    t.deepEqual(shr['importers']['_'], {
+      dependencies: {
+        'abc-parent-with-ab': '/abc-parent-with-ab/1.0.0/peer-c@1.0.0',
+        'peer-c': '1.0.0',
+      },
+      specifiers: {
+        'abc-parent-with-ab': '^1.0.0',
+        'peer-c': '^1.0.0',
+      },
+    })
+  }
+
+  await installPkgs(['peer-c@2.0.0'], await testDefaults({ shrinkwrapDirectory }))
+
+  {
+    const shr = await loadYamlFile(path.resolve('..', 'shrinkwrap.yaml'))
+    t.deepEqual(shr['importers']['_'], {
+      dependencies: {
+        'abc-parent-with-ab': '/abc-parent-with-ab/1.0.0/peer-c@2.0.0',
+        'peer-c': '2.0.0',
+      },
+      specifiers: {
+        'abc-parent-with-ab': '^1.0.0',
+        'peer-c': '^2.0.0',
+      },
+    })
+  }
+
+  t.ok(await exists(path.join('..', NM, '.localhost+4873', 'abc-parent-with-ab', '1.0.0', 'peer-c@2.0.0', NM, 'is-positive')))
+})
+
+test('regular dependencies are not removed on update from transitive packages that have children with peers resolved from above', async (t: tape.Test) => {
+  const project = prepare(t)
+  await mkdir('_')
+  process.chdir('_')
+  const shrinkwrapDirectory = path.resolve('..')
+  await addDistTag({ package: 'peer-c', version: '1.0.0', distTag: 'latest' })
+
+  await installPkgs(['abc-grand-parent-with-c@1.0.0'], await testDefaults({ shrinkwrapDirectory }))
+
+  await addDistTag({ package: 'peer-c', version: '1.0.1', distTag: 'latest' })
+  await install(await testDefaults({ shrinkwrapDirectory, update: true, depth: 2 }))
+
+  t.ok(await exists(path.join('..', NM, '.localhost+4873', 'abc-parent-with-ab', '1.0.1', 'peer-c@1.0.1', NM, 'is-positive')))
+})

packages/supi/test/install/update.ts

@@ -10,26 +10,28 @@ import {
 } from '../utils'
 
 const test = promisifyTape(tape)
+const testOnly = promisifyTape(tape.only)
 
 test('preserve subdeps on update', async (t: tape.Test) => {
   const project = prepare(t)
 
   await Promise.all([
-    addDistTag('foobarqar', '1.0.0', 'latest'),
-    addDistTag('foo', '100.0.0', 'latest'),
-    addDistTag('bar', '100.0.0', 'latest'),
     addDistTag('abc-grand-parent-with-c', '1.0.0', 'latest'),
     addDistTag('abc-parent-with-ab', '1.0.0', 'latest'),
+    addDistTag('bar', '100.0.0', 'latest'),
+    addDistTag('foo', '100.0.0', 'latest'),
+    addDistTag('foobarqar', '1.0.0', 'latest'),
+    addDistTag('peer-c', '1.0.0', 'latest'),
   ])
 
   await installPkgs(['foobarqar', 'abc-grand-parent-with-c'], await testDefaults())
 
   await Promise.all([
-    addDistTag('foobarqar', '1.0.1', 'latest'),
-    addDistTag('foo', '100.1.0', 'latest'),
-    addDistTag('bar', '100.1.0', 'latest'),
     addDistTag('abc-grand-parent-with-c', '1.0.1', 'latest'),
     addDistTag('abc-parent-with-ab', '1.0.1', 'latest'),
+    addDistTag('bar', '100.1.0', 'latest'),
+    addDistTag('foo', '100.1.0', 'latest'),
+    addDistTag('foobarqar', '1.0.1', 'latest'),
   ])
 
   await install(await testDefaults({ update: true, depth: 0 }))

shrinkwrap.yaml

@@ -300,7 +300,7 @@ importers:
       isexe: 2.0.0
       mz: 2.7.0
       npm-run-all: 4.1.3
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       rimraf: 2.6.2
       rimraf-then: 1.0.1
       sinon: 7.1.1
@@ -348,7 +348,7 @@ importers:
       p-limit: 2.0.0
       package-store: 0.23.6
       path-exists: 3.0.0
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       pnpm-shrinkwrap: 8.1.2
       ramda: 0.25.0
       rimraf: 2.6.2
@@ -803,7 +803,7 @@ importers:
       package-preview: 1.0.6
       path-exists: 3.0.0
       pnpm: 'link:'
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       retry: 0.12.0
       rimraf: 2.6.2
       rimraf-then: 1.0.1
@@ -899,7 +899,7 @@ importers:
       pnpm-default-reporter: 0.20.11
       pnpm-file-reporter: 0.1.0
       pnpm-list: 4.1.4
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       process-exists: 3.1.0
       ramda: 0.25.0
       read-ini-file: 1.0.0
@@ -1188,7 +1188,7 @@ importers:
       package-preview: 1.0.6
       path-exists: 3.0.0
       path-name: 1.0.0
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       read-pkg: 4.0.1
       rimraf: 2.6.2
       sepia: 2.0.2
@@ -1275,7 +1275,7 @@ importers:
       path-absolute: 1.0.0
       path-exists: 3.0.0
       path-name: 1.0.0
-      pnpm-registry-mock: 2.2.0
+      pnpm-registry-mock: 2.4.0
       pnpm-shrinkwrap: 8.1.2
       ramda: 0.25.0
       read-pkg: 4.0.1
@@ -7627,7 +7627,7 @@ packages:
     dev: false
     resolution:
       integrity: sha1-dB2ZeXYv362T8+Rp3rSoFNNDAAg=
-  /pnpm-registry-mock/2.2.0:
+  /pnpm-registry-mock/2.4.0:
     dependencies:
       anonymous-npm-registry-client: 0.1.2
       cpr: 3.0.1
@@ -7638,7 +7638,7 @@ packages:
       node: '>=6'
     hasBin: true
     resolution:
-      integrity: sha512-P+0pikpHEIaEW76sfRN7zfGwQ6evnSyAKSkmaKPuvqMX0/KyH8Mt/ulZmS9jNVmV9tLzZ1pZB4LOS9TN8znpAA==
+      integrity: sha512-l6RD8tioFgZo2Ouvw+iE4FAtHaO0jiTqe5nQ9WA5lA/V8ESoA8HcPAqOfNTAKi+ij1OvNrTbMJwPRo/mmYEUcw==
   /posix-character-classes/0.1.1:
     dev: false
     engines: