diff --git a/.changeset/gentle-lies-play.md b/.changeset/gentle-lies-play.md new file mode 100644 index 00000000000..dda5fa546d3 --- /dev/null +++ b/.changeset/gentle-lies-play.md @@ -0,0 +1,13 @@ +--- +"@pnpm/resolve-dependencies": patch +"pnpm": patch +--- + +When `dedupe-peer-dependents` is enabled (default), use the path (not id) to +determine compatibility. + +When multiple dependency groups can be deduplicated, the +latter ones are sorted according to number of peers to allow them to +benefit from deduplication. + +Resolves: [#6605](https://github.com/pnpm/pnpm/issues/6605) diff --git a/pkg-manager/resolve-dependencies/src/resolvePeers.ts b/pkg-manager/resolve-dependencies/src/resolvePeers.ts index cec217b77e9..711fe0c9caa 100644 --- a/pkg-manager/resolve-dependencies/src/resolvePeers.ts +++ b/pkg-manager/resolve-dependencies/src/resolvePeers.ts @@ -108,17 +108,15 @@ export function resolvePeers ( node.children = mapValues((childNodeId) => pathsByNodeId[childNodeId] ?? childNodeId, node.children) }) - const dependenciesByProjectId: { [id: string]: { [alias: string]: string } } = {} + const dependenciesByProjectId: { [id: string]: Record } = {} for (const { directNodeIdsByAlias, id } of opts.projects) { dependenciesByProjectId[id] = mapValues((nodeId) => pathsByNodeId[nodeId], directNodeIdsByAlias) } if (opts.dedupePeerDependents) { - const depPathsMap = deduplicateDepPaths(depPathsByPkgId, depGraph) - Object.values(depGraph).forEach((node) => { - node.children = mapValues((childDepPath) => depPathsMap[childDepPath] ?? childDepPath, node.children) - }) + const duplicates = Object.values(depPathsByPkgId).filter((item) => item.length > 1) + const allDepPathsMap = deduplicateAll(depGraph, duplicates) for (const { id } of opts.projects) { - dependenciesByProjectId[id] = mapValues((depPath) => depPathsMap[depPath] ?? depPath, dependenciesByProjectId[id]) + dependenciesByProjectId[id] = mapValues((depPath) => allDepPathsMap[depPath] ?? depPath, dependenciesByProjectId[id]) } } return { @@ -132,15 +130,38 @@ function nodeDepsCount (node: GenericDependenciesGraphNode) { return Object.keys(node.children).length + node.resolvedPeerNames.length } +function deduplicateAll ( + depGraph: GenericDependenciesGraph, + duplicates: string[][] +): Record { + const { depPathsMap, remainingDuplicates } = deduplicateDepPaths(duplicates, depGraph) + if (remainingDuplicates.length === duplicates.length) { + return depPathsMap + } + Object.values(depGraph).forEach((node) => { + node.children = mapValues((childDepPath) => depPathsMap[childDepPath] ?? childDepPath, node.children) + }) + if (Object.keys(depPathsMap).length > 0) { + return { + ...depPathsMap, + ...deduplicateAll(depGraph, remainingDuplicates), + } + } + return depPathsMap +} + function deduplicateDepPaths ( - depPathsByPkgId: Record, + duplicates: string[][], depGraph: GenericDependenciesGraph ) { + const depCountSorter = (depPath1: string, depPath2: string) => nodeDepsCount(depGraph[depPath1]) - nodeDepsCount(depGraph[depPath2]) const depPathsMap: Record = {} - for (let depPaths of Object.values(depPathsByPkgId)) { - if (depPaths.length === 1) continue - depPaths = depPaths.sort((depPath1, depPath2) => nodeDepsCount(depGraph[depPath1]) - nodeDepsCount(depGraph[depPath2])) - let currentDepPaths = depPaths + const remainingDuplicates: string[][] = [] + + for (const depPaths of duplicates) { + const unresolvedDepPaths = new Set(depPaths) + let currentDepPaths = depPaths.sort(depCountSorter) + while (currentDepPaths.length) { const depPath1 = currentDepPaths.pop()! const nextDepPaths = [] @@ -148,15 +169,24 @@ function deduplicateDepPaths ( const depPath2 = currentDepPaths.pop()! if (isCompatibleAndHasMoreDeps(depGraph, depPath1, depPath2)) { depPathsMap[depPath2] = depPath1 + unresolvedDepPaths.delete(depPath1) + unresolvedDepPaths.delete(depPath2) } else { nextDepPaths.push(depPath2) } } nextDepPaths.push(...currentDepPaths) - currentDepPaths = nextDepPaths + currentDepPaths = nextDepPaths.sort(depCountSorter) + } + + if (unresolvedDepPaths.size) { + remainingDuplicates.push([...unresolvedDepPaths]) } } - return depPathsMap + return { + depPathsMap, + remainingDuplicates, + } } function isCompatibleAndHasMoreDeps ( @@ -166,8 +196,8 @@ function isCompatibleAndHasMoreDeps ( ) { const node1 = depGraph[depPath1] const node2 = depGraph[depPath2] - const node1DepPaths = Object.keys(node1.children) - const node2DepPaths = Object.keys(node2.children) + const node1DepPaths = Object.values(node1.children) + const node2DepPaths = Object.values(node2.children) return nodeDepsCount(node1) > nodeDepsCount(node2) && node2DepPaths.every((depPath) => node1DepPaths.includes(depPath)) && node2.resolvedPeerNames.every((depPath) => node1.resolvedPeerNames.includes(depPath)) diff --git a/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts b/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts new file mode 100644 index 00000000000..60fff069049 --- /dev/null +++ b/pkg-manager/resolve-dependencies/test/dedupeDepPaths.test.ts @@ -0,0 +1,106 @@ +import { resolvePeers } from '../lib/resolvePeers' + +test('packages are not deduplicated when versions do not match', () => { + const fooPkg = { + name: 'foo', + version: '1.0.0', + depPath: 'foo/1.0.0', + peerDependencies: { + bar: '1.0.0 || 2.0.0', + baz: '1.0.0 || 2.0.0', + }, + peerDependenciesMeta: { + baz: { + optional: true, + }, + }, + } + + const peers = Object.fromEntries( + [ + ['bar', '1.0.0'], + ['bar', '2.0.0'], + ['baz', '1.0.0'], + ['baz', '2.0.0'], + ].map(([name, version]) => [ + `${name}_${version.replace(/\./g, '_')}`, + { + name, + version, + depPath: `${name}/${version}`, + peerDependencies: {}, + }, + ]) + ) + + const { dependenciesByProjectId } = resolvePeers({ + projects: [ + { + directNodeIdsByAlias: { + foo: '>project1>foo/1.0.0>', + bar: '>project1>bar/1.0.0>', + }, + topParents: [], + rootDir: '', + id: 'project1', + }, + { + directNodeIdsByAlias: { + foo: '>project2>foo/1.0.0>', + bar: '>project2>bar/1.0.0>', + baz: '>project2>baz/1.0.0>', + }, + topParents: [], + rootDir: '', + id: 'project2', + }, + { + directNodeIdsByAlias: { + foo: '>project3>foo/1.0.0>', + bar: '>project3>bar/2.0.0>', + }, + topParents: [], + rootDir: '', + id: 'project3', + }, + { + directNodeIdsByAlias: { + foo: '>project4>foo/1.0.0>', + bar: '>project4>bar/2.0.0>', + baz: '>project4>baz/2.0.0>', + }, + topParents: [], + rootDir: '', + id: 'project4', + }, + ], + dependenciesTree: Object.fromEntries([ + ['>project1>foo/1.0.0>', fooPkg], + ['>project1>bar/1.0.0>', peers.bar_1_0_0], + + ['>project2>foo/1.0.0>', fooPkg], + ['>project2>bar/1.0.0>', peers.bar_1_0_0], + ['>project2>baz/1.0.0>', peers.baz_1_0_0], + + ['>project3>foo/1.0.0>', fooPkg], + ['>project3>bar/2.0.0>', peers.bar_2_0_0], + + ['>project4>foo/1.0.0>', fooPkg], + ['>project4>bar/2.0.0>', peers.bar_2_0_0], + ['>project4>baz/2.0.0>', peers.baz_2_0_0], + + ].map(([path, resolvedPackage]) => [path, { + children: {}, + installable: {}, + resolvedPackage, + depth: 0, + }])), + dedupePeerDependents: true, + virtualStoreDir: '', + lockfileDir: '', + }) + + expect(dependenciesByProjectId.project1.foo).toEqual(dependenciesByProjectId.project2.foo) + expect(dependenciesByProjectId.project1.foo).not.toEqual(dependenciesByProjectId.project3.foo) + expect(dependenciesByProjectId.project3.foo).toEqual(dependenciesByProjectId.project4.foo) +})