From 3482fe17d1222b565fef93498ec1fd2410228acc Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 31 Aug 2025 10:40:02 +0200 Subject: [PATCH] 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 | 6 + .../core/test/install/peerDependencies.ts | 214 ++++++++++++------ pkg-manager/resolve-dependencies/package.json | 1 + .../src/dedupeInjectedDeps.ts | 1 + .../resolve-dependencies/src/hoistPeers.ts | 14 +- .../src/resolveDependencies.ts | 36 ++- pnpm-lock.yaml | 3 + 7 files changed, 179 insertions(+), 96 deletions(-) create mode 100644 .changeset/petite-badgers-rest.md diff --git a/.changeset/petite-badgers-rest.md b/.changeset/petite-badgers-rest.md new file mode 100644 index 0000000000..898a34c3ea --- /dev/null +++ b/.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). diff --git a/pkg-manager/core/test/install/peerDependencies.ts b/pkg-manager/core/test/install/peerDependencies.ts index 3c66b2c684..7027e0b08d 100644 --- a/pkg-manager/core/test/install/peerDependencies.ts +++ b/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 + 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 () => { diff --git a/pkg-manager/resolve-dependencies/package.json b/pkg-manager/resolve-dependencies/package.json index 86c1c2c9f8..545880b245 100644 --- a/pkg-manager/resolve-dependencies/package.json +++ b/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:", diff --git a/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts b/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts index cb035c8089..2dea35ed6c 100644 --- a/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts +++ b/pkg-manager/resolve-dependencies/src/dedupeInjectedDeps.ts @@ -82,6 +82,7 @@ function applyDedupeMap ( 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: { diff --git a/pkg-manager/resolve-dependencies/src/hoistPeers.ts b/pkg-manager/resolve-dependencies/src/hoistPeers.ts index cdcf94ac21..7471bdf5a8 100644 --- a/pkg-manager/resolve-dependencies/src/hoistPeers.ts +++ b/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 { const dependencies: Record = {} 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]) { diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 7e68f7147e..830155e603 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -102,17 +102,21 @@ DependenciesTreeNode export type ResolvedPkgsById = Record -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, } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9416d635eb..b3b947ebd7 100644 --- a/pnpm-lock.yaml +++ b/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