diff --git a/.changeset/thirty-squids-study.md b/.changeset/thirty-squids-study.md new file mode 100644 index 00000000000..9290504eae9 --- /dev/null +++ b/.changeset/thirty-squids-study.md @@ -0,0 +1,6 @@ +--- +"@pnpm/resolve-dependencies": patch +"pnpm": patch +--- + +When the same dependency with missing peers is used in multiple workspace projects, install the missing peers in each workspace project [#4820](https://github.com/pnpm/pnpm/issues/4820). diff --git a/packages/core/test/install/autoInstallPeers.ts b/packages/core/test/install/autoInstallPeers.ts index b740bcecd75..947d591533a 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. test #1', 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. test #2', 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..f610c1eb699 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,7 +186,7 @@ export type PkgAddress = { updated: boolean rootDir: string missingPeers: MissingPeers - resolvedPeers: ResolvedPeers + missingPeersOfChildren?: MissingPeersOfChildren publishedAt?: string } & ({ isLinkedDependency: true @@ -245,10 +256,13 @@ interface PostponedResolutionOpts { publishedBy?: Date } -type PostponedResolutionFunction = (opts: PostponedResolutionOpts) => Promise<{ +interface PeersResolutionResult { missingPeers: MissingPeers resolvedPeers: ResolvedPeers -}> +} + +type PostponedResolutionFunction = (opts: PostponedResolutionOpts) => Promise +type PostponedPeersResolutionFunction = (parentPkgAliases: ParentPkgAliases) => Promise interface ResolvedRootDependenciesResult { pkgAddressesByImporters: Array> @@ -281,11 +295,15 @@ export async function resolveRootDependencies ( if (!Object.keys(importerResolutionResult.missingPeers).length) break const wantedDependencies = getNonDevWantedDependencies({ dependencies: importerResolutionResult.missingPeers }) - importerResolutionResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { + const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { ...options, parentPkgAliases, publishedBy, }) + importerResolutionResult = { + pkgAddresses: resolveDependenciesResult.pkgAddresses, + ...(await resolveDependenciesResult.resolvingPeers), + } pkgAddresses.push(...importerResolutionResult.pkgAddresses) } return pkgAddresses @@ -295,8 +313,11 @@ export async function resolveRootDependencies ( interface ResolvedDependenciesResult { pkgAddresses: Array - missingPeers: MissingPeers - resolvedPeers: ResolvedPeers + resolvingPeers: Promise +} + +interface PkgAddressesByImportersWithoutPeers extends PeersResolutionResult { + pkgAddresses: Array } export interface ImporterToResolve { @@ -308,7 +329,7 @@ export interface ImporterToResolve { } interface ResolveDependenciesOfImportersResult { - pkgAddressesByImportersWithoutPeers: ResolvedDependenciesResult[] + pkgAddressesByImportersWithoutPeers: PkgAddressesByImportersWithoutPeers[] publishedBy?: Date time?: Record } @@ -328,6 +349,7 @@ async function resolveDependenciesOfImporters ( const resolveResults = await Promise.all( zipWith(async (extendedWantedDeps, importer) => { const postponedResolutionsQueue: PostponedResolutionFunction[] = [] + const postponedPeersResolutionQueue: PostponedPeersResolutionFunction[] = [] const pkgAddresses: PkgAddress[] = [] ;(await Promise.all( extendedWantedDeps.map((extendedWantedDep) => resolveDependenciesOfDependency( @@ -340,15 +362,18 @@ async function resolveDependenciesOfImporters ( }, extendedWantedDep )) - )).forEach(({ resolveDependencyResult, postponedResolution }) => { + )).forEach(({ resolveDependencyResult, postponedPeersResolution, postponedResolution }) => { if (resolveDependencyResult) { pkgAddresses.push(resolveDependencyResult as PkgAddress) } if (postponedResolution) { postponedResolutionsQueue.push(postponedResolution) } + if (postponedPeersResolution) { + postponedPeersResolutionQueue.push(postponedPeersResolution) + } }) - return { pkgAddresses, postponedResolutionsQueue } + return { pkgAddresses, postponedResolutionsQueue, postponedPeersResolutionQueue } }, extendedWantedDepsByImporters, importers) ) let publishedBy: Date | undefined @@ -360,7 +385,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, postponedPeersResolutionQueue }) => { const newPreferredVersions = { ...importer.preferredVersions } const newParentPkgAliases = { ...importer.parentPkgAliases } for (const pkgAddress of pkgAddresses) { @@ -392,16 +417,20 @@ async function resolveDependenciesOfImporters ( resolvedPeers: {}, } } + const postponedPeersResolution = await Promise.all( + postponedPeersResolutionQueue.map((postponedMissingPeers) => postponedMissingPeers(postponedResolutionOpts.parentPkgAliases)) + ) const allMissingPeers = mergePkgsDeps( [ ...pkgAddresses, ...childrenResults, + ...postponedPeersResolution, ].map(({ missingPeers }) => missingPeers).filter(Boolean) ) return { missingPeers: allMissingPeers, pkgAddresses, - resolvedPeers: [...pkgAddresses, ...childrenResults].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), + resolvedPeers: [...childrenResults, ...postponedPeersResolution].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), } }, importers, resolveResults)) return { @@ -440,6 +469,7 @@ export async function resolveDependencies ( resolvedDependencies: options.resolvedDependencies, }) const postponedResolutionsQueue: PostponedResolutionFunction[] = [] + const postponedPeersResolutionQueue: PostponedPeersResolutionFunction[] = [] const pkgAddresses: PkgAddress[] = [] ;(await Promise.all( extendedWantedDeps.map((extendedWantedDep) => resolveDependenciesOfDependency( @@ -448,19 +478,22 @@ export async function resolveDependencies ( options, extendedWantedDep )) - )).forEach(({ resolveDependencyResult, postponedResolution }) => { + )).forEach(({ resolveDependencyResult, postponedResolution, postponedPeersResolution }) => { if (resolveDependencyResult) { pkgAddresses.push(resolveDependencyResult as PkgAddress) } if (postponedResolution) { postponedResolutionsQueue.push(postponedResolution) } + if (postponedPeersResolution) { + postponedPeersResolutionQueue.push(postponedPeersResolution) + } }) const newPreferredVersions = { ...preferredVersions } - const newParentPkgAliases = { ...options.parentPkgAliases } + const currentParentPkgAliases = {} for (const pkgAddress of pkgAddresses) { - if (newParentPkgAliases[pkgAddress.alias] !== true) { - newParentPkgAliases[pkgAddress.alias] = pkgAddress + if (currentParentPkgAliases[pkgAddress.alias] !== true) { + currentParentPkgAliases[pkgAddress.alias] = pkgAddress } if (pkgAddress.updated) { ctx.updatedSet.add(pkgAddress.alias) @@ -472,6 +505,10 @@ export async function resolveDependencies ( } newPreferredVersions[resolvedPackage.name][resolvedPackage.version] = 'version' } + const newParentPkgAliases = { + ...options.parentPkgAliases, + ...currentParentPkgAliases, + } const postponedResolutionOpts = { preferredVersions: newPreferredVersions, parentPkgAliases: newParentPkgAliases, @@ -482,21 +519,66 @@ export async function resolveDependencies ( ) if (!ctx.autoInstallPeers) { return { - missingPeers: {}, + resolvingPeers: Promise.resolve({ + missingPeers: {}, + resolvedPeers: {}, + }), pkgAddresses, - resolvedPeers: {}, } } + return { + pkgAddresses, + resolvingPeers: startResolvingPeers({ + childrenResults, + pkgAddresses, + parentPkgAliases: options.parentPkgAliases, + currentParentPkgAliases, + postponedPeersResolutionQueue, + }), + } +} + +async function startResolvingPeers ( + { + childrenResults, + currentParentPkgAliases, + parentPkgAliases, + pkgAddresses, + postponedPeersResolutionQueue, + }: { + childrenResults: PeersResolutionResult[] + currentParentPkgAliases: ParentPkgAliases + parentPkgAliases: ParentPkgAliases + pkgAddresses: PkgAddress[] + postponedPeersResolutionQueue: PostponedPeersResolutionFunction[] + } +) { + const results = await Promise.all( + postponedPeersResolutionQueue.map((postponedPeersResolution) => postponedPeersResolution(parentPkgAliases)) + ) + const resolvedPeers = [...childrenResults, ...results].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}) const allMissingPeers = mergePkgsDeps( [ - ...pkgAddresses, + ...pkgAddresses.map((pkgAddress) => ({ + ...pkgAddress, + missingPeers: fromPairs( + Object.entries(pkgAddress.missingPeers || {}) + .filter(([peerName]) => { + if (!currentParentPkgAliases[peerName]) return true + if (currentParentPkgAliases[peerName] !== true) { + resolvedPeers[peerName] = currentParentPkgAliases[peerName] + } + return false + }) + ), + })), ...childrenResults, + ...results, ].map(({ missingPeers }) => missingPeers).filter(Boolean) ) return { missingPeers: allMissingPeers, - pkgAddresses, - resolvedPeers: [...pkgAddresses, ...childrenResults].reduce((acc, { resolvedPeers }) => Object.assign(acc, resolvedPeers), {}), + resolvedPeers, } } @@ -528,6 +610,7 @@ interface ExtendedWantedDependency { interface ResolveDependenciesOfDependency { postponedResolution?: PostponedResolutionFunction + postponedPeersResolution?: PostponedPeersResolutionFunction resolveDependencyResult: ResolveDependencyResult } @@ -585,7 +668,17 @@ async function resolveDependenciesOfDependency ( } return { resolveDependencyResult } } - if (!resolveDependencyResult.isNew) return { resolveDependencyResult } + if (!resolveDependencyResult.isNew) { + return { + resolveDependencyResult, + postponedPeersResolution: resolveDependencyResult.missingPeersOfChildren != null + ? async (parentPkgAliases) => { + const missingPeers = await resolveDependencyResult.missingPeersOfChildren!.get() + return filterMissingPeers(missingPeers, {}, parentPkgAliases) + } + : undefined, + } + } const postponedResolution = resolveChildren.bind(null, ctx, { parentPkg: resolveDependencyResult, @@ -596,7 +689,28 @@ async function resolveDependenciesOfDependency ( }) return { resolveDependencyResult, - postponedResolution, + postponedResolution: async (postponedResolutionOpts) => { + const { missingPeers, resolvedPeers } = await postponedResolution(postponedResolutionOpts) + resolveDependencyResult.missingPeersOfChildren!.resolve(missingPeers) + return filterMissingPeers(missingPeers, resolvedPeers, postponedResolutionOpts.parentPkgAliases) + }, + } +} + +function filterMissingPeers (missingPeers: MissingPeers, resolvedPeers: ResolvedPeers, parentPkgAliases: ParentPkgAliases): PeersResolutionResult { + const newMissing = {} as MissingPeers + for (const [peerName, peerVersion] of Object.entries(missingPeers)) { + if (parentPkgAliases[peerName]) { + if (parentPkgAliases[peerName] !== true) { + resolvedPeers[peerName] = parentPkgAliases[peerName] as PkgAddress + } + } else { + newMissing[peerName] = peerVersion + } + } + return { + resolvedPeers, + missingPeers: newMissing, } } @@ -644,8 +758,7 @@ async function resolveChildren ( const wantedDependencies = getNonDevWantedDependencies(parentPkg.pkg) const { pkgAddresses, - missingPeers, - resolvedPeers, + resolvingPeers, } = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { currentDepth: parentDepth + 1, @@ -674,10 +787,7 @@ async function resolveChildren ( installable: parentPkg.installable, resolvedPackage: ctx.resolvedPackagesByDepPath[parentPkg.depPath], } - return { - missingPeers, - resolvedPeers, - } + return resolvingPeers } function getDepsToResolve ( @@ -1159,6 +1269,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].missingPeersOfChildren + } + } else { + const p = pDefer() + missingPeersOfChildren = { + resolve: p.resolve, + reject: p.reject, + get: pShare(p.promise), + } + ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = { + parentNodeId: options.parentPkg.nodeId, + missingPeersOfChildren, + } + } + } return { alias: wantedDependency.alias || pkg.name, depIsLinked, @@ -1166,9 +1295,10 @@ async function resolveDependency ( isNew, nodeId, normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined, + missingPeersOfChildren, pkgId: pkgResponse.body.id, rootDir, - ...getMissingPeers(pkg, options.parentPkgAliases), + missingPeers: getMissingPeers(pkg), // Next fields are actually only needed when isNew = true installable, @@ -1191,19 +1321,14 @@ async function getManifestFromResponse ( } } -function getMissingPeers (pkg: PackageManifest, parentPkgAliases: ParentPkgAliases) { +function getMissingPeers (pkg: PackageManifest) { 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 } } - return { missingPeers, resolvedPeers } + return missingPeers } function pkgIsLeaf (pkg: PackageManifest) { 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