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.

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

fix: don't crash when auto-install-peers is true in a workspace #5467

Merged
merged 5 commits into from Oct 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/eight-waves-type.md
@@ -0,0 +1,6 @@
---
"@pnpm/resolve-dependencies": patch
"pnpm": patch
---

Don't crash when `auto-install-peers` is set to `true` and installation is done on a workspace with that has the same dependencies in multiple projects [#5454](https://github.com/pnpm/pnpm/issues/5454).
141 changes: 141 additions & 0 deletions packages/core/test/install/autoInstallPeers.ts
Expand Up @@ -348,6 +348,147 @@ test('installation on a package with many complex circular dependencies does not
await addDependenciesToPackage({}, ['webpack@4.46.0'], await testDefaults({ autoInstallPeers: true }))
})

// This test may be removed if autoInstallPeers will become true by default
test('installation on a workspace with many complex circular dependencies does not fail when auto install peers is on', async () => {
prepareEmpty()
await mutateModules([
{
buildIndex: 0,
manifest: {
name: 'project1',
dependencies: {
'@angular/common': '14.2.4',
'@angular/core': '14.2.4',
'@angular/forms': '14.2.4',
'@angular/platform-browser': '14.2.4',
'@angular/platform-browser-dynamic': '14.2.4',
'@angular/router': '14.2.4',
'@capacitor/app': '4.0.1',
'@capacitor/core': '4.3.0',
'@capacitor/haptics': '4.0.1',
'@capacitor/keyboard': '4.0.1',
'@capacitor/status-bar': '4.0.1',
'@ionic/angular': '6.2.9',
'@ionic/core': '6.2.9',
ionicons: '6.0.3',
'ng-particles': '3.3.3',
rxjs: '7.5.7',
tslib: '2.4.0',
tsparticles: '2.3.4',
'tsparticles-engine': '2.3.3',
'tsparticles-interaction-external-attract': '2.3.3',
'tsparticles-interaction-external-bounce': '2.3.3',
'tsparticles-interaction-external-bubble': '2.3.3',
'tsparticles-interaction-external-connect': '2.3.3',
'tsparticles-interaction-external-grab': '2.3.3',
'tsparticles-interaction-external-pause': '2.3.3',
'tsparticles-interaction-external-push': '2.3.3',
'tsparticles-interaction-external-remove': '2.3.3',
'tsparticles-interaction-external-repulse': '2.3.4',
'tsparticles-interaction-external-slow': '2.3.3',
'tsparticles-interaction-external-trail': '2.3.3',
'tsparticles-interaction-particles-attract': '2.3.3',
'tsparticles-interaction-particles-collisions': '2.3.3',
'tsparticles-interaction-particles-links': '2.3.3',
'tsparticles-move-base': '2.3.3',
'tsparticles-move-parallax': '2.3.3',
'tsparticles-particles.js': '2.3.3',
'tsparticles-plugin-absorbers': '2.3.4',
'tsparticles-plugin-emitters': '2.3.4',
'tsparticles-plugin-polygon-mask': '2.3.3',
'tsparticles-shape-circle': '2.3.3',
'tsparticles-shape-image': '2.3.3',
'tsparticles-shape-line': '2.3.3',
'tsparticles-shape-polygon': '2.3.3',
'tsparticles-shape-square': '2.3.3',
'tsparticles-shape-star': '2.3.3',
'tsparticles-shape-text': '2.3.3',
'tsparticles-slim': '2.3.4',
'tsparticles-updater-angle': '2.3.3',
'tsparticles-updater-color': '2.3.3',
'tsparticles-updater-destroy': '2.3.3',
'tsparticles-updater-life': '2.3.3',
'tsparticles-updater-opacity': '2.3.3',
'tsparticles-updater-out-modes': '2.3.3',
'tsparticles-updater-roll': '2.3.3',
'tsparticles-updater-size': '2.3.3',
'tsparticles-updater-stroke-color': '2.3.3',
'tsparticles-updater-tilt': '2.3.3',
'tsparticles-updater-twinkle': '2.3.3',
'tsparticles-updater-wobble': '2.3.3',
'zone.js': '0.11.8',
},
devDependencies: {
'@angular-devkit/build-angular': '14.2.4',
'@angular-eslint/builder': '14.1.2',
'@angular-eslint/eslint-plugin': '14.1.2',
'@angular-eslint/eslint-plugin-template': '14.1.2',
'@angular-eslint/template-parser': '14.1.2',
'@angular/cli': '14.2.4',
'@angular/compiler': '14.2.4',
'@angular/compiler-cli': '14.2.4',
'@angular/language-service': '14.2.4',
'@capacitor/cli': '4.3.0',
'@ionic/angular-toolkit': '7.0.0',
'@types/jasmine': '4.3.0',
'@types/jasminewd2': '2.0.10',
'@types/node': '18.7.23',
'@typescript-eslint/eslint-plugin': '5.38.1',
'@typescript-eslint/parser': '5.38.1',
eslint: '8.24.0',
'eslint-plugin-import': '2.26.0',
'eslint-plugin-jsdoc': '39.3.6',
'eslint-plugin-prefer-arrow': '1.2.3',
'jasmine-core': '4.4.0',
'jasmine-spec-reporter': '7.0.0',
karma: '6.4.1',
'karma-chrome-launcher': '3.1.1',
'karma-coverage': '2.2.0',
'karma-coverage-istanbul-reporter': '3.0.3',
'karma-jasmine': '5.1.0',
'karma-jasmine-html-reporter': '2.0.0',
protractor: '7.0.0',
'ts-node': '10.9.1',
typescript: '4.8.4',
},
},
mutation: 'install',
rootDir: path.resolve('project1'),
},
{
buildIndex: 0,
manifest: {
name: 'project2',
devDependencies: {
'@typescript-eslint/eslint-plugin': '5.38.1',
'@typescript-eslint/parser': '5.38.1',
copyfiles: '2.4.1',
enzyme: '3.11.0',
'enzyme-adapter-preact-pure': '4.0.1',
eslint: '8.24.0',
'eslint-config-preact': '1.3.0',
'eslint-config-prettier': '8.5.0',
'eslint-plugin-react-hooks': '4.6.0',
'identity-obj-proxy': '3.0.0',
'preact-cli': '3.4.1',
prettier: '2.7.1',
'sirv-cli': '2.0.2',
},
dependencies: {
preact: '10.11.0',
'preact-particles': '2.3.3',
'preact-render-to-string': '5.2.4',
'preact-router': '4.1.0',
tsparticles: '2.3.4',
'tsparticles-engine': '2.3.3',
},
},
mutation: 'install',
rootDir: path.resolve('project2'),
},
], await testDefaults({ autoInstallPeers: true, ignoreScripts: true, lockfileOnly: true }))
})

