fix: auto install of peer dependencies (#4776)

close #4684
pnpm · May 22, 2022 · 53b92ff · 53b92ff
1 parent b960513
commit 53b92ff
Show file tree

Hide file tree

Showing 33 changed files with 248 additions and 135 deletions.
diff --git a/.changeset/chilly-seahorses-visit.md b/.changeset/chilly-seahorses-visit.md
@@ -0,0 +1,6 @@
+---
+"@pnpm/core": minor
+"@pnpm/resolve-dependencies": minor
+---
+
+New option added for automatically installing missing peer dependencies: `autoInstallPeers`.
diff --git a/.changeset/forty-fireants-fail.md b/.changeset/forty-fireants-fail.md
@@ -0,0 +1,5 @@
+---
+"@pnpm/prune-lockfile": patch
+---
+
+Don't prune peer deps.
diff --git a/.changeset/giant-mice-bathe.md b/.changeset/giant-mice-bathe.md
@@ -0,0 +1,6 @@
+---
+"@pnpm/plugin-commands-installation": patch
+"pnpm": patch
+---
+
+When `auto-install-peers` is set to `true`, automatically install missing peer dependencies without writing them to `package.json` as dependencies. This makes pnpm handle peer dependencies the same way as npm v7 [#4776](https://github.com/pnpm/pnpm/pull/4776).
diff --git a/.changeset/quick-suits-yell.md b/.changeset/quick-suits-yell.md
@@ -0,0 +1,5 @@
+---
+"@pnpm/default-reporter": patch
+---
+
+Add hints to the peer dependencies error.
diff --git a/package.json b/package.json
@@ -37,7 +37,7 @@
     "@commitlint/prompt-cli": "^16.0.0",
     "@pnpm/eslint-config": "workspace:*",
     "@pnpm/meta-updater": "0.0.6",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/tsconfig": "workspace:*",
     "@types/jest": "^27.4.0",
     "@types/node": "^14.17.32",
@@ -120,7 +120,9 @@
         "@typescript-eslint/eslint-plugin": "^5.6.0",
         "@yarnpkg/core": "*"
       },
-      "ignoreMissing": ["@yarnpkg/plugin-patch"]
+      "ignoreMissing": [
+        "@yarnpkg/plugin-patch"
+      ]
     }
   }
 }
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -75,7 +75,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/package-store": "workspace:12.1.15",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/store-path": "^5.0.0",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/fs-extra": "^9.0.5",

