feat: better reporting of unmet peer issues (#4169)

2025-12-23 23:29:17 -05:00 · 2021-12-29 01:19:14 +02:00
parent b390c75a6f
commit b5734a4a7c
11 changed files with 204 additions and 66 deletions
--- a/.changeset/brave-rules-knock.md
+++ b/.changeset/brave-rules-knock.md
@@ -0,0 +1,5 @@
+---
+"@pnpm/resolve-dependencies": minor
+---
+
+BadPeerDependencyIssue should contain the path to the package that has the dependency from which the peer dependency is resolved.
--- a/.changeset/serious-rings-develop.md
+++ b/.changeset/serious-rings-develop.md
@@ -0,0 +1,8 @@
+---
+"@pnpm/render-peer-issues": minor
+"pnpm": patch
+---
+
+When reporting unmet peer dependency issues, if the peer dependency is resolved not from a dependency installed by the user, then print the name of the parent package that has the bad peer dependency installed as a dependency.
+
+![](https://i.imgur.com/0kjij22.png)
--- a/.changeset/stupid-fans-sparkle.md
+++ b/.changeset/stupid-fans-sparkle.md
@@ -0,0 +1,5 @@
+---
+"@pnpm/types": minor
+---
+
+Add `resolvedFrom` field to `BadPeerDependencyIssues`.
--- a/packages/core/test/install/peerDependencies.ts
+++ b/packages/core/test/install/peerDependencies.ts
@@ -384,6 +384,7 @@ test('warning is reported when bad version of resolved peer dependency for non-t
                  },
                ],
                foundVersion: '2.0.0',
+                resolvedFrom: [],
                optional: false,
                wantedRange: '^1.0.0',
              },
@@ -428,6 +429,7 @@ test('strict-peer-dependencies: error is thrown when bad version of resolved pee
            },
          ],
          foundVersion: '2.0.0',
+          resolvedFrom: [],
          optional: false,
          wantedRange: '^1.0.0',
        },
@@ -1017,6 +1019,7 @@ test('warning is not reported when cannot resolve optional peer dependency', asy
                },
              ],
              foundVersion: '2.0.0',
+              resolvedFrom: [],
              optional: true,
              wantedRange: '^1.0.0',
            }],
--- a/packages/default-reporter/test/reportingPeerDependencyIssues.ts
+++ b/packages/default-reporter/test/reportingPeerDependencyIssues.ts
@@ -27,6 +27,7 @@ test('print peer dependency issues warning', (done) => {
                },
              ],
              foundVersion: '2',
+              resolvedFrom: [],
              optional: false,
              wantedRange: '3',
            },
