diff --git a/packages/core/test/install/autoInstallPeers.ts b/packages/core/test/install/autoInstallPeers.ts index b740bcecd75..d7df4f7fd75 100644 --- a/packages/core/test/install/autoInstallPeers.ts +++ b/packages/core/test/install/autoInstallPeers.ts @@ -266,3 +266,61 @@ test('automatically install peer dependency when it is a dev dependency in anoth 'is-positive': '1.0.0', }) }) + +// Covers https://github.com/pnpm/pnpm/issues/4820 +test('auto install peer deps in a workspace', async () => { + prepareEmpty() + await mutateModules([ + { + buildIndex: 0, + manifest: { + name: 'root-project', + devDependencies: { + '@pnpm.e2e/abc-parent-with-ab': '1.0.0', + }, + }, + mutation: 'install', + rootDir: process.cwd(), + }, + { + buildIndex: 0, + manifest: { + name: 'project', + peerDependencies: { + '@pnpm.e2e/abc-parent-with-ab': '1.0.0', + }, + }, + mutation: 'install', + rootDir: path.resolve('project'), + }, + ], await testDefaults({ autoInstallPeers: true })) +}) + +test('auto install peer deps in a workspace', async () => { + prepareEmpty() + await mutateModules([ + { + buildIndex: 0, + manifest: { + name: 'root-project', + devDependencies: { + '@pnpm.e2e/abc-parent-with-ab': '1.0.0', + '@pnpm.e2e/peer-c': '1.0.0', + }, + }, + mutation: 'install', + rootDir: process.cwd(), + }, + { + buildIndex: 0, + manifest: { + name: 'project', + peerDependencies: { + '@pnpm.e2e/abc-parent-with-ab': '1.0.0', + }, + }, + mutation: 'install', + rootDir: path.resolve('project'), + }, + ], await testDefaults({ autoInstallPeers: true })) +}) diff --git a/packages/resolve-dependencies/package.json b/packages/resolve-dependencies/package.json index 83158d3d56f..f88f5f14c4e 100644 --- a/packages/resolve-dependencies/package.json +++ b/packages/resolve-dependencies/package.json @@ -50,6 +50,7 @@ "get-npm-tarball-url": "^2.0.3", "is-inner-link": "^4.0.0", "is-subdir": "^1.2.0", + "p-defer": "^3.0.0", "path-exists": "^4.0.0", "promise-share": "^1.0.0", "ramda": "npm:@pnpm/ramda@0.28.1", diff --git a/packages/resolve-dependencies/src/nodeIdUtils.ts b/packages/resolve-dependencies/src/nodeIdUtils.ts index f0c857525cb..6fef2be16a0 100644 --- a/packages/resolve-dependencies/src/nodeIdUtils.ts +++ b/packages/resolve-dependencies/src/nodeIdUtils.ts @@ -1,3 +1,8 @@ +export function nodeIdContains (nodeId: string, pkgId: string) { + const pkgIds = splitNodeId(nodeId) + return pkgIds.includes(pkgId) +} + export function nodeIdContainsSequence (nodeId: string, pkgId1: string, pkgId2: string) { const pkgIds = splitNodeId(nodeId) pkgIds.pop() diff --git a/packages/resolve-dependencies/src/resolveDependencies.ts b/packages/resolve-dependencies/src/resolveDependencies.ts index 5ce3eb8c7ff..013fbd85647 100644 --- a/packages/resolve-dependencies/src/resolveDependencies.ts +++ b/packages/resolve-dependencies/src/resolveDependencies.ts @@ -40,7 +40,10 @@ import { } from '@pnpm/types' import * as dp from 'dependency-path' import exists from 'path-exists' +import pDefer from 'p-defer' +import pShare from 'promise-share' import isEmpty from 'ramda/src/isEmpty' +import fromPairs from 'ramda/src/fromPairs' import omit from 'ramda/src/omit' import zipWith from 'ramda/src/zipWith' import semver from 'semver' @@ -50,6 +53,7 @@ import { safeIntersect } from './mergePeers' import { createNodeId, nodeIdContainsSequence, + nodeIdContains, splitNodeId, } from './nodeIdUtils' import wantedDepIsLocallyAvailable from './wantedDepIsLocallyAvailable' @@ -154,12 +158,19 @@ export interface ResolutionContext { virtualStoreDir: string updateMatching?: (pkgName: string) => boolean workspacePackages?: WorkspacePackages + missingPeersOfChildrenByPkgId: Record } export type MissingPeers = Record export type ResolvedPeers = Record +interface MissingPeersOfChildren { + resolve: (missingPeers: MissingPeers) => void + reject: (err: Error) => void + get: () => Promise +} + export type PkgAddress = { alias: string depIsLinked: boolean @@ -175,6 +186,7 @@ export type PkgAddress = { updated: boolean rootDir: string missingPeers: MissingPeers + missingPeersOfChildren?: MissingPeersOfChildren resolvedPeers: ResolvedPeers publishedAt?: string } & ({ @@ -281,11 +293,16 @@ export async function resolveRootDependencies ( if (!Object.keys(importerResolutionResult.missingPeers).length) break const wantedDependencies = getNonDevWantedDependencies({ dependencies: importerResolutionResult.missingPeers }) - importerResolutionResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { + const importerResolutionResultt = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { ...options, parentPkgAliases, publishedBy, }) + importerResolutionResult = { + pkgAddresses: importerResolutionResultt.pkgAddresses, + missingPeers: (await importerResolutionResultt.peers).missingPeers, + resolvedPeers: (await importerResolutionResultt.peers).resolvedPeers, + } pkgAddresses.push(...importerResolutionResult.pkgAddresses) } return pkgAddresses @@ -294,6 +311,14 @@ export async function resolveRootDependencies ( } interface ResolvedDependenciesResult { + pkgAddresses: Array + peers: Promise<{ + missingPeers: MissingPeers + resolvedPeers: ResolvedPeers + }> +} + +interface ResolvedDependenciesResultWithoutPromise { pkgAddresses: Array missingPeers: MissingPeers resolvedPeers: ResolvedPeers @@ -308,7 +333,7 @@ export interface ImporterToResolve { } interface ResolveDependenciesOfImportersResult { - pkgAddressesByImportersWithoutPeers: ResolvedDependenciesResult[] + pkgAddressesByImportersWithoutPeers: ResolvedDependenciesResultWithoutPromise[] publishedBy?: Date time?: Record } @@ -328,6 +353,7 @@ async function resolveDependenciesOfImporters ( const resolveResults = await Promise.all( zipWith(async (extendedWantedDeps, importer) => { const postponedResolutionsQueue: PostponedResolutionFunction[] = [] + const postponedMissingPeersQueue: PostponedResolutionFunction[] = [] const pkgAddresses: PkgAddress[] = [] ;(await Promise.all( extendedWantedDeps.map((extendedWantedDep) => resolveDependenciesOfDependency( @@ -340,15 +366,18 @@ async function resolveDependenciesOfImporters ( }, extendedWantedDep )) - )).forEach(({ resolveDependencyResult, postponedResolution }) => { + )).forEach(({ resolveDependencyResult, missingPeersPromise, postponedResolution }) => { if (resolveDependencyResult) { pkgAddresses.push(resolveDependencyResult as PkgAddress) } if (postponedResolution) { postponedResolutionsQueue.push(postponedResolution) } + if (missingPeersPromise) { + postponedMissingPeersQueue.push(missingPeersPromise) + } }) - return { pkgAddresses, postponedResolutionsQueue } + return { pkgAddresses, postponedResolutionsQueue, postponedMissingPeersQueue } }, extendedWantedDepsByImporters, importers) ) let publishedBy: Date | undefined @@ -360,7 +389,7 @@ async function resolveDependenciesOfImporters ( time = result.newTime } } - const pkgAddressesByImportersWithoutPeers = await Promise.all(zipWith(async (importer, { pkgAddresses, postponedResolutionsQueue }) => { + const pkgAddressesByImportersWithoutPeers = await Promise.all(zipWith(async (importer, { pkgAddresses, postponedResolutionsQueue, postponedMissingPeersQueue }) => { const newPreferredVersions = { ...importer.preferredVersions } const newParentPkgAliases = { ...importer.parentPkgAliases } for (const pkgAddress of pkgAddresses) { @@ -392,16 +421,20 @@ async function resolveDependenciesOfImporters ( resolvedPeers: {}, } } + const missingPeersPromise = await Promise.all( + postponedMissingPeersQueue.map((postponedMissingPeers) => postponedMissingPeers(postponedResolutionOpts)) + ) const allMissingPeers = mergePkgsDeps( [ ...pkgAddresses, ...childrenResults, + ...missingPeersPromise, ].map(({ missingPeers }) => missingPeers).filter(Boolean) ) return { missingPeers: allMissingPeers, pkgAddresses, - resolvedPeers: [...pkgAddresses, ...childrenResults].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), + resolvedPeers: [...pkgAddresses, ...childrenResults, ...missingPeersPromise].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), } }, importers, resolveResults)) return { @@ -440,6 +473,7 @@ export async function resolveDependencies ( resolvedDependencies: options.resolvedDependencies, }) const postponedResolutionsQueue: PostponedResolutionFunction[] = [] + const postponedMissingPeersQueue: PostponedResolutionFunction[] = [] const pkgAddresses: PkgAddress[] = [] ;(await Promise.all( extendedWantedDeps.map((extendedWantedDep) => resolveDependenciesOfDependency( @@ -448,19 +482,22 @@ export async function resolveDependencies ( options, extendedWantedDep )) - )).forEach(({ resolveDependencyResult, postponedResolution }) => { + )).forEach(({ resolveDependencyResult, postponedResolution, missingPeersPromise }) => { if (resolveDependencyResult) { pkgAddresses.push(resolveDependencyResult as PkgAddress) } if (postponedResolution) { postponedResolutionsQueue.push(postponedResolution) } + if (missingPeersPromise) { + postponedMissingPeersQueue.push(missingPeersPromise) + } }) const newPreferredVersions = { ...preferredVersions } - const newParentPkgAliases = { ...options.parentPkgAliases } + const newNewParentPkgAliases = {} for (const pkgAddress of pkgAddresses) { - if (newParentPkgAliases[pkgAddress.alias] !== true) { - newParentPkgAliases[pkgAddress.alias] = pkgAddress + if (newNewParentPkgAliases[pkgAddress.alias] !== true) { + newNewParentPkgAliases[pkgAddress.alias] = pkgAddress } if (pkgAddress.updated) { ctx.updatedSet.add(pkgAddress.alias) @@ -472,6 +509,10 @@ export async function resolveDependencies ( } newPreferredVersions[resolvedPackage.name][resolvedPackage.version] = 'version' } + const newParentPkgAliases = { + ...options.parentPkgAliases, + ...newNewParentPkgAliases, + } const postponedResolutionOpts = { preferredVersions: newPreferredVersions, parentPkgAliases: newParentPkgAliases, @@ -482,21 +523,35 @@ export async function resolveDependencies ( ) if (!ctx.autoInstallPeers) { return { - missingPeers: {}, + peers: Promise.resolve({ + missingPeers: {}, + resolvedPeers: {}, + }), pkgAddresses, - resolvedPeers: {}, } } - const allMissingPeers = mergePkgsDeps( - [ - ...pkgAddresses, - ...childrenResults, - ].map(({ missingPeers }) => missingPeers).filter(Boolean) - ) + const peers = (async () => { + const results = await Promise.all( + postponedMissingPeersQueue.map((postponedMissingPeers) => postponedMissingPeers(postponedResolutionOpts)) + ) + const allMissingPeers = mergePkgsDeps( + [ + ...pkgAddresses.map((pkgAddress) => ({ + ...pkgAddress, + missingPeers: fromPairs(Object.entries(pkgAddress.missingPeers || {}).filter(([peerName]) => !newNewParentPkgAliases[peerName])), + })), + ...childrenResults, + ...results, + ].map(({ missingPeers }) => missingPeers).filter(Boolean) + ) + return { + missingPeers: allMissingPeers, + resolvedPeers: [...pkgAddresses, ...childrenResults, ...results].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), + } + })() return { - missingPeers: allMissingPeers, pkgAddresses, - resolvedPeers: [...pkgAddresses, ...childrenResults].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), + peers, } } @@ -528,6 +583,7 @@ interface ExtendedWantedDependency { interface ResolveDependenciesOfDependency { postponedResolution?: PostponedResolutionFunction + missingPeersPromise?: PostponedResolutionFunction resolveDependencyResult: ResolveDependencyResult } @@ -585,7 +641,31 @@ async function resolveDependenciesOfDependency ( } return { resolveDependencyResult } } - if (!resolveDependencyResult.isNew) return { resolveDependencyResult } + if (!resolveDependencyResult.isNew) { + return { + resolveDependencyResult, + missingPeersPromise: resolveDependencyResult.missingPeersOfChildren != null + ? async (postponedResolutionOpts) => { + const missingPeers = await resolveDependencyResult.missingPeersOfChildren!.get() + const newMissing = {} as MissingPeers + const resolvedPeers = {} as ResolvedPeers + for (const [peerName, peerVersion] of Object.entries(missingPeers)) { + if (postponedResolutionOpts.parentPkgAliases[peerName]) { + if (postponedResolutionOpts.parentPkgAliases[peerName] !== true) { + resolvedPeers[peerName] = postponedResolutionOpts.parentPkgAliases[peerName] as PkgAddress + } + } else { + newMissing[peerName] = peerVersion + } + } + return { + missingPeers: newMissing, + resolvedPeers, + } + } + : undefined, + } + } const postponedResolution = resolveChildren.bind(null, ctx, { parentPkg: resolveDependencyResult, @@ -596,7 +676,31 @@ async function resolveDependenciesOfDependency ( }) return { resolveDependencyResult, - postponedResolution, + postponedResolution: async (postponedResolutionOpts) => { + try { + const result = await postponedResolution(postponedResolutionOpts) + const missingPeers = (await result.peers).missingPeers + resolveDependencyResult.missingPeersOfChildren!.resolve(missingPeers) + const newMissing = {} as MissingPeers + const resolvedPeers = {} as ResolvedPeers + for (const [peerName, peerVersion] of Object.entries(missingPeers)) { + if (postponedResolutionOpts.parentPkgAliases[peerName]) { + if (postponedResolutionOpts.parentPkgAliases[peerName] !== true) { + resolvedPeers[peerName] = postponedResolutionOpts.parentPkgAliases[peerName] as PkgAddress + } + } else { + newMissing[peerName] = peerVersion + } + } + return { + resolvedPeers, + missingPeers: newMissing, + } + } catch (err) { + resolveDependencyResult.missingPeersOfChildren!.reject(err as Error) + throw err + } + }, } } @@ -644,8 +748,7 @@ async function resolveChildren ( const wantedDependencies = getNonDevWantedDependencies(parentPkg.pkg) const { pkgAddresses, - missingPeers, - resolvedPeers, + peers, } = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { currentDepth: parentDepth + 1, @@ -675,8 +778,7 @@ async function resolveChildren ( resolvedPackage: ctx.resolvedPackagesByDepPath[parentPkg.depPath], } return { - missingPeers, - resolvedPeers, + peers, } } @@ -1159,6 +1261,25 @@ async function resolveDependency ( const rootDir = pkgResponse.body.resolution.type === 'directory' ? path.resolve(ctx.lockfileDir, pkgResponse.body.resolution['directory']) : options.prefix + let missingPeersOfChildren!: MissingPeersOfChildren | undefined + if (!nodeIdContains(options.parentPkg.nodeId, depPath)) { + if (ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id]) { + if (!options.parentPkg.nodeId.startsWith(ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].parentNodeId)) { + missingPeersOfChildren = ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeers + } + } else { + const p = pDefer() + missingPeersOfChildren = { + resolve: p.resolve, + reject: p.reject, + get: pShare(p.promise), + } + ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = { + parentNodeId: options.parentPkg.nodeId, + missingPeers: missingPeersOfChildren, + } + } + } return { alias: wantedDependency.alias || pkg.name, depIsLinked, @@ -1166,6 +1287,7 @@ async function resolveDependency ( isNew, nodeId, normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined, + missingPeersOfChildren, pkgId: pkgResponse.body.id, rootDir, ...getMissingPeers(pkg, options.parentPkgAliases), @@ -1195,11 +1317,7 @@ function getMissingPeers (pkg: PackageManifest, parentPkgAliases: ParentPkgAlias const missingPeers = {} as MissingPeers const resolvedPeers = {} as ResolvedPeers for (const [peerName, peerVersion] of Object.entries(pkg.peerDependencies ?? {})) { - if (parentPkgAliases[peerName]) { - if (parentPkgAliases[peerName] !== true) { - resolvedPeers[peerName] = parentPkgAliases[peerName] as PkgAddress - } - } else if (!pkg.peerDependenciesMeta?.[peerName]?.optional) { + if (!pkg.peerDependenciesMeta?.[peerName]?.optional) { missingPeers[peerName] = peerVersion } } diff --git a/packages/resolve-dependencies/src/resolveDependencyTree.ts b/packages/resolve-dependencies/src/resolveDependencyTree.ts index 758389b07ad..b45a1f93a8a 100644 --- a/packages/resolve-dependencies/src/resolveDependencyTree.ts +++ b/packages/resolve-dependencies/src/resolveDependencyTree.ts @@ -125,6 +125,7 @@ export default async function ( appliedPatches: new Set(), updatedSet: new Set(), workspacePackages: opts.workspacePackages, + missingPeersOfChildrenByPkgId: {}, } const resolveArgs: ImporterToResolve[] = importers.map((importer) => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1fa4b33952a..0dcf555f529 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4784,6 +4784,9 @@ importers: is-subdir: specifier: ^1.2.0 version: 1.2.0 + p-defer: + specifier: ^3.0.0 + version: 3.0.0 path-exists: specifier: ^4.0.0 version: 4.0.0