fix: pick package by real name to resolve a peer dependency (#9919)

* fix: pick package by real name to resolve a peer dependency

close #9913

This fixes a regression introduced in #9835

* fix: resolve from alias

* test: fix

* refactor: test

* fix: sort aliases

* docs: add changesets

* refactor: types

.changeset/petite-badgers-rest.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/resolve-dependencies": patch
+"pnpm": patch
+---
+
+When resolving peer dependencies, pnpm looks whether the peer dependency is present in the root workspace project's dependencies. This change makes it so that the peer dependency is correctly resolved even from aliased npm-hosted dependencies or other types of dependencies [#9913](https://github.com/pnpm/pnpm/issues/9913).

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

@@ -1,5 +1,6 @@
 import fs from 'fs'
 import path from 'path'
+import { type Project } from '@pnpm/assert-project'
 import { WANTED_LOCKFILE } from '@pnpm/constants'
 import { type LockfileFile } from '@pnpm/lockfile.fs'
 import { prepareEmpty, preparePackages } from '@pnpm/prepare'
@@ -331,83 +332,152 @@ test('peer dependency is resolved from the dependencies of the workspace root pr
   }
 })
 
-test('peer dependency is resolved from the dependencies of the workspace root project even if there are other versions of the peer dependency present in the dependency graph', async () => {
-  const projects = preparePackages([
-    {
-      location: '.',
-      package: { name: 'root' },
-    },
-    {
-      location: 'pkg',
-      package: {},
-    },
-    {
-      location: 'pkg2',
-      package: {},
-    },
-  ])
-  const allProjects: ProjectOptions[] = [
-    {
-      buildIndex: 0,
-      manifest: {
-        name: 'root',
-        version: '1.0.0',
-
-        dependencies: {
-          ajv: '4.10.0',
-        },
+describe('peer dependency is resolved from the root of the workspace even if there are other versions of the peer dependency present in the dependency graph', () => {
+  let nonRootProjects: ProjectOptions[]
+  let projects: Record<string, Project>
+  let mutatedProjects: MutatedProject[]
+  beforeEach(() => {
+    projects = preparePackages([
+      {
+        location: '.',
+        package: { name: 'root' },
       },
-      rootDir: process.cwd() as ProjectRootDir,
-    },
-    {
-      buildIndex: 0,
-      manifest: {
-        name: 'pkg',
-        version: '1.0.0',
-
-        dependencies: {
-          'ajv-keywords': '1.5.0',
-        },
+      {
+        location: 'pkg',
+        package: {},
       },
-      rootDir: path.resolve('pkg') as ProjectRootDir,
-    },
-    {
-      buildIndex: 0,
-      manifest: {
-        name: 'pkg2',
-        version: '1.0.0',
-
-        dependencies: {
-          ajv: '5.0.0',
-        },
+      {
+        location: 'pkg2',
+        package: {},
       },
-      rootDir: path.resolve('pkg2') as ProjectRootDir,
-    },
-  ]
-  const reporter = jest.fn()
-  await mutateModules([
-    {
-      mutation: 'install',
-      rootDir: process.cwd() as ProjectRootDir,
-    },
-    {
-      mutation: 'install',
-      rootDir: path.resolve('pkg') as ProjectRootDir,
-    },
-    {
-      mutation: 'install',
-      rootDir: path.resolve('pkg2') as ProjectRootDir,
-    },
-  ], testDefaults({ allProjects, reporter, resolvePeersFromWorkspaceRoot: true }))
+    ])
+    nonRootProjects = [
+      {
+        buildIndex: 0,
+        manifest: {
+          name: 'pkg',
+          version: '1.0.0',
 
-  expect(reporter).not.toHaveBeenCalledWith(expect.objectContaining({
-    name: 'pnpm:peer-dependency-issues',
-  }))
+          dependencies: {
+            'ajv-keywords': '1.5.0',
+          },
+        },
+        rootDir: path.resolve('pkg') as ProjectRootDir,
+      },
+      {
+        buildIndex: 0,
+        manifest: {
+          name: 'pkg2',
+          version: '1.0.0',
 
-  {
-    const lockfile = projects.root.readLockfile()
-    expect(lockfile.importers.pkg?.dependencies?.['ajv-keywords'].version).toBe('1.5.0(ajv@4.10.0)')
-  }
+          dependencies: {
+            ajv: '5.0.0',
+          },
+        },
+        rootDir: path.resolve('pkg2') as ProjectRootDir,
+      },
+    ]
+    mutatedProjects = [
+      {
+        mutation: 'install',
+        rootDir: process.cwd() as ProjectRootDir,
+      },
+      {
+        mutation: 'install',
+        rootDir: path.resolve('pkg') as ProjectRootDir,
+      },
+      {
+        mutation: 'install',
+        rootDir: path.resolve('pkg2') as ProjectRootDir,
+      },
+    ]
+  })
+  test('the package in the root is a regular non-aliased npm-hosted dependency', async () => {
+    const allProjects: ProjectOptions[] = [
+      {
+        buildIndex: 0,
+        manifest: {
+          name: 'root',
+          version: '1.0.0',
+
+          dependencies: {
+            ajv: '4.10.0',
+          },
+        },
+        rootDir: process.cwd() as ProjectRootDir,
+      },
+      ...nonRootProjects,
+    ]
+    const reporter = jest.fn()
+    await mutateModules(mutatedProjects, testDefaults({ allProjects, reporter, resolvePeersFromWorkspaceRoot: true }))
+
+    expect(reporter).not.toHaveBeenCalledWith(expect.objectContaining({
+      name: 'pnpm:peer-dependency-issues',
+    }))
+
+    {
+      const lockfile = projects.root.readLockfile()
+      expect(lockfile.importers.pkg?.dependencies?.['ajv-keywords'].version).toBe('1.5.0(ajv@4.10.0)')
+    }
+  })
+  test('the package in the root is aliasing a package with a different name', async () => {
+    const allProjects: ProjectOptions[] = [
+      {
+        buildIndex: 0,
+        manifest: {
+          name: 'root',
+          version: '1.0.0',
+
+          dependencies: {
+            ajv: 'npm:@pnpm.e2e/foo@100.0.0',
+          },
+        },
+        rootDir: process.cwd() as ProjectRootDir,
+      },
+      ...nonRootProjects,
+    ]
+    const reporter = jest.fn()
+    await mutateModules(mutatedProjects, testDefaults({ allProjects, reporter, resolvePeersFromWorkspaceRoot: true }))
+
+    expect(reporter).not.toHaveBeenCalledWith(expect.objectContaining({
+      name: 'pnpm:peer-dependency-issues',
+    }))
+
+    {
+      const lockfile = projects.root.readLockfile()
+      expect(lockfile.importers.pkg?.dependencies?.['ajv-keywords'].version).toBe('1.5.0(@pnpm.e2e/foo@100.0.0)')
+    }
+  })
+  test('the package in the root is under an alias', async () => {
+    const allProjects: ProjectOptions[] = [
+      {
+        buildIndex: 0,
+        manifest: {
+          name: 'root',
+          version: '1.0.0',
+
+          dependencies: {
+            b: 'npm:ajv@1.0.0',
+            a: 'npm:ajv@4.10.0',
+            c: 'npm:ajv@2.0.0',
+          },
+        },
+        rootDir: process.cwd() as ProjectRootDir,
+      },
+      ...nonRootProjects,
+    ]
+    const reporter = jest.fn()
+    await mutateModules(mutatedProjects, testDefaults({ allProjects, reporter, resolvePeersFromWorkspaceRoot: true }))
+
+    expect(reporter).not.toHaveBeenCalledWith(expect.objectContaining({
+      name: 'pnpm:peer-dependency-issues',
+    }))
+
+    {
+      const lockfile = projects.root.readLockfile()
+      expect(lockfile.importers.pkg?.dependencies?.['ajv-keywords'].version).toBe('1.5.0(ajv@4.10.0)')
+    }
+  })
 })
 
 test('warning is reported when cannot resolve peer dependency for non-top-level dependency', async () => {

pkg-manager/resolve-dependencies/package.json

@@ -53,6 +53,7 @@
     "@pnpm/semver.peer-range": "workspace:*",
     "@pnpm/store-controller-types": "workspace:*",
     "@pnpm/types": "workspace:*",
+    "@pnpm/util.lex-comparator": "catalog:",
     "@pnpm/workspace.spec-parser": "workspace:*",
     "@yarnpkg/core": "catalog:",
     "filenamify": "catalog:",

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

@@ -82,6 +82,7 @@ function applyDedupeMap<T extends PartialResolvedPackage> (
       const prev = opts.resolvedImporters[id].directDependencies[index]
       const linkedDep: LinkedDependency & ResolvedDirectDependency = {
         ...prev,
+        pkg: prev,
         isLinkedDependency: true,
         pkgId: `link:${normalize(path.relative(id, dedupedProjectId))}` as PkgResolutionId,
         resolution: {

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

@@ -1,4 +1,5 @@
 import { type PreferredVersions } from '@pnpm/resolver-base'
+import { lexCompare } from '@pnpm/util.lex-comparator'
 import semver from 'semver'
 import { type PkgAddressOrLink } from './resolveDependencies.js'
 
@@ -12,9 +13,16 @@ export function hoistPeers (
 ): Record<string, string> {
   const dependencies: Record<string, string> = {}
   for (const [peerName, { range }] of missingRequiredPeers) {
-    const rootDep = opts.workspaceRootDeps.find((rootDep) => rootDep.alias === peerName)
-    if (rootDep?.version) {
-      dependencies[peerName] = rootDep.version
+    const rootDepByAlias = opts.workspaceRootDeps.find((rootDep) => rootDep.alias === peerName)
+    if (rootDepByAlias?.normalizedBareSpecifier) {
+      dependencies[peerName] = rootDepByAlias.normalizedBareSpecifier
+      continue
+    }
+    const rootDep = opts.workspaceRootDeps
+      .filter((rootDep) => rootDep.pkg.name === peerName)
+      .sort((rootDep1, rootDep2) => lexCompare(rootDep1.alias, rootDep2.alias))[0]
+    if (rootDep?.normalizedBareSpecifier) {
+      dependencies[peerName] = rootDep.normalizedBareSpecifier
       continue
     }
     if (opts.allPreferredVersions![peerName]) {

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

@@ -102,17 +102,21 @@ DependenciesTreeNode<T>
 
 export type ResolvedPkgsById = Record<PkgResolutionId, ResolvedPackage>
 
-export interface LinkedDependency {
-  isLinkedDependency: true
-  optional: boolean
-  dev: boolean
-  resolution: DirectoryResolution
-  pkgId: PkgResolutionId
-  version: string
-  name: string
+export interface PkgAddressOrLinkBase {
   alias: string
   catalogLookup?: CatalogLookupMetadata
   normalizedBareSpecifier?: string
+  optional: boolean
+  pkg: PackageManifest
+  pkgId: PkgResolutionId
+}
+
+export interface LinkedDependency extends PkgAddressOrLinkBase {
+  isLinkedDependency: true
+  dev: boolean
+  resolution: DirectoryResolution
+  version: string
+  name: string
 }
 
 export interface PendingNode {
@@ -190,32 +194,21 @@ interface MissingPeersOfChildren {
   resolved?: boolean
 }
 
-export type PkgAddress = {
-  alias: string
+export interface PkgAddress extends PkgAddressOrLinkBase {
   depIsLinked: boolean
   isNew: boolean
   isLinkedDependency?: false
   resolvedVia?: string
   nodeId: NodeId
-  pkgId: PkgResolutionId
   installable: boolean
-  pkg: PackageManifest
   version?: string
   updated: boolean
   rootDir: string
   missingPeers: MissingPeers
   missingPeersOfChildren?: MissingPeersOfChildren
   publishedAt?: string
-  catalogLookup?: CatalogLookupMetadata
-  optional: boolean
-  normalizedBareSpecifier?: string
   saveCatalogName?: string
-} & ({
-  isLinkedDependency: true
-  version: string
-} | {
-  isLinkedDependency: undefined
-})
+}
 
 export type PkgAddressOrLink = PkgAddress | LinkedDependency
 
@@ -1406,6 +1399,7 @@ async function resolveDependency (
       resolution: pkgResponse.body.resolution,
       version: pkgResponse.body.manifest.version,
       normalizedBareSpecifier: pkgResponse.body.normalizedBareSpecifier,
+      pkg: pkgResponse.body.manifest,
     }
   }
 

pnpm-lock.yaml

@@ -6068,6 +6068,9 @@ importers:
       '@pnpm/types':
         specifier: workspace:*
         version: link:../../packages/types
+      '@pnpm/util.lex-comparator':
+        specifier: 'catalog:'
+        version: 3.0.2
       '@pnpm/workspace.spec-parser':
         specifier: workspace:*
         version: link:../../workspace/spec-parser