From c0694cad3c0ad84847eaa89a2143a0d81458b12b Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 14 May 2021 16:18:05 -0400 Subject: [PATCH] fix(@angular/cli): clean node modules directory prior to updating Prior to performing the initial updated package installation during the `ng update` process, the workspace node modules directory will be removed. This cleaning increases the guarantees that the package manager will hoist packages into the correct locations and avoid peer dependency inconsistencies. (cherry picked from commit 6926b37c0c8093dd5427e80cb8da9e41a15c6cc0) --- packages/angular/cli/commands/update-impl.ts | 22 ++++++++- .../src/commands/update/schematic/index.ts | 3 -- .../angular/cli/utilities/install-package.ts | 47 +++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/packages/angular/cli/commands/update-impl.ts b/packages/angular/cli/commands/update-impl.ts index 0c2965440acb..2c9cea0eb2c8 100644 --- a/packages/angular/cli/commands/update-impl.ts +++ b/packages/angular/cli/commands/update-impl.ts @@ -17,7 +17,7 @@ import { Command } from '../models/command'; import { Arguments } from '../models/interface'; import { SchematicEngineHost } from '../models/schematic-engine-host'; import { colors } from '../utilities/color'; -import { runTempPackageBin } from '../utilities/install-package'; +import { installAllPackages, runTempPackageBin } from '../utilities/install-package'; import { writeErrorToLogFile } from '../utilities/log-file'; import { ensureCompatibleNpm, getPackageManager } from '../utilities/package-manager'; import { @@ -655,6 +655,26 @@ export class UpdateCommand extends Command { packages: packagesToUpdate, }); + if (success) { + try { + // Remove existing node modules directory to provide a stronger guarantee that packages + // will be hoisted into the correct locations. + await fs.promises.rmdir(path.join(this.context.root, 'node_modules'), { + recursive: true, + maxRetries: 3, + }); + } catch {} + + const result = await installAllPackages( + this.packageManager, + options.force ? ['--force'] : [], + this.context.root, + ); + if (result !== 0) { + return result; + } + } + if (success && options.createCommits) { const committed = this.commit( `Angular CLI update for packages - ${packagesToUpdate.join(', ')}`, diff --git a/packages/angular/cli/src/commands/update/schematic/index.ts b/packages/angular/cli/src/commands/update/schematic/index.ts index 7291bea7e2a7..56ba9ee8be47 100644 --- a/packages/angular/cli/src/commands/update/schematic/index.ts +++ b/packages/angular/cli/src/commands/update/schematic/index.ts @@ -8,7 +8,6 @@ import { logging, tags } from '@angular-devkit/core'; import { Rule, SchematicContext, SchematicsException, Tree } from '@angular-devkit/schematics'; -import { NodePackageInstallTask } from '@angular-devkit/schematics/tasks'; import * as npa from 'npm-package-arg'; import * as semver from 'semver'; import { getNpmPackageJson } from './npm'; @@ -310,9 +309,7 @@ function _performUpdate( const newContent = JSON.stringify(packageJson, null, 2); if (packageJsonContent.toString() != newContent || migrateOnly) { if (!migrateOnly) { - // If something changed, also hook up the task. tree.overwrite('/package.json', JSON.stringify(packageJson, null, 2)); - context.addTask(new NodePackageInstallTask()); } const externalMigrations: {}[] = []; diff --git a/packages/angular/cli/utilities/install-package.ts b/packages/angular/cli/utilities/install-package.ts index 9b499e9cd62f..ea1776c3e07a 100644 --- a/packages/angular/cli/utilities/install-package.ts +++ b/packages/angular/cli/utilities/install-package.ts @@ -19,10 +19,55 @@ interface PackageManagerOptions { silent: string; saveDev: string; install: string; + installAll?: string; prefix: string; noLockfile: string; } +export async function installAllPackages( + packageManager: PackageManager = PackageManager.Npm, + extraArgs: string[] = [], + cwd = process.cwd(), +): Promise<1 | 0> { + const packageManagerArgs = getPackageManagerArguments(packageManager); + + const installArgs: string[] = []; + if (packageManagerArgs.installAll) { + installArgs.push(packageManagerArgs.installAll); + } + installArgs.push(packageManagerArgs.silent); + + const spinner = new Spinner(); + spinner.start('Installing packages...'); + + const bufferedOutput: { stream: NodeJS.WriteStream; data: Buffer }[] = []; + + return new Promise((resolve, reject) => { + const childProcess = spawn(packageManager, [...installArgs, ...extraArgs], { + stdio: 'pipe', + shell: true, + cwd, + }).on('close', (code: number) => { + if (code === 0) { + spinner.succeed('Packages successfully installed.'); + resolve(0); + } else { + spinner.stop(); + bufferedOutput.forEach(({ stream, data }) => stream.write(data)); + spinner.fail('Package install failed, see above.'); + reject(1); + } + }); + + childProcess.stdout?.on('data', (data: Buffer) => + bufferedOutput.push({ stream: process.stdout, data: data }), + ); + childProcess.stderr?.on('data', (data: Buffer) => + bufferedOutput.push({ stream: process.stderr, data: data }), + ); + }); +} + export async function installPackage( packageName: string, packageManager: PackageManager = PackageManager.Npm, @@ -193,6 +238,7 @@ function getPackageManagerArguments(packageManager: PackageManager): PackageMana silent: '--silent', saveDev: '--save-dev', install: 'add', + installAll: 'install', prefix: '--prefix', noLockfile: '--no-lockfile', }; @@ -201,6 +247,7 @@ function getPackageManagerArguments(packageManager: PackageManager): PackageMana silent: '--quiet', saveDev: '--save-dev', install: 'install', + installAll: 'install', prefix: '--prefix', noLockfile: '--no-package-lock', };