Skip to content

Commit

Permalink
feat(@angular/cli): confirm ng add action before installation
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
The `ng add` command will now ask the user to confirm the package and version prior to installing and executing an uninstalled package.
This new behavior allows a user to abort the action if the version selected is not appropriate or if a typo occurred on the command line and an incorrect package would be installed.
A `--skip-confirmation` option has been added to skip the prompt and directly install and execute the package. This option is useful in CI and non-TTY scenarios such as automated scripts.
  • Loading branch information
clydin authored and alan-agius4 committed Mar 11, 2021
1 parent 3c2f583 commit 985dc1a
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 20 deletions.
24 changes: 24 additions & 0 deletions packages/angular/cli/commands/add-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import {
fetchPackageManifest,
fetchPackageMetadata,
} from '../utilities/package-metadata';
import { askConfirmation } from '../utilities/prompt';
import { Spinner } from '../utilities/spinner';
import { isTTY } from '../utilities/tty';
import { Schema as AddCommandSchema } from './add';

const npa = require('npm-package-arg');
Expand Down Expand Up @@ -179,6 +181,28 @@ export class AddCommand extends SchematicCommand<AddCommandSchema> {
return 1;
}

if (!options.skipConfirmation) {
const confirmationResponse = await askConfirmation(
`\nThe package ${colors.blue(packageIdentifier.raw)} will be installed and executed.\n` +
'Would you like to proceed?',
true,
false,
);

if (!confirmationResponse) {
if (!isTTY) {
this.logger.error(
'No terminal detected. ' +
`'--skip-confirmation' can be used to bypass installation confirmation. ` +
`Ensure package name is correct prior to '--skip-confirmation' option usage.`,
);
}
this.logger.error('Command aborted.');

return 1;
}
}

