Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: cli crash with auto-install-peers=true #5394

Merged
merged 2 commits into from Sep 22, 2022
Merged

fix: cli crash with auto-install-peers=true #5394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf2a102
fix: cli crash with auto-install-peers=true
zkochan Sep 22, 2022
e4cfc45
test: auto-install-peers
zkochan Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/twenty-wolves-destroy.md
View file
Open in desktop
@@ -0,0 +1,6 @@
---
"@pnpm/resolve-dependencies": patch
"pnpm": patch
---

Don't crash when auto-install-peers is true and the project has many complex circular dependencies.
6 changes: 6 additions & 0 deletions packages/core/test/install/autoInstallPeers.ts
View file
Open in desktop
Expand Up @@ -341,3 +341,9 @@ test('auto install peer deps in a workspace. test #2', async () => {
},
], await testDefaults({ autoInstallPeers: true }))
})

// This test may be removed if autoInstallPeers will become true by default
test('installation on a package with many complex circular dependencies does not fail when auto install peers is on', async () => {
prepareEmpty()
await addDependenciesToPackage({}, ['webpack@4.46.0'], await testDefaults({ autoInstallPeers: true }))
})
6 changes: 3 additions & 3 deletions packages/resolve-dependencies/src/resolveDependencies.ts
View file
Open in desktop
Expand Up @@ -158,7 +158,7 @@ export interface ResolutionContext {
virtualStoreDir: string
updateMatching?: (pkgName: string) => boolean
workspacePackages?: WorkspacePackages
missingPeersOfChildrenByPkgId: Record<string, { parentNodeId: string, missingPeersOfChildren: MissingPeersOfChildren }>
missingPeersOfChildrenByPkgId: Record<string, { parentImporterId: string, missingPeersOfChildren: MissingPeersOfChildren }>
}

export type MissingPeers = Record<string, string>
Expand Down Expand Up @@ -1278,7 +1278,7 @@ async function resolveDependency (
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)) {
if (!options.parentPkg.nodeId.startsWith(ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].parentImporterId)) {
missingPeersOfChildren = ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren
}
} else {
Expand All @@ -1289,7 +1289,7 @@ async function resolveDependency (
get: pShare(p.promise),
}
ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = {
parentNodeId: options.parentPkg.nodeId,
parentImporterId: options.parentPkg.nodeId.substring(0, options.parentPkg.nodeId.indexOf('>', 1) + 1),
missingPeersOfChildren,
}
}
Expand Down