diff --git a/.changeset/soft-hotels-run.md b/.changeset/soft-hotels-run.md new file mode 100644 index 0000000000..c659b4f192 --- /dev/null +++ b/.changeset/soft-hotels-run.md @@ -0,0 +1,6 @@ +--- +"@pnpm/resolve-dependencies": patch +"pnpm": patch +--- + +Detection of circular peer dependencies should not crash with aliased dependencies [#8759](https://github.com/pnpm/pnpm/issues/8759). Fixes a regression introduced in the previous version. diff --git a/pkg-manager/core/test/install/peerDependencies.ts b/pkg-manager/core/test/install/peerDependencies.ts index b6839bff52..71792975ed 100644 --- a/pkg-manager/core/test/install/peerDependencies.ts +++ b/pkg-manager/core/test/install/peerDependencies.ts @@ -1892,3 +1892,18 @@ test('peer dependency cache is invalidated correctly when the peer of a peer mis expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@1.0.0))']).toBeTruthy() expect(lockfile.snapshots['@pnpm.e2e/repeat-peers.d@1.0.0(@pnpm.e2e/repeat-peers.b@1.0.0(@pnpm.e2e/repeat-peers.a@2.0.0))']).toBeTruthy() }) + +// Covers https://github.com/pnpm/pnpm/issues/8759 +test('detection of circular peer dependencies should not crash with aliased dependencies', async () => { + prepareEmpty() + + await install({ + dependencies: { + fastify: '5.1.0', + fastify4: 'npm:fastify@4.28.1', + 'ts-jest': '29.2.5', + }, + }, testDefaults()) + + expect(fs.existsSync(path.resolve(WANTED_LOCKFILE))).toBeTruthy() +}) diff --git a/pkg-manager/resolve-dependencies/src/resolvePeers.ts b/pkg-manager/resolve-dependencies/src/resolvePeers.ts index 54bc2c6699..4d91efd92a 100644 --- a/pkg-manager/resolve-dependencies/src/resolvePeers.ts +++ b/pkg-manager/resolve-dependencies/src/resolvePeers.ts @@ -366,6 +366,7 @@ interface ParentPkgInfo { type ParentPkgsOfNode = Map> async function resolvePeersOfNode ( + currentAlias: string, nodeId: NodeId, parentParentPkgs: ParentRefs, ctx: ResolvePeersContext & { @@ -519,7 +520,7 @@ async function resolvePeersOfNode ( addDepPathToGraph(resolvedPackage.pkgIdWithPatchHash as unknown as DepPath) } else { const peerIds: PeerId[] = [] - const pendingPeerNodeIds: NodeId[] = [] + const pendingPeers: Array<{ alias: string, nodeId: NodeId }> = [] for (const [alias, peerNodeId] of allResolvedPeers.entries()) { if (typeof peerNodeId === 'string' && peerNodeId.startsWith('link:')) { const linkedDir = peerNodeId.slice(5) @@ -534,13 +535,13 @@ async function resolvePeersOfNode ( peerIds.push(peerDepPath) continue } - pendingPeerNodeIds.push(peerNodeId) + pendingPeers.push({ alias, nodeId: peerNodeId }) } - if (pendingPeerNodeIds.length === 0) { + if (pendingPeers.length === 0) { const peersDirSuffix = createPeersDirSuffix(peerIds, ctx.peersSuffixMaxLength) addDepPathToGraph(`${resolvedPackage.pkgIdWithPatchHash}${peersDirSuffix}` as DepPath) } else { - calculateDepPathIfNeeded = calculateDepPath.bind(null, peerIds, pendingPeerNodeIds) + calculateDepPathIfNeeded = calculateDepPath.bind(null, peerIds, pendingPeers) } } @@ -553,31 +554,28 @@ async function resolvePeersOfNode ( async function calculateDepPath ( peerIds: PeerId[], - pendingPeerNodeIds: NodeId[], + pendingPeerNodes: Array<{ alias: string, nodeId: NodeId }>, cycles: string[][] ): Promise { - const cyclicPeerNodeIds = new Set() - const node = ctx.dependenciesTree.get(nodeId) - if (node?.resolvedPackage.name != null) { - for (const cycle of cycles) { - if (cycle.includes(node.resolvedPackage.name)) { - for (const peerNodeId of cycle) { - cyclicPeerNodeIds.add(peerNodeId) - } + const cyclicPeerAliases = new Set() + for (const cycle of cycles) { + if (cycle.includes(currentAlias)) { + for (const peerAlias of cycle) { + cyclicPeerAliases.add(peerAlias) } } } const peersDirSuffix = createPeersDirSuffix([ ...peerIds, - ...await Promise.all(pendingPeerNodeIds - .map(async (peerNodeId) => { - const peerNode = ctx.dependenciesTree.get(peerNodeId)?.resolvedPackage as T - if (peerNode && cyclicPeerNodeIds.has(peerNode.name)) { - const id = `${peerNode.name}@${peerNode.version}` - ctx.pathsByNodeIdPromises.get(peerNodeId)?.resolve(id as DepPath) + ...await Promise.all(pendingPeerNodes + .map(async (pendingPeer) => { + if (cyclicPeerAliases.has(pendingPeer.alias)) { + const { name, version } = ctx.dependenciesTree.get(pendingPeer.nodeId)?.resolvedPackage as T + const id = `${name}@${version}` + ctx.pathsByNodeIdPromises.get(pendingPeer.nodeId)?.resolve(id as DepPath) return id } - return ctx.pathsByNodeIdPromises.get(peerNodeId)!.promise + return ctx.pathsByNodeIdPromises.get(pendingPeer.nodeId)!.promise }) ), ], ctx.peersSuffixMaxLength) @@ -794,6 +792,7 @@ async function resolvePeersOfChildren ( // We check repeated first as the peers resolution of those probably are cached already. const [repeated, notRepeated] = partition(([alias]) => parentPkgs[alias] != null, Object.entries(children)) const nodeIds = Array.from(new Set([...repeated, ...notRepeated].map(([, nodeId]) => nodeId))) + const aliasByNodeId = Object.fromEntries(Object.entries(children).map(([alias, nodeId]) => [nodeId, alias])) for (const nodeId of nodeIds) { if (!ctx.pathsByNodeIdPromises.has(nodeId)) { @@ -822,12 +821,13 @@ async function resolvePeersOfChildren ( ctx.parentPkgsOfNode.set(childNodeId, parentDepPaths) } for (const childNodeId of nodeIds) { + const currentAlias = aliasByNodeId[childNodeId] const { resolvedPeers, missingPeers, calculateDepPath, finishing, - } = await resolvePeersOfNode(childNodeId, parentPkgs, ctx) // eslint-disable-line no-await-in-loop + } = await resolvePeersOfNode(currentAlias, childNodeId, parentPkgs, ctx) // eslint-disable-line no-await-in-loop if (finishing) { finishingList.push(finishing) } @@ -838,15 +838,8 @@ async function resolvePeersOfChildren ( for (const [peerName, peerNodeId] of resolvedPeers) { allResolvedPeers.set(peerName, peerNodeId) edges.push(peerName) - const node = ctx.dependenciesTree.get(peerNodeId) - if (node?.resolvedPackage.name && node.resolvedPackage.name !== peerName) { - edges.push(node.resolvedPackage.name) - } - } - const childNode = ctx.dependenciesTree.get(childNodeId) - if (childNode?.resolvedPackage.name != null) { - graph.push([childNode.resolvedPackage.name, edges]) } + graph.push([currentAlias, edges]) for (const [missingPeer, range] of missingPeers.entries()) { allMissingPeers.set(missingPeer, range) }