From 6caec8109b563fd27ad262ce06a1f587cebbc044 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sat, 30 Mar 2024 12:23:29 +0100 Subject: [PATCH] fix: hoist peer dependencies (#7841) --- .../core/src/getPeerDependencyIssues.ts | 2 + .../core/test/install/peerDependencies.ts | 18 ++++++- .../src/resolveDependencies.ts | 51 ++++++++++++------- .../src/resolveDependencyTree.ts | 8 ++- 4 files changed, 58 insertions(+), 21 deletions(-) diff --git a/pkg-manager/core/src/getPeerDependencyIssues.ts b/pkg-manager/core/src/getPeerDependencyIssues.ts index bdfb2ef5fd..61c6f47dfc 100644 --- a/pkg-manager/core/src/getPeerDependencyIssues.ts +++ b/pkg-manager/core/src/getPeerDependencyIssues.ts @@ -8,6 +8,7 @@ import { DEFAULT_REGISTRIES } from '@pnpm/normalize-registries' export type ListMissingPeersOptions = Partial & Pick(path.resolve(WANTED_LOCKFILE)) // eslint-disable-line + console.log(JSON.stringify(lockfile, null, 2)) expect(lockfile.snapshots['@pnpm.e2e/abc@1.0.0(@pnpm.e2e/peer-c@2.0.0)']).toBeTruthy() }) @@ -1756,7 +1757,8 @@ test('3 circular peers in workspace root', async () => { ], testDefaults({ allProjects, reporter, autoInstallPeers: false, resolvePeersFromWorkspaceRoot: true, strictPeerDependencies: false })) const lockfile = projects.root.readLockfile() - expect(lockfile.importers.pkg?.dependencies?.['@pnpm.e2e/circular-peers-1-of-3'].version).toBe('1.0.0(@pnpm.e2e/circular-peers-2-of-3@1.0.0(@pnpm.e2e/circular-peers-3-of-3@1.0.0)(@pnpm.e2e/peer-a@1.0.0))(@pnpm.e2e/peer-a@1.0.0)') + expect(Object.keys(lockfile.snapshots).length).toBe(4) + expect(lockfile.importers.pkg?.dependencies?.['@pnpm.e2e/circular-peers-1-of-3'].version).toBe('1.0.0(@pnpm.e2e/circular-peers-2-of-3@1.0.0)(@pnpm.e2e/peer-a@1.0.0)') }) test('resolves complex circular deps', async () => { @@ -1797,6 +1799,20 @@ test('optional peer dependency is resolved if it is installed anywhere in the de expect(lockfile.snapshots['@pnpm.e2e/abc-optional-peers@1.0.0(@pnpm.e2e/peer-a@1.0.0)(@pnpm.e2e/peer-b@1.0.0)(@pnpm.e2e/peer-c@1.0.0)']).toBeDefined() }) +test('optional peer dependency is resolved if it is installed anywhere in the dependency graph and auto install peers is false', async () => { + await addDistTag({ package: '@pnpm.e2e/abc-parent-with-ab', version: '1.0.0', distTag: 'latest' }) + const project = prepareEmpty() + + await addDependenciesToPackage( + {}, + ['@pnpm.e2e/abc-regular-deps@1.0.0', '@pnpm.e2e/abc-optional-peers@1.0.0'], + testDefaults({ autoInstallPeers: false }) + ) + + const lockfile = project.readLockfile() + expect(lockfile.snapshots['@pnpm.e2e/abc-optional-peers@1.0.0(@pnpm.e2e/peer-a@1.0.0)(@pnpm.e2e/peer-b@1.0.0)(@pnpm.e2e/peer-c@1.0.0)']).toBeDefined() +}) + // It is resolved on the second iteration only test('optional peer dependency is resolved if it is installed anywhere in the dependency graph and auto install peers is true #2', async () => { await addDistTag({ package: '@pnpm.e2e/abc-parent-with-ab', version: '1.0.0', distTag: 'latest' }) diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index ccbd410faf..b2f5842c7c 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -164,6 +164,7 @@ export interface ResolutionContext { virtualStoreDir: string workspacePackages?: WorkspacePackages missingPeersOfChildrenByPkgId: Record + hoistPeers?: boolean } export type MissingPeers = Record @@ -292,11 +293,13 @@ export async function resolveRootDependencies ( ): Promise { if (ctx.autoInstallPeers) { ctx.allPreferredVersions = getPreferredVersionsFromLockfileAndManifests(ctx.wantedLockfile.packages, []) + } else if (ctx.hoistPeers) { + ctx.allPreferredVersions = {} } const { pkgAddressesByImportersWithoutPeers, publishedBy, time } = await resolveDependenciesOfImporters(ctx, importers) const pkgAddressesByImporters = await Promise.all(zipWith(async (importerResolutionResult, { parentPkgAliases, preferredVersions, options }) => { const pkgAddresses = importerResolutionResult.pkgAddresses - if (!ctx.autoInstallPeers) return pkgAddresses + if (!ctx.hoistPeers) return pkgAddresses let prevMissingOptionalPeers: string[] = [] while (true) { for (const pkgAddress of importerResolutionResult.pkgAddresses) { @@ -306,12 +309,14 @@ export async function resolveRootDependencies ( for (const missingPeerName of Object.keys(missingRequiredPeers)) { parentPkgAliases[missingPeerName] = true } - // All the missing peers should get installed in the root. - // Otherwise, pending nodes will not work. - // even those peers should be hoisted that are not autoinstalled - for (const [resolvedPeerName, resolvedPeerAddress] of Object.entries(importerResolutionResult.resolvedPeers ?? {})) { - if (!parentPkgAliases[resolvedPeerName]) { - pkgAddresses.push(resolvedPeerAddress) + if (ctx.autoInstallPeers) { + // All the missing peers should get installed in the root. + // Otherwise, pending nodes will not work. + // even those peers should be hoisted that are not autoinstalled + for (const [resolvedPeerName, resolvedPeerAddress] of Object.entries(importerResolutionResult.resolvedPeers ?? {})) { + if (!parentPkgAliases[resolvedPeerName]) { + pkgAddresses.push(resolvedPeerAddress) + } } } const missingOptionalPeerNames = Array.from( @@ -323,13 +328,23 @@ export async function resolveRootDependencies ( ) ) if (!missingRequiredPeers.length && !missingOptionalPeerNames.length) break - const dependencies = Object.fromEntries( - missingRequiredPeers - .map(([peerName, { range }]) => { - if (!ctx.allPreferredVersions![peerName]) return [peerName, range] - return [peerName, Object.keys(ctx.allPreferredVersions![peerName]).join(' || ')] - }) - ) + const dependencies: Record = {} + for (const [peerName, { range }] of missingRequiredPeers) { + if (ctx.allPreferredVersions![peerName]) { + const versions: string[] = [] + const nonVersions: string[] = [] + for (const [spec, specType] of Object.entries(ctx.allPreferredVersions![peerName])) { + if (specType === 'version') { + versions.push(spec) + } else { + nonVersions.push(spec) + } + } + dependencies[peerName] = [semver.maxSatisfying(versions, '*'), ...nonVersions].join(' || ') + } else if (ctx.autoInstallPeers) { + dependencies[peerName] = range + } + } const nextMissingOptionalPeers: string[] = [] for (const missingOptionalPeerName of missingOptionalPeerNames) { if (ctx.allPreferredVersions![missingOptionalPeerName]) { @@ -465,7 +480,7 @@ async function resolveDependenciesOfImporters ( const childrenResults = await Promise.all( postponedResolutionsQueue.map((postponedResolution) => postponedResolution(postponedResolutionOpts)) ) - if (!ctx.autoInstallPeers) { + if (!ctx.hoistPeers) { return { missingPeers: {}, pkgAddresses, @@ -595,7 +610,7 @@ export async function resolveDependencies ( const childrenResults = await Promise.all( postponedResolutionsQueue.map((postponedResolution) => postponedResolution(postponedResolutionOpts)) ) - if (!ctx.autoInstallPeers) { + if (!ctx.hoistPeers) { return { resolvingPeers: Promise.resolve({ missingPeers: {}, @@ -1372,7 +1387,7 @@ async function resolveDependency ( ctx.resolvedPackagesByDepPath[depPath].prod = ctx.resolvedPackagesByDepPath[depPath].prod || !wantedDependency.dev && !wantedDependency.optional ctx.resolvedPackagesByDepPath[depPath].dev = ctx.resolvedPackagesByDepPath[depPath].dev || wantedDependency.dev ctx.resolvedPackagesByDepPath[depPath].optional = ctx.resolvedPackagesByDepPath[depPath].optional && currentIsOptional - if (ctx.autoInstallPeers) { + if (ctx.hoistPeers) { resolveChildren = !ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved && !ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.has(parentImporterId) ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.add(parentImporterId) @@ -1399,7 +1414,7 @@ async function resolveDependency ( ? path.resolve(ctx.lockfileDir, (pkgResponse.body.resolution as DirectoryResolution).directory) : options.prefix let missingPeersOfChildren!: MissingPeersOfChildren | undefined - if (ctx.autoInstallPeers && !nodeIdContains(options.parentPkg.nodeId, depPath)) { + if (ctx.hoistPeers && !nodeIdContains(options.parentPkg.nodeId, depPath)) { if (ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id]) { if (!options.parentPkg.nodeId.startsWith(ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].parentImporterId)) { missingPeersOfChildren = ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts index 789108e9d0..7059c59ad3 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts @@ -26,6 +26,7 @@ import { resolveRootDependencies, type ResolvedPackage, type ResolvedPackagesByDepPath, + type ResolutionContext, } from './resolveDependencies' export * from './nodeIdUtils' @@ -76,6 +77,7 @@ export interface ResolveDependenciesOptions { allowedDeprecatedVersions: AllowedDeprecatedVersions allowNonAppliedPatches: boolean currentLockfile: Lockfile + dedupePeerDependents?: boolean dryRun: boolean engineStrict: boolean force: boolean @@ -108,8 +110,9 @@ export async function resolveDependencyTree ( opts: ResolveDependenciesOptions ) { const wantedToBeSkippedPackageIds = new Set() - const ctx = { - autoInstallPeers: opts.autoInstallPeers === true, + const autoInstallPeers = opts.autoInstallPeers === true + const ctx: ResolutionContext = { + autoInstallPeers, autoInstallPeersFromHighestMatch: opts.autoInstallPeersFromHighestMatch === true, allowBuild: opts.allowBuild, allowedDeprecatedVersions: opts.allowedDeprecatedVersions, @@ -142,6 +145,7 @@ export async function resolveDependencyTree ( updatedSet: new Set(), workspacePackages: opts.workspacePackages, missingPeersOfChildrenByPkgId: {}, + hoistPeers: autoInstallPeers || opts.dedupePeerDependents, } const resolveArgs: ImporterToResolve[] = importers.map((importer) => {