diff --git a/packages/core/src/install/extendInstallOptions.ts b/packages/core/src/install/extendInstallOptions.ts
@@ -16,6 +16,7 @@ import pnpmPkgJson from '../pnpmPkgJson'
 import { ReporterFunction } from '../types'
 
 export interface StrictInstallOptions {
+  autoInstallPeers: boolean
   forceSharedLockfile: boolean
   frozenLockfile: boolean
   frozenLockfileIfExists: boolean
@@ -108,6 +109,7 @@ const defaults = async (opts: InstallOptions) => {
     version: pnpmPkgJson.version,
   }
   return {
+    autoInstallPeers: false,
     childConcurrency: 5,
     depth: 0,
     enablePnp: false,

diff --git a/packages/core/src/install/index.ts b/packages/core/src/install/index.ts
@@ -738,6 +738,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
     projects,
     {
       allowBuild: createAllowBuildFunction(opts),
+      autoInstallPeers: opts.autoInstallPeers,
       currentLockfile: ctx.currentLockfile,
       defaultUpdateDepth: (opts.update || (opts.updateMatching != null)) ? opts.depth : -1,
       dryRun: opts.lockfileOnly,

diff --git a/packages/core/test/install/autoInstallPeers.ts b/packages/core/test/install/autoInstallPeers.ts
@@ -0,0 +1,37 @@
+import { addDependenciesToPackage } from '@pnpm/core'
+import { prepareEmpty } from '@pnpm/prepare'
+import { addDistTag } from '@pnpm/registry-mock'
+import { testDefaults } from '../utils'
+
+test('auto install non-optional peer dependencies', async () => {
+  await addDistTag({ package: 'peer-a', version: '1.0.0', distTag: 'latest' })
+  const project = prepareEmpty()
+  await addDependenciesToPackage({}, ['abc-optional-peers@1.0.0'], await testDefaults({ autoInstallPeers: true }))
+  const lockfile = await project.readLockfile()
+  expect(Object.keys(lockfile.packages)).toStrictEqual([
+    '/abc-optional-peers/1.0.0_peer-a@1.0.0',
+    '/peer-a/1.0.0',
+  ])
+})
+
+test('auto install the common peer dependency', async () => {
+  await addDistTag({ package: 'peer-c', version: '1.0.1', distTag: 'latest' })
+  const project = prepareEmpty()
+  await addDependenciesToPackage({}, ['wants-peer-c-1', 'wants-peer-c-1.0.0'], await testDefaults({ autoInstallPeers: true }))
+  const lockfile = await project.readLockfile()
+  expect(Object.keys(lockfile.packages)).toStrictEqual([
+    '/peer-c/1.0.0',
+    '/wants-peer-c-1.0.0/1.0.0_peer-c@1.0.0',
+    '/wants-peer-c-1/1.0.0_peer-c@1.0.0',
+  ])
+})
+
+test('do not auto install when there is no common peer dependency range intersection', async () => {
+  const project = prepareEmpty()
+  await addDependenciesToPackage({}, ['wants-peer-c-1', 'wants-peer-c-2'], await testDefaults({ autoInstallPeers: true }))
+  const lockfile = await project.readLockfile()
+  expect(Object.keys(lockfile.packages)).toStrictEqual([
+    '/wants-peer-c-1/1.0.0',
+    '/wants-peer-c-2/1.0.0',
+  ])
+})
diff --git a/packages/core/test/install/peerDependencies.ts b/packages/core/test/install/peerDependencies.ts
@@ -1190,7 +1190,7 @@ test('peer dependency that is resolved by a dev dependency', async () => {
   ], await testDefaults({ fastUnpack: false, lockfileOnly: true }))
 
   const lockfile = await project.readLockfile()
-  expect(lockfile.packages['/@types/mongoose/5.7.32'].dev).toBeTruthy()
+  expect(lockfile.packages['/@types/mongoose/5.7.32'].dev).toBeUndefined()
 
   await mutateModules([
     {

diff --git a/packages/default-reporter/src/reportError.ts b/packages/default-reporter/src/reportError.ts
@@ -397,8 +397,21 @@ function reportPeerDependencyIssuesError (
   err: Error,
   msg: { issuesByProjects: PeerDependencyIssuesByProjects }
 ) {
+  const hasMissingPeers = getHasMissingPeers(msg.issuesByProjects)
+  const hints: string[] = []
+  if (hasMissingPeers) {
+    hints.push('If you want peer dependencies to be automatically installed, set the "auto-install-peers" setting to "true".')
+  }
+  hints.push('If you don\'t want pnpm to fail on peer dependency issues, set the "strict-peer-dependencies" setting to "false".')
   return {
     title: err.message,
-    body: renderPeerIssues(msg.issuesByProjects),
+    body: `${renderPeerIssues(msg.issuesByProjects)}
+${hints.map((hint) => `hint: ${hint}`).join('\n')}
+`,
   }
 }
+
+function getHasMissingPeers (issuesByProjects: PeerDependencyIssuesByProjects) {
+  return Object.values(issuesByProjects)
+    .some((issues) => Object.values(issues.missing).flat().some(({ optional }) => !optional))
+}
diff --git a/packages/headless/package.json b/packages/headless/package.json
@@ -22,7 +22,7 @@
     "@pnpm/package-store": "workspace:12.1.15",
     "@pnpm/prepare": "workspace:*",
     "@pnpm/read-projects-context": "workspace:5.0.19",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/store-path": "^5.0.0",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/fs-extra": "^9.0.5",

diff --git a/packages/package-requester/package.json b/packages/package-requester/package.json
@@ -65,7 +65,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/package-requester": "workspace:17.0.3",
     "@pnpm/package-store": "workspace:12.1.15",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/normalize-path": "^3.0.0",
     "@types/ramda": "0.27.39",

diff --git a/packages/plugin-commands-installation/package.json b/packages/plugin-commands-installation/package.json
@@ -39,7 +39,7 @@
     "@pnpm/modules-yaml": "workspace:9.1.1",
     "@pnpm/plugin-commands-installation": "workspace:8.4.15",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/is-ci": "^3.0.0",
     "@types/proxyquire": "^1.3.28",

diff --git a/packages/plugin-commands-installation/src/installDeps.ts b/packages/plugin-commands-installation/src/installDeps.ts
@@ -20,7 +20,6 @@ import {
 import logger from '@pnpm/logger'
 import { sequenceGraph } from '@pnpm/sort-packages'
 import isSubdir from 'is-subdir'
-import isEmpty from 'ramda/src/isEmpty'
 import getOptionsFromRootManifest from './getOptionsFromRootManifest'
 import getPinnedVersion from './getPinnedVersion'
 import getSaveType from './getSaveType'
@@ -233,26 +232,8 @@ when running add/update with the --workspace option')
       rootDir: opts.dir,
       targetDependenciesField: getSaveType(opts),
     }
-    let [updatedImporter] = await mutateModules([mutatedProject], installOpts)
+    const [updatedImporter] = await mutateModules([mutatedProject], installOpts)
     if (opts.save !== false) {
-      if (opts.autoInstallPeers && !isEmpty(updatedImporter.peerDependencyIssues?.intersections ?? {})) {
-        logger.info({
-          message: 'Installing missing peer dependencies',
-          prefix: opts.dir,
-        })
-        const dependencySelectors = Object.entries(updatedImporter.peerDependencyIssues!.intersections)
-          .map(([name, version]: [string, string]) => `${name}@${version}`)
-        const result = await mutateModules([
-          {
-            ...mutatedProject,
-            dependencySelectors,
-            manifest: updatedImporter.manifest,
-            peer: false,
-            targetDependenciesField: 'devDependencies',
-          },
-        ], installOpts)
-        updatedImporter = result[0]
-      }
       await writeProjectManifest(updatedImporter.manifest)
     }
     return

diff --git a/packages/plugin-commands-installation/test/add.ts b/packages/plugin-commands-installation/test/add.ts
@@ -290,18 +290,14 @@ test('pnpm add - should add prefix when set in .npmrc when a range is not specif
 })
 
 test('pnpm add automatically installs missing peer dependencies', async () => {
-  prepare()
+  const project = prepare()
   await add.handler({
     ...DEFAULT_OPTIONS,
     autoInstallPeers: true,
     dir: process.cwd(),
     linkWorkspacePackages: false,
   }, ['abc@1.0.0'])
 
-  const manifest = (await import(path.resolve('package.json')))
-
-  expect(manifest.dependencies['abc']).toBe('1.0.0')
-  expect(manifest.devDependencies['peer-a']).toBe('^1.0.0')
-  expect(manifest.devDependencies['peer-b']).toBe('^1.0.0')
-  expect(manifest.devDependencies['peer-c']).toBe('^1.0.0')
+  const lockfile = await project.readLockfile()
+  expect(Object.keys(lockfile.packages).length).toBe(5)
 })
diff --git a/packages/plugin-commands-listing/package.json b/packages/plugin-commands-listing/package.json
@@ -38,7 +38,7 @@
     "@pnpm/plugin-commands-installation": "workspace:8.4.15",
     "@pnpm/plugin-commands-listing": "workspace:4.1.15",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@types/ramda": "0.27.39",
     "execa": "npm:safe-execa@^0.1.1",
     "strip-ansi": "^6.0.0",

diff --git a/packages/plugin-commands-outdated/package.json b/packages/plugin-commands-outdated/package.json
@@ -38,7 +38,7 @@
     "@pnpm/plugin-commands-installation": "workspace:8.4.15",
     "@pnpm/plugin-commands-outdated": "workspace:5.1.14",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@types/lru-cache": "^5.1.0",
     "@types/ramda": "0.27.39",
     "@types/wrap-ansi": "^3.0.0",

diff --git a/packages/plugin-commands-publishing/package.json b/packages/plugin-commands-publishing/package.json
@@ -39,7 +39,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/plugin-commands-publishing": "workspace:4.5.8",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@types/cross-spawn": "^6.0.2",
     "@types/is-ci": "^3.0.0",
     "@types/is-windows": "^1.0.0",

diff --git a/packages/plugin-commands-rebuild/package.json b/packages/plugin-commands-rebuild/package.json
@@ -37,7 +37,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/plugin-commands-rebuild": "workspace:5.4.21",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/test-fixtures": "workspace:*",
     "@types/ramda": "0.27.39",
     "@types/semver": "^7.3.4",

diff --git a/packages/plugin-commands-script-runners/package.json b/packages/plugin-commands-script-runners/package.json
@@ -38,7 +38,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/plugin-commands-script-runners": "workspace:4.6.9",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@types/is-windows": "^1.0.0",
     "@types/ramda": "0.27.39",
     "is-windows": "^1.0.2",

diff --git a/packages/plugin-commands-store/package.json b/packages/plugin-commands-store/package.json
@@ -38,7 +38,7 @@
     "@pnpm/logger": "^4.0.0",
     "@pnpm/plugin-commands-store": "workspace:4.1.19",
     "@pnpm/prepare": "workspace:*",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@types/archy": "0.0.31",
     "@types/ramda": "0.27.39",
     "@types/ssri": "^7.1.0",

diff --git a/packages/pnpm/package.json b/packages/pnpm/package.json
@@ -54,7 +54,7 @@
     "@pnpm/prepare": "workspace:*",
     "@pnpm/read-package-json": "workspace:5.0.12",
     "@pnpm/read-project-manifest": "workspace:2.0.13",
-    "@pnpm/registry-mock": "2.16.0",
+    "@pnpm/registry-mock": "2.17.0",
     "@pnpm/run-npm": "workspace:3.1.1",
     "@pnpm/store-path": "^5.0.0",
     "@pnpm/tabtab": "^0.1.2",

diff --git a/packages/prune-lockfile/src/index.ts b/packages/prune-lockfile/src/index.ts
@@ -9,7 +9,6 @@ import { PackageManifest } from '@pnpm/types'
 import { refToRelative } from 'dependency-path'
 import difference from 'ramda/src/difference'
 import isEmpty from 'ramda/src/isEmpty'
-import omit from 'ramda/src/omit'
 import unnest from 'ramda/src/unnest'
 
 export * from '@pnpm/lockfile-types'
@@ -197,7 +196,7 @@ function copyDependencySubGraph (
     } else if (depLockfile.dev === undefined && !ctx.notProdOnly.has(depPath)) {
       depLockfile.dev = false
     }
-    const newDependencies = resolvedDepsToDepPaths(omit(Object.keys(depLockfile.peerDependencies ?? {}) ?? [], depLockfile.dependencies ?? {}))
+    const newDependencies = resolvedDepsToDepPaths(depLockfile.dependencies ?? {})
     copyDependencySubGraph(ctx, newDependencies, opts)
     const newOptionalDependencies = resolvedDepsToDepPaths(depLockfile.optionalDependencies ?? {})
     copyDependencySubGraph(ctx, newOptionalDependencies, { dev: opts.dev, optional: true })

diff --git a/packages/resolve-dependencies/src/getNonDevWantedDependencies.ts b/packages/resolve-dependencies/src/getNonDevWantedDependencies.ts
@@ -8,7 +8,7 @@ export interface WantedDependency {
   injected?: boolean
 }
 
-export default function getNonDevWantedDependencies (pkg: DependencyManifest) {
+export default function getNonDevWantedDependencies (pkg: Pick<DependencyManifest, 'bundleDependencies' | 'optionalDependencies' | 'dependencies' | 'dependenciesMeta'>) {
   const bd = pkg.bundleDependencies ?? pkg.bundleDependencies
   const bundledDeps = new Set(Array.isArray(bd) ? bd : [])
   const filterDeps = getNotBundledDeps.bind(null, bundledDeps)

diff --git a/packages/resolve-dependencies/src/index.ts b/packages/resolve-dependencies/src/index.ts
@@ -132,7 +132,7 @@ export default async function (
         difference(
           Object.keys(getAllDependenciesFromManifest(project.manifest)),
           resolvedImporter.directDependencies
-            .filter((dep, index) => project.wantedDependencies[index].isNew === true)
+            .filter((dep, index) => project.wantedDependencies[index]?.isNew === true)
             .map(({ alias }) => alias) || []
         ),
         project.modulesDir

diff --git a/packages/resolve-dependencies/src/mergePeers.ts b/packages/resolve-dependencies/src/mergePeers.ts
@@ -20,7 +20,7 @@ export function mergePeers (missingPeers: MissingPeerIssuesByPeerName) {
   return { conflicts, intersections }
 }
 
-function safeIntersect (ranges: string[]): null | string {
+export function safeIntersect (ranges: string[]): null | string {
   try {
     return intersect(...ranges)
   } catch {