try {
spinner.start('Installing package...');
if (savePackage === false) {
Expand Down
5 changes: 5 additions & 0 deletions packages/angular/cli/commands/add.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@
"description": "Display additional details about internal operations during execution.",
"type": "boolean",
"default": false
},
"skipConfirmation": {
"description": "Skip asking a confirmation prompt before installing and executing the package. Ensure package name is correct prior to using this option.",
"type": "boolean",
"default": false
}
},
"required": [
Expand Down
31 changes: 31 additions & 0 deletions packages/angular/cli/utilities/prompt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as inquirer from 'inquirer';
import { isTTY } from './tty';

export async function askConfirmation(
message: string,
defaultResponse: boolean,
noTTYResponse?: boolean,
): Promise<boolean> {
if (!isTTY()) {
return noTTYResponse ?? defaultResponse;
}

const question: inquirer.Question = {
type: 'confirm',
name: 'confirmation',
prefix: '',
message,
default: defaultResponse,
};

const answers = await inquirer.prompt([question]);

return answers['confirmation'];
}
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/build/material.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const snapshots = require('../../ng-snapshot/package.json');

export default async function () {
const tag = await isPrereleaseCli() ? '@next' : '';
await ng('add', `@angular/material${tag}`);
await ng('add', `@angular/material${tag}`, '--skip-confirmation');

const isSnapshotBuild = getGlobalVariable('argv')['ng-snapshots'];
if (isSnapshotBuild) {
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/commands/add/add-material.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ export default async function () {
const tag = await isPrereleaseCli() ? '@next' : '';

try {
await ng('add', `@angular/material${tag}`, '--unknown');
await ng('add', `@angular/material${tag}`, '--unknown', '--skip-confirmation');
} catch (error) {
if (!(error.message && error.message.includes(`Unknown option: '--unknown'`))) {
throw error;
}
}

await ng('add', `@angular/material${tag}`, '--theme', 'custom', '--verbose');
await ng('add', `@angular/material${tag}`, '--theme', 'custom', '--verbose', '--skip-confirmation');
await expectFileToMatch('package.json', /@angular\/material/);

// Clean up existing cdk package
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/add-pwa-yarn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default async function () {

// set yarn as package manager
await ng('config', 'cli.packageManager', 'yarn');
await ng('add', '@angular/pwa');
await ng('add', '@angular/pwa', '--skip-confirmation');
await expectFileToExist(join(process.cwd(), 'src/manifest.webmanifest'));

// Angular PWA doesn't install as a dependency
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ng } from '../../../utils/process';
export default async function () {
// forcibly remove in case another test doesn't clean itself up
await rimraf('node_modules/@angular/pwa');
await ng('add', '@angular/pwa');
await ng('add', '@angular/pwa', '--skip-confirmation');
await expectFileToExist(join(process.cwd(), 'src/manifest.webmanifest'));

// Angular PWA doesn't install as a dependency
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/add-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ng } from '../../../utils/process';


export default async function () {
await ng('add', '@angular-devkit-tests/ng-add-simple@^1.0.0');
await ng('add', '@angular-devkit-tests/ng-add-simple@^1.0.0', '--skip-confirmation');
await expectFileToMatch('package.json', /\/ng-add-simple.*\^1\.0\.0/);
await expectFileToExist('ng-add-test');
await rimraf('node_modules/@angular-devkit-tests/ng-add-simple');
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ng } from '../../../utils/process';


export default async function () {
await ng('add', '@angular-devkit-tests/ng-add-simple');
await ng('add', '@angular-devkit-tests/ng-add-simple', '--skip-confirmation');
await expectFileToMatch('package.json', /@angular-devkit-tests\/ng-add-simple/);
await expectFileToExist('ng-add-test');
await rimraf('node_modules/@angular-devkit-tests/ng-add-simple');
Expand Down
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import { ng } from '../../../utils/process';


export default async function () {
await ng('add', assetDir('add-collection'), '--name=blah');
await ng('add', assetDir('add-collection'), '--name=blah', '--skip-confirmation');
await expectFileToExist('blah');
}
2 changes: 1 addition & 1 deletion tests/legacy-cli/e2e/tests/commands/add/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import { ng } from '../../../utils/process';


export default async function () {
await ng('add', assetDir('add-collection.tgz'), '--name=blah');
await ng('add', assetDir('add-collection.tgz'), '--name=blah', '--skip-confirmation');
await expectFileToExist('blah');
}
6 changes: 3 additions & 3 deletions tests/legacy-cli/e2e/tests/commands/add/peer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import { ng } from '../../../utils/process';
const warning = 'Adding the package may not succeed.';

export default async function () {
const { stderr: bad } = await ng('add', assetDir('add-collection-peer-bad'));
const { stderr: bad } = await ng('add', assetDir('add-collection-peer-bad'), '--skip-confirmation');
if (!bad.includes(warning)) {
throw new Error('peer warning not shown on bad package');
}

const { stderr: base } = await ng('add', assetDir('add-collection'));
const { stderr: base } = await ng('add', assetDir('add-collection'), '--skip-confirmation');
if (base.includes(warning)) {
throw new Error('peer warning shown on base package');
}

const { stderr: good } = await ng('add', assetDir('add-collection-peer-good'));
const { stderr: good } = await ng('add', assetDir('add-collection-peer-good'), '--skip-confirmation');
if (good.includes(warning)) {
throw new Error('peer warning shown on good package');
}
Expand Down
4 changes: 2 additions & 2 deletions tests/legacy-cli/e2e/tests/commands/add/registry-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ export default async function () {
process.env['NPM_CONFIG_REGISTRY'] = undefined;

try {
await expectToFail(() => ng('add', '@angular/pwa'));
await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation'));

await ng('add', `--registry=${testRegistry}`, '@angular/pwa');
await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation');
await expectFileToExist('src/manifest.webmanifest');
} finally {
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
Expand Down
10 changes: 5 additions & 5 deletions tests/legacy-cli/e2e/tests/commands/add/version-specifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,25 @@ export default async function () {

const tag = await isPrereleaseCli() ? '@next' : '';

await ng('add', `@angular/localize${tag}`);
await ng('add', `@angular/localize${tag}`, '--skip-confirmation');
await expectFileToMatch('package.json', /@angular\/localize/);

const output1 = await ng('add', '@angular/localize');
const output1 = await ng('add', '@angular/localize', '--skip-confirmation');
if (!output1.stdout.includes('Skipping installation: Package already installed')) {
throw new Error('Installation was not skipped');
}

const output2 = await ng('add', '@angular/localize@latest');
const output2 = await ng('add', '@angular/localize@latest', '--skip-confirmation');
if (output2.stdout.includes('Skipping installation: Package already installed')) {
throw new Error('Installation should not have been skipped');
}

const output3 = await ng('add', '@angular/[email protected]');
const output3 = await ng('add', '@angular/[email protected]', '--skip-confirmation');
if (output3.stdout.includes('Skipping installation: Package already installed')) {
throw new Error('Installation should not have been skipped');
}

const output4 = await ng('add', '@angular/localize@10');
const output4 = await ng('add', '@angular/localize@10', '--skip-confirmation');
if (!output4.stdout.includes('Skipping installation: Package already installed')) {
throw new Error('Installation was not skipped');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default async function () {
await installPackage('@schematics/angular@7');

const tag = (await isPrereleaseCli()) ? '@next' : '';
await ng('add', `@angular/material${tag}`);
await ng('add', `@angular/material${tag}`, '--skip-confirmation');
await expectFileToMatch('package.json', /@angular\/material/);

// Clean up existing cdk package
Expand Down

0 comments on commit 985dc1a

Please sign in to comment.