From 96821291a816464ef8f77745a4a77249cdec6b94 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Thu, 25 Jul 2024 18:00:24 +0200 Subject: [PATCH] fix: install peer of optional peer (#8330) close #8323 --- .changeset/tasty-tips-know.md | 6 + .../core/test/install/autoInstallPeers.ts | 45 +++++++ .../src/resolveDependencies.ts | 126 ++++++++++-------- pnpm-lock.yaml | 70 +++++++--- 4 files changed, 172 insertions(+), 75 deletions(-) create mode 100644 .changeset/tasty-tips-know.md diff --git a/.changeset/tasty-tips-know.md b/.changeset/tasty-tips-know.md new file mode 100644 index 0000000000..77d58bf3d9 --- /dev/null +++ b/.changeset/tasty-tips-know.md @@ -0,0 +1,6 @@ +--- +"@pnpm/resolve-dependencies": patch +"pnpm": patch +--- + +Peer dependencies of optional peer dependencies should be automatically installed [#8323](https://github.com/pnpm/pnpm/issues/8323). diff --git a/pkg-manager/core/test/install/autoInstallPeers.ts b/pkg-manager/core/test/install/autoInstallPeers.ts index 08040180e7..918df4f162 100644 --- a/pkg-manager/core/test/install/autoInstallPeers.ts +++ b/pkg-manager/core/test/install/autoInstallPeers.ts @@ -619,3 +619,48 @@ test('auto install hoisted peer dependency', async () => { '@pnpm.e2e/peer-c@2.0.0', ]) }) + +test('auto install peer of optional peer', async () => { + const project = prepareEmpty() + await mutateModules([ + { + mutation: 'install', + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + mutation: 'install', + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ], testDefaults({ + allProjects: [ + { + buildIndex: 0, + manifest: { + name: 'project-1', + dependencies: { + '@pnpm.e2e/has-optional-peer-with-peer': '1.0.0', + }, + }, + rootDir: path.resolve('project-1') as ProjectRootDir, + }, + { + buildIndex: 0, + manifest: { + name: 'project-2', + dependencies: { + '@pnpm.e2e/has-y-peer': '1.0.0', + }, + }, + rootDir: path.resolve('project-2') as ProjectRootDir, + }, + ], + autoInstallPeers: true, + })) + const lockfile = project.readLockfile() + expect(Object.keys(lockfile.snapshots)).toStrictEqual([ + '@pnpm.e2e/has-optional-peer-with-peer@1.0.0(@pnpm.e2e/has-y-peer@1.0.0(@pnpm/y@2.0.0))', + '@pnpm.e2e/has-y-peer@1.0.0(@pnpm/y@2.0.0)', + '@pnpm/y@2.0.0', + ]) + project.hasNot('@pnpm.e2e/peer-a') +}) diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 76d4e535a8..047cfab1ca 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -303,72 +303,86 @@ export async function resolveRootDependencies ( 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.hoistPeers) return pkgAddresses - const allMissingOptionalPeers: Record = {} - while (true) { - for (const pkgAddress of importerResolutionResult.pkgAddresses) { - parentPkgAliases[pkgAddress.alias] = true - } - const [missingOptionalPeers, missingRequiredPeers] = partition(([, { optional }]) => optional, Object.entries(importerResolutionResult.missingPeers ?? {})) - for (const missingPeerName of Object.keys(missingRequiredPeers)) { - parentPkgAliases[missingPeerName] = true - } - 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) + if (!ctx.hoistPeers) { + return { + pkgAddressesByImporters: pkgAddressesByImportersWithoutPeers.map(({ pkgAddresses }) => pkgAddresses), + time, + } + } + /* eslint-disable no-await-in-loop */ + while (true) { + const allMissingOptionalPeersByImporters = await Promise.all(pkgAddressesByImportersWithoutPeers.map(async (importerResolutionResult, index) => { + const { parentPkgAliases, preferredVersions, options } = importers[index] + const allMissingOptionalPeers: Record = {} + while (true) { + for (const pkgAddress of importerResolutionResult.pkgAddresses) { + parentPkgAliases[pkgAddress.alias] = true + } + const [missingOptionalPeers, missingRequiredPeers] = partition(([, { optional }]) => optional, Object.entries(importerResolutionResult.missingPeers ?? {})) + for (const missingPeerName of Object.keys(missingRequiredPeers)) { + parentPkgAliases[missingPeerName] = true + } + 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]) { + importerResolutionResult.pkgAddresses.push(resolvedPeerAddress) + } } } - } - for (const [missingOptionalPeerName, { range: missingOptionalPeerRange }] of missingOptionalPeers) { - if (!allMissingOptionalPeers[missingOptionalPeerName]) { - allMissingOptionalPeers[missingOptionalPeerName] = [missingOptionalPeerRange] - } else if (!allMissingOptionalPeers[missingOptionalPeerName].includes(missingOptionalPeerRange)) { - allMissingOptionalPeers[missingOptionalPeerName].push(missingOptionalPeerRange) + for (const [missingOptionalPeerName, { range: missingOptionalPeerRange }] of missingOptionalPeers) { + if (!allMissingOptionalPeers[missingOptionalPeerName]) { + allMissingOptionalPeers[missingOptionalPeerName] = [missingOptionalPeerRange] + } else if (!allMissingOptionalPeers[missingOptionalPeerName].includes(missingOptionalPeerRange)) { + allMissingOptionalPeers[missingOptionalPeerName].push(missingOptionalPeerRange) + } } - } - if (!missingRequiredPeers.length) break - const dependencies = hoistPeers(missingRequiredPeers, ctx) - if (!Object.keys(dependencies).length) break - const wantedDependencies = getNonDevWantedDependencies({ dependencies }) + if (!missingRequiredPeers.length) break + const dependencies = hoistPeers(missingRequiredPeers, ctx) + if (!Object.keys(dependencies).length) break + const wantedDependencies = getNonDevWantedDependencies({ dependencies }) - // eslint-disable-next-line no-await-in-loop - const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { - ...options, - parentPkgAliases, - publishedBy, - }) - importerResolutionResult = { - pkgAddresses: resolveDependenciesResult.pkgAddresses, - // eslint-disable-next-line no-await-in-loop - ...filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases), - } - pkgAddresses.push(...importerResolutionResult.pkgAddresses) - } - if (Object.keys(allMissingOptionalPeers).length && ctx.allPreferredVersions) { - const optionalDependencies = getHoistableOptionalPeers(allMissingOptionalPeers, ctx.allPreferredVersions) - if (Object.keys(optionalDependencies).length) { - const wantedDependencies = getNonDevWantedDependencies({ optionalDependencies }) const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { ...options, parentPkgAliases, publishedBy, }) - importerResolutionResult = { - pkgAddresses: resolveDependenciesResult.pkgAddresses, - ...filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases), - } - pkgAddresses.push(...importerResolutionResult.pkgAddresses) + importerResolutionResult.pkgAddresses.push(...resolveDependenciesResult.pkgAddresses) + Object.assign(importerResolutionResult, + filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases) + ) } - } - return pkgAddresses - }, pkgAddressesByImportersWithoutPeers, importers)) - return { pkgAddressesByImporters, time } + return allMissingOptionalPeers + })) + let hasNewMissingPeers = false + await Promise.all(allMissingOptionalPeersByImporters.map(async (allMissingOptionalPeers, index) => { + const { preferredVersions, parentPkgAliases, options } = importers[index] + if (Object.keys(allMissingOptionalPeers).length && ctx.allPreferredVersions) { + const optionalDependencies = getHoistableOptionalPeers(allMissingOptionalPeers, ctx.allPreferredVersions) + if (Object.keys(optionalDependencies).length) { + hasNewMissingPeers = true + const wantedDependencies = getNonDevWantedDependencies({ optionalDependencies }) + const resolveDependenciesResult = await resolveDependencies(ctx, preferredVersions, wantedDependencies, { + ...options, + parentPkgAliases, + publishedBy, + }) + pkgAddressesByImportersWithoutPeers[index].pkgAddresses.push(...resolveDependenciesResult.pkgAddresses) + Object.assign(pkgAddressesByImportersWithoutPeers[index], + filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases) + ) + } + } + })) + if (!hasNewMissingPeers) break + } + /* eslint-enable no-await-in-loop */ + return { + pkgAddressesByImporters: pkgAddressesByImportersWithoutPeers.map(({ pkgAddresses }) => pkgAddresses), + time, + } } interface ResolvedDependenciesResult { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b8a380cafc..712011d260 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -14689,7 +14689,7 @@ snapshots: '@jest/console@29.7.0': dependencies: '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 jest-message-util: 29.7.0 jest-util: 29.7.0 @@ -14702,14 +14702,14 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0(@babel/types@7.24.7) '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 ansi-escapes: 4.3.2 chalk: 4.1.2 ci-info: 3.9.0 exit: 0.1.2 graceful-fs: 4.2.11(patch_hash=ivtm2a2cfr5pomcfbedhmr5v2q) jest-changed-files: 29.7.0 - jest-config: 29.7.0(@babel/types@7.24.7)(@types/node@18.19.34)(ts-node@10.9.2(@types/node@18.19.34)(typescript@5.4.5)) + jest-config: 29.7.0(@babel/types@7.24.7)(@types/node@20.14.9)(ts-node@10.9.2(@types/node@18.19.34)(typescript@5.4.5)) jest-haste-map: 29.7.0 jest-message-util: 29.7.0 jest-regex-util: 29.6.3 @@ -14735,7 +14735,7 @@ snapshots: dependencies: '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 jest-mock: 29.7.0 '@jest/expect-utils@29.7.0': @@ -14753,7 +14753,7 @@ snapshots: dependencies: '@jest/types': 29.6.3 '@sinonjs/fake-timers': 10.3.0 - '@types/node': 18.19.34 + '@types/node': 20.14.9 jest-message-util: 29.7.0 jest-mock: 29.7.0 jest-util: 29.7.0 @@ -14775,7 +14775,7 @@ snapshots: '@jest/transform': 29.7.0(@babel/types@7.24.7) '@jest/types': 29.6.3 '@jridgewell/trace-mapping': 0.3.25 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 collect-v8-coverage: 1.0.2 exit: 0.1.2 @@ -14847,7 +14847,7 @@ snapshots: '@jest/schemas': 29.6.3 '@types/istanbul-lib-coverage': 2.0.6 '@types/istanbul-reports': 3.0.4 - '@types/node': 18.19.34 + '@types/node': 20.14.9 '@types/yargs': 17.0.32 chalk: 4.1.2 @@ -15484,7 +15484,7 @@ snapshots: '@types/isexe@2.0.2': dependencies: - '@types/node': 18.19.34 + '@types/node': 20.14.9 '@types/istanbul-lib-coverage@2.0.6': {} @@ -15607,7 +15607,7 @@ snapshots: '@types/touch@3.1.5': dependencies: - '@types/node': 18.19.34 + '@types/node': 20.14.9 '@types/treeify@1.0.3': {} @@ -15795,7 +15795,7 @@ snapshots: '@yarnpkg/core@4.0.3(typanion@3.14.0)': dependencies: '@arcanis/slice-ansi': 1.1.1 - '@types/semver': 7.5.3 + '@types/semver': 7.5.8 '@types/treeify': 1.0.3 '@yarnpkg/fslib': 3.1.0 '@yarnpkg/libzip': 3.1.0(@yarnpkg/fslib@3.1.0) @@ -18445,7 +18445,7 @@ snapshots: '@jest/expect': 29.7.0 '@jest/test-result': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 co: 4.6.0 dedent: 1.5.3 @@ -18518,6 +18518,38 @@ snapshots: - babel-plugin-macros - supports-color + jest-config@29.7.0(@babel/types@7.24.7)(@types/node@20.14.9)(ts-node@10.9.2(@types/node@18.19.34)(typescript@5.4.5)): + dependencies: + '@babel/core': 7.24.7 + '@jest/test-sequencer': 29.7.0 + '@jest/types': 29.6.3 + babel-jest: 29.7.0(@babel/core@7.24.7)(@babel/types@7.24.7) + chalk: 4.1.2 + ci-info: 3.9.0 + deepmerge: 4.3.1 + glob: 7.2.3 + graceful-fs: 4.2.11(patch_hash=ivtm2a2cfr5pomcfbedhmr5v2q) + jest-circus: 29.7.0(@babel/types@7.24.7) + jest-environment-node: 29.7.0 + jest-get-type: 29.6.3 + jest-regex-util: 29.6.3 + jest-resolve: 29.7.0 + jest-runner: 29.7.0(@babel/types@7.24.7) + jest-util: 29.7.0 + jest-validate: 29.7.0 + micromatch: 4.0.7 + parse-json: 5.2.0 + pretty-format: 29.7.0 + slash: 3.0.0 + strip-json-comments: 3.1.1 + optionalDependencies: + '@types/node': 20.14.9 + ts-node: 10.9.2(@types/node@18.19.34)(typescript@5.4.5) + transitivePeerDependencies: + - '@babel/types' + - babel-plugin-macros + - supports-color + jest-diff@29.7.0: dependencies: chalk: 4.1.2 @@ -18542,7 +18574,7 @@ snapshots: '@jest/environment': 29.7.0 '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 jest-mock: 29.7.0 jest-util: 29.7.0 @@ -18552,7 +18584,7 @@ snapshots: dependencies: '@jest/types': 29.6.3 '@types/graceful-fs': 4.1.9 - '@types/node': 18.19.34 + '@types/node': 20.14.9 anymatch: 3.1.3 fb-watchman: 2.0.2 graceful-fs: 4.2.11(patch_hash=ivtm2a2cfr5pomcfbedhmr5v2q) @@ -18591,7 +18623,7 @@ snapshots: jest-mock@29.7.0: dependencies: '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 jest-util: 29.7.0 jest-pnp-resolver@1.2.3(jest-resolve@29.7.0): @@ -18626,7 +18658,7 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0(@babel/types@7.24.7) '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 emittery: 0.13.1 graceful-fs: 4.2.11(patch_hash=ivtm2a2cfr5pomcfbedhmr5v2q) @@ -18655,7 +18687,7 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0(@babel/types@7.24.7) '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 cjs-module-lexer: 1.3.1 collect-v8-coverage: 1.0.2 @@ -18702,7 +18734,7 @@ snapshots: jest-util@29.7.0: dependencies: '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 chalk: 4.1.2 ci-info: 3.9.0 graceful-fs: 4.2.11(patch_hash=ivtm2a2cfr5pomcfbedhmr5v2q) @@ -18721,7 +18753,7 @@ snapshots: dependencies: '@jest/test-result': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 18.19.34 + '@types/node': 20.14.9 ansi-escapes: 4.3.2 chalk: 4.1.2 emittery: 0.13.1 @@ -18730,7 +18762,7 @@ snapshots: jest-worker@29.7.0: dependencies: - '@types/node': 18.19.34 + '@types/node': 20.14.9 jest-util: 29.7.0 merge-stream: 2.0.0 supports-color: 8.1.1