@@ -63,6 +64,7 @@ test('print peer dependency issues error', (done) => {
      bad: {
        a: [
          {
+            foundVersion: '2',
            parents: [
              {
                name: 'b',
@@ -70,6 +72,7 @@ test('print peer dependency issues error', (done) => {
              },
            ],
            optional: false,
+            resolvedFrom: [],
            wantedRange: '3',
          },
        ],
--- a/packages/render-peer-issues/src/index.ts
+++ b/packages/render-peer-issues/src/index.ts
@@ -1,4 +1,4 @@
-import { PeerDependencyIssuesByProjects } from '@pnpm/types'
+import { BadPeerDependencyIssue, PeerDependencyIssuesByProjects } from '@pnpm/types'
 import archy from 'archy'
 import chalk from 'chalk'
 import cliColumns from 'cli-columns'
@@ -22,9 +22,11 @@ export default function (
      }
    }
    for (const [peerName, issues] of Object.entries(bad)) {
-      for (const { parents, foundVersion, wantedRange } of issues) {
-        // eslint-disable-next-line
-        createTree(projects[projectId], parents, `${chalk.red('✕ unmet peer')} ${formatNameAndRange(peerName, wantedRange)}: found ${foundVersion}`)
+      for (const issue of issues) {
+        createTree(projects[projectId], issue.parents, formatUnmetPeerMessage({
+          peerName,
+          ...issue,
+        }))
      }
    }
  }
@@ -49,8 +51,24 @@ export default function (
        )
      }
      const title = chalk.white(projectKey)
-      return `${archy(toArchyData(title, project))}${summaries.join('\n')}`
-    }).join('\n\n')
+      let summariesConcatenated = summaries.join('\n')
+      if (summariesConcatenated) {
+        summariesConcatenated += '\n'
+      }
+      return `${archy(toArchyData(title, project))}${summariesConcatenated}`
+    }).join('\n')
+}
+
+function formatUnmetPeerMessage (
+  { foundVersion, peerName, wantedRange, resolvedFrom }: BadPeerDependencyIssue & {
+    peerName: string
+  }
+) {
+  const nameAndRange = formatNameAndRange(peerName, wantedRange)
+  if (resolvedFrom && resolvedFrom.length > 0) {
+    return `✕ unmet peer ${nameAndRange}: found ${foundVersion} in ${resolvedFrom[resolvedFrom.length - 1].name}`
+  }
+  return `${chalk.yellowBright('✕ unmet peer')} ${nameAndRange}: found ${foundVersion}`
 }

 function formatNameAndRange (name: string, range: string) {
--- a/packages/render-peer-issues/test/snapshots/index.ts.snap
+++ b/packages/render-peer-issues/test/snapshots/index.ts.snap
@@ -6,7 +6,7 @@ exports[`renderPeerIssues() 1`] = `
  ├── ✕ unmet peer bbb@^1.0.0: found 2
  └─┬ yyy
    ├── ✕ missing peer aaa@\\">=1.0.0 <3.0.0\\"
-    └── ✕ unmet peer ccc@^1.0.0: found 2
+    └── ✕ unmet peer ccc@^1.0.0: found 2 in xxx
 Peer dependencies that should be installed:
  aaa@^1.0.0  

@@ -19,7 +19,8 @@ packages/0
 ✕ Conflicting peer dependencies:
  eee  
 Peer dependencies that should be installed:
-  ddd@^1.0.0  "
+  ddd@^1.0.0  
+"
 `;

 exports[`renderPeerIssues() format correctly the version ranges with spaces and "*" 1`] = `
@@ -28,7 +29,8 @@ exports[`renderPeerIssues() format correctly the version ranges with spaces and
  ├── ✕ missing peer a@\\"*\\"
  └── ✕ missing peer b@\\"1 || 2\\"
 Peer dependencies that should be installed:
-  a@\\"*\\"       b@\\"1 || 2\\"  "
+  a@\\"*\\"       b@\\"1 || 2\\"  
+"
 `;

 exports[`renderPeerIssues() optional peer dependencies are printed only if they are in conflict with non-optional peers 1`] = `
@@ -38,5 +40,6 @@ exports[`renderPeerIssues() optional peer dependencies are printed only if they
    ├── ✕ missing peer aaa@^1.0.0
    └── ✕ missing peer aaa@^2.0.0
 ✕ Conflicting peer dependencies:
-  aaa  "
+  aaa  
+"
 `;
--- a/packages/render-peer-issues/test/index.ts
+++ b/packages/render-peer-issues/test/index.ts
@@ -73,6 +73,7 @@ test('renderPeerIssues()', () => {
              },
            ],
            foundVersion: '2',
+            resolvedFrom: [],
            optional: false,
            wantedRange: '^1.0.0',
          },
@@ -90,6 +91,12 @@ test('renderPeerIssues()', () => {
              },
            ],
            foundVersion: '2',
+            resolvedFrom: [
+              {
+                name: 'xxx',
+                version: '1.0.0',
+              },
+            ],
            optional: false,
            wantedRange: '^1.0.0',
          },
