From 84f440419083c84ac80911752114943c1a2b2595 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 9 Oct 2022 04:29:50 +0300 Subject: [PATCH] fix: don't crash when auto-install-peers is true in a workspace (#5467) close #5454 --- .changeset/eight-waves-type.md | 6 + .../core/test/install/autoInstallPeers.ts | 141 ++++++++++++++++++ .../src/resolveDependencies.ts | 23 ++- 3 files changed, 166 insertions(+), 4 deletions(-) create mode 100644 .changeset/eight-waves-type.md diff --git a/.changeset/eight-waves-type.md b/.changeset/eight-waves-type.md new file mode 100644 index 00000000000..ebefef9fe60 --- /dev/null +++ b/.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). diff --git a/packages/core/test/install/autoInstallPeers.ts b/packages/core/test/install/autoInstallPeers.ts index 158655696ce..5b293e2c362 100644 --- a/packages/core/test/install/autoInstallPeers.ts +++ b/packages/core/test/install/autoInstallPeers.ts @@ -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', diff --git a/packages/resolve-dependencies/src/resolveDependencies.ts b/packages/resolve-dependencies/src/resolveDependencies.ts index f3bf7b6fd53..be1e17e235b 100644 --- a/packages/resolve-dependencies/src/resolveDependencies.ts +++ b/packages/resolve-dependencies/src/resolveDependencies.ts @@ -169,6 +169,7 @@ interface MissingPeersOfChildren { resolve: (missingPeers: MissingPeers) => void reject: (err: Error) => void get: () => Promise + resolved?: boolean } export type PkgAddress = { @@ -228,6 +229,7 @@ export interface ResolvedPackage { os?: string[] libc?: string[] } + parentImporterIds: Set } type ParentPkg = Pick @@ -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) }, } @@ -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 ( @@ -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! @@ -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 @@ -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, } } @@ -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, @@ -1354,6 +1367,7 @@ function getResolvedPackage ( depPath: string force: boolean hasBin: boolean + parentImporterId: string patchFile?: PatchFile pkg: PackageManifest pkgResponse: PackageResponse @@ -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,