test('do not override the direct dependency with an auto installed peer dependency', async () => {
const includedDeps = new Set([
'@angular-devkit/build-angular',
Expand Down
23 changes: 19 additions & 4 deletions packages/resolve-dependencies/src/resolveDependencies.ts
Expand Up @@ -169,6 +169,7 @@ interface MissingPeersOfChildren {
resolve: (missingPeers: MissingPeers) => void
reject: (err: Error) => void
get: () => Promise<MissingPeers>
resolved?: boolean
}

export type PkgAddress = {
Expand Down Expand Up @@ -228,6 +229,7 @@ export interface ResolvedPackage {
os?: string[]
libc?: string[]
}
parentImporterIds: Set<string>
}

type ParentPkg = Pick<PkgAddress, 'nodeId' | 'installable' | 'depPath' | 'rootDir'>
Expand Down Expand Up @@ -697,7 +699,10 @@ async function resolveDependenciesOfDependency (
resolveDependencyResult,
postponedResolution: async (postponedResolutionOpts) => {
const { missingPeers, resolvedPeers } = await postponedResolution(postponedResolutionOpts)
resolveDependencyResult.missingPeersOfChildren!.resolve(missingPeers)
if (resolveDependencyResult.missingPeersOfChildren) {
resolveDependencyResult.missingPeersOfChildren.resolved = true
resolveDependencyResult.missingPeersOfChildren.resolve(missingPeers)
}
return filterMissingPeers({ missingPeers, resolvedPeers }, postponedResolutionOpts.parentPkgAliases)
},
}
Expand Down Expand Up @@ -1213,6 +1218,8 @@ async function resolveDependency (
const parentIsInstallable = options.parentPkg.installable === undefined || options.parentPkg.installable
const installable = parentIsInstallable && pkgResponse.body.isInstallable !== false
const isNew = !ctx.resolvedPackagesByDepPath[depPath]
const parentImporterId = options.parentPkg.nodeId.substring(0, options.parentPkg.nodeId.indexOf('>', 1) + 1)
let resolveChildren = false

if (isNew) {
if (
Expand Down Expand Up @@ -1251,11 +1258,17 @@ async function resolveDependency (
pkgResponse,
prepare,
wantedDependency,
parentImporterId,
})
} else {
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 && wantedDependency.optional
if (ctx.autoInstallPeers) {
resolveChildren = !ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id].missingPeersOfChildren.resolved &&
!ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.has(parentImporterId)
ctx.resolvedPackagesByDepPath[depPath].parentImporterIds.add(parentImporterId)
}
if (ctx.resolvedPackagesByDepPath[depPath].fetchingFiles == null && pkgResponse.files != null) {
ctx.resolvedPackagesByDepPath[depPath].fetchingFiles = pkgResponse.files
ctx.resolvedPackagesByDepPath[depPath].filesIndexFile = pkgResponse.filesIndexFile!
Expand All @@ -1280,7 +1293,7 @@ async function resolveDependency (
? path.resolve(ctx.lockfileDir, pkgResponse.body.resolution['directory'])
: options.prefix
let missingPeersOfChildren!: MissingPeersOfChildren | undefined
if (!nodeIdContains(options.parentPkg.nodeId, depPath)) {
if (ctx.autoInstallPeers && !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
Expand All @@ -1293,7 +1306,7 @@ async function resolveDependency (
get: pShare(p.promise),
}
ctx.missingPeersOfChildrenByPkgId[pkgResponse.body.id] = {
parentImporterId: options.parentPkg.nodeId.substring(0, options.parentPkg.nodeId.indexOf('>', 1) + 1),
parentImporterId,
missingPeersOfChildren,
}
}
Expand All @@ -1302,7 +1315,7 @@ async function resolveDependency (
alias: wantedDependency.alias || pkg.name,
depIsLinked,
depPath,
isNew,
isNew: isNew || resolveChildren,
nodeId,
normalizedPref: options.currentDepth === 0 ? pkgResponse.body.normalizedPref : undefined,
missingPeersOfChildren,
Expand Down Expand Up @@ -1354,6 +1367,7 @@ function getResolvedPackage (
depPath: string
force: boolean
hasBin: boolean
parentImporterId: string
patchFile?: PatchFile
pkg: PackageManifest
pkgResponse: PackageResponse
Expand All @@ -1377,6 +1391,7 @@ function getResolvedPackage (
os: options.pkg.os,
libc: options.pkg.libc,
},
parentImporterIds: new Set([options.parentImporterId]),
depPath: options.depPath,
dev: options.wantedDependency.dev,
fetchingBundledManifest: options.pkgResponse.bundledManifest,
Expand Down