Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(@angular/cli): add support for ng-add packages that should not b… #15815

Merged
merged 2 commits into from
Oct 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 28 additions & 18 deletions packages/angular/cli/commands/add-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -113,31 +115,39 @@ export class AddCommand extends SchematicCommand<AddCommandSchema> {
}

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(
Expand Down
9 changes: 7 additions & 2 deletions packages/angular/cli/tasks/npm-install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) => {
Expand Down
53 changes: 53 additions & 0 deletions packages/angular/cli/tasks/npm-uninstall.ts
Original file line number Diff line number Diff line change
@@ -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.');
}
},
);
});
}
7 changes: 5 additions & 2 deletions packages/angular/cli/utilities/package-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 };
Expand Down
7 changes: 7 additions & 0 deletions packages/angular/cli/utilities/package-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -22,6 +26,9 @@ export interface PackageTreeNodeBase {
'ng-update'?: {
migrations?: string;
};
'ng-add'?: {
save?: NgAddSaveDepedency;
};
};
parent?: PackageTreeNode;
children: PackageTreeNode[];
Expand Down
3 changes: 3 additions & 0 deletions packages/angular/pwa/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { latestVersions } from '../../utility/latest-versions';
export function updateDependencies() {
return (host: Tree) => {
const dependenciesToUpdate: Record<string, string> = {
'@angular/pwa': latestVersions.AngularPWA,
'@angular/pwa': '^0.803.9',
'@angular-devkit/build-angular': latestVersions.DevkitBuildAngular,
'@angular-devkit/build-ng-packagr': latestVersions.DevkitBuildNgPackagr,
'@angular-devkit/build-webpack': latestVersions.DevkitBuildWebpack,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@
* found in the LICENSE file at https://angular.io/license
*/
import { Rule } from '@angular-devkit/schematics';
import { addPackageJsonDependency, getPackageJsonDependency } from '../../utility/dependencies';
import {
addPackageJsonDependency,
getPackageJsonDependency,
removePackageJsonDependency,
} from '../../utility/dependencies';
import { latestVersions } from '../../utility/latest-versions';

export function updateDependencies(): Rule {
return host => {
const dependenciesToUpdate: Record<string, string> = {
'@angular/pwa': latestVersions.AngularPWA,
'@angular-devkit/build-angular': latestVersions.DevkitBuildAngular,
'@angular-devkit/build-ng-packagr': latestVersions.DevkitBuildNgPackagr,
'@angular-devkit/build-webpack': latestVersions.DevkitBuildWebpack,
Expand All @@ -35,5 +38,8 @@ export function updateDependencies(): Rule {
overwrite: true,
});
}

// `@angular/pwa` package is only needed when running `ng-add`.
removePackageJsonDependency(host, '@angular/pwa');
};
}
1 change: 0 additions & 1 deletion packages/schematics/angular/utility/latest-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const latestVersions = {
DevkitBuildAngular: '~0.900.0-next.9',
DevkitBuildNgPackagr: '~0.900.0-next.9',
DevkitBuildWebpack: '~0.900.0-next.9',
AngularPWA: '~0.900.0-next.9',

ngPackagr: '^5.5.1',
};
20 changes: 20 additions & 0 deletions tests/legacy-cli/e2e/tests/commands/add/add-pwa.ts
Original file line number Diff line number Diff line change
@@ -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'.`);
}
}