From d133ba68c0e2bc05472712fbe7eecd63a54f142c Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 11 Oct 2019 12:43:01 +0200 Subject: [PATCH] feat(@angular/cli): add support for ng-add packages that should not be saved as `dependencies` With this change the CLI offers a way for a package authors to specify if during `ng add` the package should be saved as a `dependencies`, `devDependencies` or not saved at all. Such config needs to be specified in `package.json` Example: ```json "ng-add": { "save": false } ``` Possible values are; - false - Don't add the package to `package.json` - true - Add the package to the `dependencies` - `dependencies` - Add the package to the `dependencies` - `devDependencies` - Add the package to the `devDependencies` Closes #12003 , closes #15764 and closes #13237 --- packages/angular/cli/commands/add-impl.ts | 46 +++++++++------- packages/angular/cli/tasks/npm-install.ts | 9 +++- packages/angular/cli/tasks/npm-uninstall.ts | 53 +++++++++++++++++++ .../angular/cli/utilities/package-metadata.ts | 7 ++- .../angular/cli/utilities/package-tree.ts | 7 +++ packages/angular/pwa/package.json | 3 ++ .../e2e/tests/commands/add/add-pwa.ts | 20 +++++++ 7 files changed, 123 insertions(+), 22 deletions(-) create mode 100644 packages/angular/cli/tasks/npm-uninstall.ts create mode 100644 tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts diff --git a/packages/angular/cli/commands/add-impl.ts b/packages/angular/cli/commands/add-impl.ts index 790a8618115d..9e0e213eabe4 100644 --- a/packages/angular/cli/commands/add-impl.ts +++ b/packages/angular/cli/commands/add-impl.ts @@ -13,9 +13,11 @@ import { isPackageNameSafeForAnalytics } from '../models/analytics'; import { Arguments } from '../models/interface'; import { RunSchematicOptions, SchematicCommand } from '../models/schematic-command'; import npmInstall from '../tasks/npm-install'; +import npmUninstall from '../tasks/npm-uninstall'; import { colors } from '../utilities/color'; import { getPackageManager } from '../utilities/package-manager'; import { + NgAddSaveDepedency, PackageManifest, fetchPackageManifest, fetchPackageMetadata, @@ -113,31 +115,39 @@ export class AddCommand extends SchematicCommand { } let collectionName = packageIdentifier.name; - if (!packageIdentifier.registry) { - try { - const manifest = await fetchPackageManifest(packageIdentifier, this.logger, { - registry: options.registry, - verbose: options.verbose, - usingYarn, - }); + let dependencyType: NgAddSaveDepedency | undefined; - collectionName = manifest.name; + try { + const manifest = await fetchPackageManifest(packageIdentifier, this.logger, { + registry: options.registry, + verbose: options.verbose, + usingYarn, + }); - if (await this.hasMismatchedPeer(manifest)) { - this.logger.warn( - 'Package has unmet peer dependencies. Adding the package may not succeed.', - ); - } - } catch (e) { - this.logger.error('Unable to fetch package manifest: ' + e.message); + dependencyType = manifest['ng-add'] && manifest['ng-add'].save; + collectionName = manifest.name; - return 1; + if (await this.hasMismatchedPeer(manifest)) { + this.logger.warn( + 'Package has unmet peer dependencies. Adding the package may not succeed.', + ); } + } catch (e) { + this.logger.error('Unable to fetch package manifest: ' + e.message); + + return 1; } - await npmInstall(packageIdentifier.raw, this.logger, packageManager, this.workspace.root); + await npmInstall(packageIdentifier.raw, this.logger, packageManager, dependencyType); + + const schematicResult = await this.executeSchematic(collectionName, options['--']); + + if (dependencyType === false) { + // Uninstall the package if it was not meant to be retained. + return npmUninstall(packageIdentifier.raw, this.logger, packageManager); + } - return this.executeSchematic(collectionName, options['--']); + return schematicResult; } async reportAnalytics( diff --git a/packages/angular/cli/tasks/npm-install.ts b/packages/angular/cli/tasks/npm-install.ts index f60fce0f8991..33deaabd3964 100644 --- a/packages/angular/cli/tasks/npm-install.ts +++ b/packages/angular/cli/tasks/npm-install.ts @@ -9,13 +9,13 @@ import { logging } from '@angular-devkit/core'; import { spawn } from 'child_process'; import { colors } from '../utilities/color'; +import { NgAddSaveDepedency } from '../utilities/package-metadata'; export default async function( packageName: string, logger: logging.Logger, packageManager: string, - projectRoot: string, - save = true, + save: NgAddSaveDepedency = true, ) { const installArgs: string[] = []; switch (packageManager) { @@ -42,9 +42,14 @@ export default async function( } if (!save) { + // IMP: yarn doesn't have a no-save option installArgs.push('--no-save'); } + if (save === 'devDependencies') { + installArgs.push(packageManager === 'yarn' ? '--dev' : '--save-dev'); + } + installArgs.push('--quiet'); await new Promise((resolve, reject) => { diff --git a/packages/angular/cli/tasks/npm-uninstall.ts b/packages/angular/cli/tasks/npm-uninstall.ts new file mode 100644 index 000000000000..a30160195a0d --- /dev/null +++ b/packages/angular/cli/tasks/npm-uninstall.ts @@ -0,0 +1,53 @@ +/** + * @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 { logging } from '@angular-devkit/core'; +import { spawn } from 'child_process'; +import { colors } from '../utilities/color'; + +export default async function( + packageName: string, + logger: logging.Logger, + packageManager: string, +) { + const installArgs: string[] = []; + switch (packageManager) { + case 'cnpm': + case 'pnpm': + case 'npm': + installArgs.push('uninstall'); + break; + + case 'yarn': + installArgs.push('remove'); + break; + + default: + packageManager = 'npm'; + installArgs.push('uninstall'); + break; + } + + installArgs.push(packageName, '--quiet'); + + logger.info(colors.green(`Uninstalling packages for tooling via ${packageManager}.`)); + + await new Promise((resolve, reject) => { + spawn(packageManager, installArgs, { stdio: 'inherit', shell: true }).on( + 'close', + (code: number) => { + if (code === 0) { + logger.info(colors.green(`Uninstalling packages for tooling via ${packageManager}.`)); + resolve(); + } else { + reject('Package uninstallation failed, see above.'); + } + }, + ); + }); +} diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index 0be552aa09a9..303468da908d 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -18,6 +18,8 @@ export interface PackageDependencies { [dependency: string]: string; } +export type NgAddSaveDepedency = 'dependencies' | 'devDependencies' | boolean; + export interface PackageIdentifier { type: 'git' | 'tag' | 'version' | 'range' | 'file' | 'directory' | 'remote'; name: string; @@ -39,8 +41,9 @@ export interface PackageManifest { devDependencies: PackageDependencies; peerDependencies: PackageDependencies; optionalDependencies: PackageDependencies; - - 'ng-add'?: {}; + 'ng-add'?: { + save?: NgAddSaveDepedency; + }; 'ng-update'?: { migrations: string; packageGroup: { [name: string]: string }; diff --git a/packages/angular/cli/utilities/package-tree.ts b/packages/angular/cli/utilities/package-tree.ts index afff17122046..48af91ceadab 100644 --- a/packages/angular/cli/utilities/package-tree.ts +++ b/packages/angular/cli/utilities/package-tree.ts @@ -5,6 +5,10 @@ * 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 { NgAddSaveDepedency } from './package-metadata'; + export interface PackageTreeNodeBase { name: string; path: string; @@ -22,6 +26,9 @@ export interface PackageTreeNodeBase { 'ng-update'?: { migrations?: string; }; + 'ng-add'?: { + save?: NgAddSaveDepedency; + }; }; parent?: PackageTreeNode; children: PackageTreeNode[]; diff --git a/packages/angular/pwa/package.json b/packages/angular/pwa/package.json index a9ef0a0996b6..7ccefc72d2be 100644 --- a/packages/angular/pwa/package.json +++ b/packages/angular/pwa/package.json @@ -9,6 +9,9 @@ "schematics" ], "schematics": "./collection.json", + "ng-add": { + "save": false + }, "dependencies": { "@angular-devkit/core": "0.0.0", "@angular-devkit/schematics": "0.0.0", diff --git a/tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts b/tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts new file mode 100644 index 000000000000..330297ec1866 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts @@ -0,0 +1,20 @@ +import { join } from 'path'; +import { expectFileToExist, expectFileToMatch, rimraf, readFile } from '../../../utils/fs'; +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 expectFileToExist(join(process.cwd(), 'src/manifest.webmanifest')); + + // Angular PWA doesn't install as a dependency + const { dependencies, devDependencies } = JSON.parse(await readFile(join(process.cwd(), 'package.json'))); + const hasPWADep = Object.keys({ ...dependencies, ...devDependencies }) + .some(d => d === '@angular/pwa'); + if (hasPWADep) { + throw new Error(`Expected 'package.json' not to contain a dependency on '@angular/pwa'.`); + } +}