Skip to content

Commit

Permalink
fix(@angular/cli): clean node modules directory prior to updating
Browse files Browse the repository at this point in the history
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 6926b37)
  • Loading branch information
clydin authored and alan-agius4 committed May 17, 2021
1 parent 4ac556b commit c0694ca
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 4 deletions.
22 changes: 21 additions & 1 deletion packages/angular/cli/commands/update-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -655,6 +655,26 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
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(', ')}`,
Expand Down
3 changes: 0 additions & 3 deletions packages/angular/cli/src/commands/update/schematic/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: {}[] = [];
Expand Down
47 changes: 47 additions & 0 deletions packages/angular/cli/utilities/install-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -193,6 +238,7 @@ function getPackageManagerArguments(packageManager: PackageManager): PackageMana
silent: '--silent',
saveDev: '--save-dev',
install: 'add',
installAll: 'install',
prefix: '--prefix',
noLockfile: '--no-lockfile',
};
Expand All @@ -201,6 +247,7 @@ function getPackageManagerArguments(packageManager: PackageManager): PackageMana
silent: '--quiet',
saveDev: '--save-dev',
install: 'install',
installAll: 'install',
prefix: '--prefix',
noLockfile: '--no-package-lock',
};
Expand Down

0 comments on commit c0694ca

Please sign in to comment.