--- a/packages/resolve-dependencies/src/resolvePeers.ts
+++ b/packages/resolve-dependencies/src/resolvePeers.ts
@@ -399,7 +399,7 @@ function resolvePeers<T extends PartialResolvedPackage> (

    if (!resolved) {
      missingPeers.push(peerName)
-      const location = getLocationFromNodeId({
+      const location = getLocationFromNodeIdAndPkg({
        dependenciesTree: ctx.dependenciesTree,
        nodeId: ctx.nodeId,
        pkg: ctx.resolvedPackage,
@@ -416,7 +416,7 @@ function resolvePeers<T extends PartialResolvedPackage> (
    }

    if (!satisfiesWithPrereleases(resolved.version, peerVersionRange, true)) {
-      const location = getLocationFromNodeId({
+      const location = getLocationFromNodeIdAndPkg({
        dependenciesTree: ctx.dependenciesTree,
        nodeId: ctx.nodeId,
        pkg: ctx.resolvedPackage,
@@ -424,8 +424,15 @@ function resolvePeers<T extends PartialResolvedPackage> (
      if (!ctx.peerDependencyIssues.bad[peerName]) {
        ctx.peerDependencyIssues.bad[peerName] = []
      }
+      const peerLocation = resolved.nodeId == null
+        ? []
+        : getLocationFromNodeId({
+          dependenciesTree: ctx.dependenciesTree,
+          nodeId: resolved.nodeId,
+        }).parents
      ctx.peerDependencyIssues.bad[peerName].push({
        foundVersion: resolved.version,
+        resolvedFrom: peerLocation,
        parents: location.parents,
        optional: optionalPeer,
        wantedRange: peerVersionRange,
@@ -437,7 +444,7 @@ function resolvePeers<T extends PartialResolvedPackage> (
  return { resolvedPeers, missingPeers }
 }

-function getLocationFromNodeId<T> (
+function getLocationFromNodeIdAndPkg<T> (
  {
    dependenciesTree,
    nodeId,
@@ -445,14 +452,30 @@ function getLocationFromNodeId<T> (
  }: {
    dependenciesTree: DependenciesTree<T>
    nodeId: string
-    pkg: PartialResolvedPackage
+    pkg: { name: string, version: string }
+  }
+) {
+  const { projectId, parents } = getLocationFromNodeId({ dependenciesTree, nodeId })
+  parents.push({ name: pkg.name, version: pkg.version })
+  return {
+    projectId,
+    parents,
+  }
+}
+
+function getLocationFromNodeId<T> (
+  {
+    dependenciesTree,
+    nodeId,
+  }: {
+    dependenciesTree: DependenciesTree<T>
+    nodeId: string
  }
 ) {
  const parts = splitNodeId(nodeId).slice(0, -1)
  const parents = scan((prevNodeId, pkgId) => createNodeId(prevNodeId, pkgId), '>', parts)
    .slice(2)
    .map((nid) => pick(['name', 'version'], dependenciesTree[nid].resolvedPackage as ResolvedPackage))
-  parents.push({ name: pkg.name, version: pkg.version })
  return {
    projectId: parts[0],
    parents,
--- a/packages/resolve-dependencies/test/resolvePeers.ts
+++ b/packages/resolve-dependencies/test/resolvePeers.ts
@@ -24,7 +24,7 @@ test('resolve peer dependencies of cyclic dependencies', () => {
    projects: [
      {
        directNodeIdsByAlias: {
-          foo: 'foo/1.0.0',
+          foo: '>foo/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -32,25 +32,25 @@ test('resolve peer dependencies of cyclic dependencies', () => {
      },
    ],
    dependenciesTree: {
-      'foo/1.0.0': {
+      '>foo/1.0.0>': {
        children: {
-          bar: 'foo/1.0.0>bar/1.0.0',
+          bar: '>foo/1.0.0>bar/1.0.0>',
        },
        installable: true,
        resolvedPackage: fooPkg,
        depth: 0,
      },
-      'foo/1.0.0>bar/1.0.0': {
+      '>foo/1.0.0>bar/1.0.0>': {
        children: {
-          qar: 'foo/1.0.0>bar/1.0.0>qar/1.0.0',
+          qar: '>foo/1.0.0>bar/1.0.0>qar/1.0.0>',
        },
        installable: true,
        resolvedPackage: barPkg,
        depth: 1,
      },
-      'foo/1.0.0>bar/1.0.0>qar/1.0.0': {
+      '>foo/1.0.0>bar/1.0.0>qar/1.0.0>': {
        children: {
-          zoo: 'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0',
+          zoo: '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>',
        },
        installable: true,
        resolvedPackage: {
@@ -64,10 +64,10 @@ test('resolve peer dependencies of cyclic dependencies', () => {
        },
        depth: 2,
      },
-      'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0': {
+      '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>': {
        children: {
-          foo: 'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>foo/1.0.0',
-          bar: 'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>bar/1.0.0',
+          foo: '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>foo/1.0.0>',
+          bar: '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>bar/1.0.0>',
        },
        installable: true,
        resolvedPackage: {
@@ -80,13 +80,13 @@ test('resolve peer dependencies of cyclic dependencies', () => {
        },
        depth: 3,
      },
-      'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>foo/1.0.0': {
+      '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooPkg,
        depth: 4,
      },
-      'foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>bar/1.0.0': {
+      '>foo/1.0.0>bar/1.0.0>qar/1.0.0>zoo/1.0.0>bar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barPkg,
@@ -131,8 +131,8 @@ test('when a package is referenced twice in the dependencies graph and one of th
    projects: [
      {
        directNodeIdsByAlias: {
-          zoo: 'zoo/1.0.0',
-          bar: 'bar/1.0.0',
+          zoo: '>zoo/1.0.0>',
+          bar: '>bar/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -140,44 +140,44 @@ test('when a package is referenced twice in the dependencies graph and one of th
      },
    ],
    dependenciesTree: {
-      'zoo/1.0.0': {
+      '>zoo/1.0.0>': {
        children: {
-          foo: 'zoo/1.0.0>foo/1.0.0',
+          foo: '>zoo/1.0.0>foo/1.0.0>',
        },
        installable: true,
        resolvedPackage: zooPkg,
        depth: 0,
      },
-      'zoo/1.0.0>foo/1.0.0': {
+      '>zoo/1.0.0>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooPkg,
        depth: 1,
      },
-      'bar/1.0.0': {
+      '>bar/1.0.0>': {
        children: {
-          zoo: 'bar/1.0.0>zoo/1.0.0',
-          qar: 'bar/1.0.0>qar/1.0.0',
+          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': {
+      '>bar/1.0.0>zoo/1.0.0>': {
        children: {
-          foo: 'bar/1.0.0>zoo/1.0.0>foo/1.0.0',
+          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': {
+      '>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': {
+      '>bar/1.0.0>qar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: {
@@ -257,7 +257,7 @@ describe('peer dependency issues', () => {
    projects: [
      {
        directNodeIdsByAlias: {
-          foo: '>project1>foo/1.0.0',
+          foo: '>project1>foo/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -265,7 +265,7 @@ describe('peer dependency issues', () => {
      },
      {
        directNodeIdsByAlias: {
-          bar: '>project2>bar/1.0.0',
+          bar: '>project2>bar/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -273,8 +273,8 @@ describe('peer dependency issues', () => {
      },
      {
        directNodeIdsByAlias: {
-          foo: '>project3>foo/1.0.0',
-          bar: '>project3>bar/1.0.0',
+          foo: '>project3>foo/1.0.0>',
+          bar: '>project3>bar/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -282,8 +282,8 @@ describe('peer dependency issues', () => {
      },
      {
        directNodeIdsByAlias: {
-          bar: '>project4>bar/1.0.0',
-          qar: '>project4>qar/1.0.0',
+          bar: '>project4>bar/1.0.0>',
+          qar: '>project4>qar/1.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -291,8 +291,8 @@ describe('peer dependency issues', () => {
      },
      {
        directNodeIdsByAlias: {
-          foo: '>project5>foo/1.0.0',
-          bar: '>project5>bar/2.0.0',
+          foo: '>project5>foo/1.0.0>',
+          bar: '>project5>bar/2.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -300,8 +300,8 @@ describe('peer dependency issues', () => {
      },
      {
        directNodeIdsByAlias: {
-          foo: '>project6>foo/2.0.0',
-          bar: '>project6>bar/2.0.0',
+          foo: '>project6>foo/2.0.0>',
+          bar: '>project6>bar/2.0.0>',
        },
        topParents: [],
        rootDir: '',
@@ -309,61 +309,61 @@ describe('peer dependency issues', () => {
      },
    ],
    dependenciesTree: {
-      '>project1>foo/1.0.0': {
+      '>project1>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooPkg,
        depth: 0,
      },
-      '>project2>bar/1.0.0': {
+      '>project2>bar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barPkg,
        depth: 0,
      },
-      '>project3>foo/1.0.0': {
+      '>project3>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooPkg,
        depth: 0,
      },
-      '>project3>bar/1.0.0': {
+      '>project3>bar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barPkg,
        depth: 0,
      },
-      '>project4>bar/1.0.0': {
+      '>project4>bar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barPkg,
        depth: 0,
      },
-      '>project4>qar/1.0.0': {
+      '>project4>qar/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: qarPkg,
        depth: 0,
      },
-      '>project5>foo/1.0.0': {
+      '>project5>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooPkg,
        depth: 0,
      },
-      '>project5>bar/2.0.0': {
+      '>project5>bar/2.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barWithOptionalPeer,
        depth: 0,
      },
-      '>project6>foo/2.0.0': {
+      '>project6>foo/2.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: fooWithOptionalPeer,
        depth: 0,
      },
-      '>project6>bar/2.0.0': {
+      '>project6>bar/2.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: barWithOptionalPeer,
@@ -399,9 +399,9 @@ describe('unmet peer dependency issues', () => {
    projects: [
      {
        directNodeIdsByAlias: {
-          foo: '>project1>foo/1.0.0',
-          peer1: '>project1>peer1/1.0.0-rc.0',
-          peer2: '>project1>peer2/1.1.0-rc.0',
+          foo: '>project1>foo/1.0.0>',
+          peer1: '>project1>peer1/1.0.0-rc.0>',
+          peer2: '>project1>peer2/1.1.0-rc.0>',
        },
        topParents: [],
        rootDir: '',
@@ -409,7 +409,7 @@ describe('unmet peer dependency issues', () => {
      },
    ],
    dependenciesTree: {
-      '>project1>foo/1.0.0': {
+      '>project1>foo/1.0.0>': {
        children: {},
        installable: true,
        resolvedPackage: {
@@ -423,7 +423,7 @@ describe('unmet peer dependency issues', () => {
        },
        depth: 0,
      },
-      '>project1>peer1/1.0.0-rc.0': {
+      '>project1>peer1/1.0.0-rc.0>': {
        children: {},
        installable: true,
        resolvedPackage: {
@@ -434,7 +434,7 @@ describe('unmet peer dependency issues', () => {
        },
        depth: 0,
      },
-      '>project1>peer2/1.1.0-rc.0': {
+      '>project1>peer2/1.1.0-rc.0>': {
        children: {},
        installable: true,
        resolvedPackage: {
@@ -456,3 +456,63 @@ describe('unmet peer dependency issues', () => {
    expect(peerDependencyIssuesByProjects).not.toHaveProperty(['project1', 'bad', 'peer2'])
  })
 })
+
+describe('unmet peer dependency issue resolved from subdependency', () => {
+  const { peerDependencyIssuesByProjects } = resolvePeers({
+    projects: [
+      {
+        directNodeIdsByAlias: {
+          foo: '>project>foo/1.0.0>',
+        },
+        topParents: [],
+        rootDir: '',
+        id: 'project',
+      },
+    ],
+    dependenciesTree: {
+      '>project>foo/1.0.0>': {
+        children: {
+          dep: '>project>foo/1.0.0>dep/1.0.0>',
+          bar: '>project>foo/1.0.0>bar/1.0.0>',
+        },
+        installable: true,
+        resolvedPackage: {
+          name: 'foo',
+          depPath: 'foo/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+        },
+        depth: 0,
+      },
+      '>project>foo/1.0.0>dep/1.0.0>': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'dep',
+          depPath: 'dep/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {},
+        },
+        depth: 1,
+      },
+      '>project>foo/1.0.0>bar/1.0.0>': {
+        children: {},
+        installable: true,
+        resolvedPackage: {
+          name: 'bar',
+          depPath: 'bar/1.0.0',
+          version: '1.0.0',
+          peerDependencies: {
+            dep: '10',
+          },
+        },
+        depth: 1,
+      },
+    },
+    virtualStoreDir: '',
+    lockfileDir: '',
+  })
+  it('should return from where the bad peer dependency is resolved', () => {
+    expect(peerDependencyIssuesByProjects.project.bad.dep[0].resolvedFrom).toStrictEqual([{ name: 'foo', version: '1.0.0' }])
+  })
+})
--- a/packages/types/src/peerDependencyIssues.ts
+++ b/packages/types/src/peerDependencyIssues.ts
@@ -1,5 +1,7 @@
+export type ParentPackages = Array<{ name: string, version: string }>
+
 export interface MissingPeerDependencyIssue {
-  parents: Array<{ name: string, version: string }>
+  parents: ParentPackages
  optional: boolean
  wantedRange: string
 }
@@ -8,6 +10,7 @@ export type MissingPeerIssuesByPeerName = Record<string, MissingPeerDependencyIs

 export interface BadPeerDependencyIssue extends MissingPeerDependencyIssue {
  foundVersion: string
+  resolvedFrom: ParentPackages
 }

 export type BadPeerIssuesByPeerName = Record<string, BadPeerDependencyIssue[]>