Skip to content

Commit

Permalink
fix(@angular/cli): improve error handling of update command
Browse files Browse the repository at this point in the history
This adds extensive option checking and error handling with the goal to catch user errors as early in the process as possible.  Bad option combinations and/or values will result in actionable information and stop the process before network access and processing occurs.
  • Loading branch information
clydin authored and kyliau committed May 15, 2019
1 parent b12118e commit b7fc612
Show file tree
Hide file tree
Showing 6 changed files with 372 additions and 37 deletions.
277 changes: 240 additions & 37 deletions packages/angular/cli/commands/update-impl.ts
Expand Up @@ -5,62 +5,265 @@
* 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 { normalize } from '@angular-devkit/core';
import * as path from 'path';
import * as semver from 'semver';
import { Arguments, Option } from '../models/interface';
import { SchematicCommand } from '../models/schematic-command';
import { findUp } from '../utilities/find-up';
import { getPackageManager } from '../utilities/package-manager';
import {
PackageIdentifier,
PackageManifest,
fetchPackageMetadata,
} from '../utilities/package-metadata';
import { findNodeDependencies, readPackageTree } from '../utilities/package-tree';
import { Schema as UpdateCommandSchema } from './update';

const npa = require('npm-package-arg');

const oldConfigFileNames = [
'.angular-cli.json',
'angular-cli.json',
];

export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
public readonly allowMissingWorkspace = true;

collectionName = '@schematics/update';
schematicName = 'update';

async parseArguments(schematicOptions: string[], schema: Option[]): Promise<Arguments> {
const args = await super.parseArguments(schematicOptions, schema);
const maybeArgsLeftovers = args['--'];

if (maybeArgsLeftovers
&& maybeArgsLeftovers.length == 1
&& maybeArgsLeftovers[0] == '@angular/cli'
&& args.migrateOnly === undefined
&& args.from === undefined) {
// Check for a 1.7 angular-cli.json file.
const oldConfigFileNames = [
normalize('.angular-cli.json'),
normalize('angular-cli.json'),
];
const oldConfigFilePath = findUp(oldConfigFileNames, process.cwd())
|| findUp(oldConfigFileNames, __dirname);

if (oldConfigFilePath) {
args.migrateOnly = true;
args.from = '1.0.0';
async parseArguments(_schematicOptions: string[], _schema: Option[]): Promise<Arguments> {
return {};
}

// tslint:disable-next-line:no-big-function
async run(options: UpdateCommandSchema & Arguments) {
const packages: PackageIdentifier[] = [];
for (const request of options['--'] || []) {
try {
const packageIdentifier: PackageIdentifier = npa(request);

// only registry identifiers are supported
if (!packageIdentifier.registry) {
this.logger.error(`Package '${request}' is not a registry package identifer.`);

return 1;
}

if (packages.some(v => v.name === packageIdentifier.name)) {
this.logger.error(`Duplicate package '${packageIdentifier.name}' specified.`);

return 1;
}

// If next option is used and no specifier supplied, use next tag
if (options.next && !packageIdentifier.rawSpec) {
packageIdentifier.fetchSpec = 'next';
}

packages.push(packageIdentifier);
} catch (e) {
this.logger.error(e.message);

return 1;
}
}

// Move `--` to packages.
if (args.packages == undefined && args['--']) {
args.packages = args['--'];
delete args['--'];
}
if (options.all && packages.length > 0) {
this.logger.error('Cannot specify packages when using the "all" option.');

return args;
}
return 1;
} else if (options.all && options.migrateOnly) {
this.logger.error('Cannot use "all" option with "migrate-only" option.');

return 1;
} else if (!options.migrateOnly && (options.from || options.to)) {
this.logger.error('Can only use "from" or "to" options with "migrate-only" option.');

return 1;
}

async run(options: UpdateCommandSchema & Arguments) {
const packageManager = getPackageManager(this.workspace.root);
this.logger.info(`Using package manager: '${packageManager}'`);

// Special handling for Angular CLI 1.x migrations
if (options.migrateOnly === undefined && options.from === undefined) {
if (!options.all && packages.length === 1 && packages[0].name === '@angular/cli') {
const oldConfigFilePath = findUp(oldConfigFileNames, process.cwd());
if (oldConfigFilePath) {
options.migrateOnly = true;
options.from = '1.0.0';
}
}
}

this.logger.info('Collecting installed dependencies...');

const packageTree = await readPackageTree(this.workspace.root);
const rootDependencies = findNodeDependencies(packageTree);

this.logger.info(`Found ${Object.keys(rootDependencies).length} dependencies.`);

if (options.all || packages.length === 0) {
// Either update all packages or show status
return this.runSchematic({
collectionName: '@schematics/update',
schematicName: 'update',
dryRun: !!options.dryRun,
showNothingDone: false,
additionalOptions: {
force: options.force || false,
next: options.next || false,
packageManager,
packages: options.all ? Object.keys(rootDependencies) : [],
},
});
}

if (options.migrateOnly) {
if (!options.from) {
this.logger.error('"from" option is required when using the "migrate-only" option.');

return 1;
} else if (packages.length !== 1) {
this.logger.error(
'A single package must be specified when using the "migrate-only" option.',
);

return 1;
}

if (options.next) {
this.logger.warn('"next" option has no effect when using "migrate-only" option.');
}

const packageName = packages[0].name;
let packageNode = rootDependencies[packageName];
if (typeof packageNode === 'string') {
this.logger.error('Package found in package.json but is not installed.');

return 1;
} else if (!packageNode) {
// Allow running migrations on transitively installed dependencies
// There can technically be nested multiple versions
// TODO: If multiple, this should find all versions and ask which one to use
const child = packageTree.children.find(c => c.name === packageName);
if (child) {
// A link represents a symlinked package so use the actual in this case
packageNode = child.isLink ? child.target : child;
}

if (!packageNode) {
this.logger.error('Package is not installed.');

return 1;
}
}

const updateMetadata = packageNode.package['ng-update'];
let migrations = updateMetadata && updateMetadata.migrations;
if (migrations === undefined) {
this.logger.error('Package does not provide migrations.');

return 1;
} else if (typeof migrations !== 'string') {
this.logger.error('Package contains a malformed migrations field.');

return 1;
}

// if not non-relative, add package name
if (migrations.startsWith('.') || migrations.startsWith('/')) {
migrations = path.join(packageName, migrations);
}

return this.runSchematic({
collectionName: '@schematics/update',
schematicName: 'migrate',
dryRun: !!options.dryRun,
force: false,
showNothingDone: false,
additionalOptions: {
package: packageName,
collection: migrations,
from: options.from,
to: options.to || packageNode.package.version,
},
});
}

const requests: PackageIdentifier[] = [];

// Validate packages actually are part of the workspace
for (const pkg of packages) {
const node = rootDependencies[pkg.name];
if (!node) {
this.logger.error(`Package '${pkg.name}' is not a dependency.`);

return 1;
}

// If a specific version is requested and matches the installed version, skip.
if (pkg.type === 'version' &&
typeof node === 'object' &&
node.package.version === pkg.fetchSpec) {
this.logger.info(`Package '${pkg.name}' is already at '${pkg.fetchSpec}'.`);
continue;
}

requests.push(pkg);
}

if (requests.length === 0) {
return 0;
}

this.logger.info('Fetching dependency metadata from registry...');
for (const requestIdentifier of requests) {
let metadata;
try {
// Metadata requests are internally cached; multiple requests for same name
// does not result in additional network traffic
metadata = await fetchPackageMetadata(requestIdentifier.name, this.logger);
} catch (e) {
this.logger.error(`Error fetching metadata for '${requestIdentifier.name}': ` + e.message);

return 1;
}

// Try to find a package version based on the user requested package specifier
// registry specifier types are either version, range, or tag
let manifest: PackageManifest | undefined;
if (requestIdentifier.type === 'version') {
manifest = metadata.versions.get(requestIdentifier.fetchSpec);
} else if (requestIdentifier.type === 'range') {
const maxVersion = semver.maxSatisfying(
Array.from(metadata.versions.keys()),
requestIdentifier.fetchSpec,
);
if (maxVersion) {
manifest = metadata.versions.get(maxVersion);
}
} else if (requestIdentifier.type === 'tag') {
manifest = metadata.tags[requestIdentifier.fetchSpec];
}

if (!manifest) {
this.logger.error(
`Package specified by '${requestIdentifier.raw}' does not exist within the registry.`,
);

return 1;
}
}

return this.runSchematic({
collectionName: this.collectionName,
schematicName: this.schematicName,
schematicOptions: options['--'],
collectionName: '@schematics/update',
schematicName: 'update',
dryRun: !!options.dryRun,
force: false,
showNothingDone: false,
additionalOptions: { packageManager },
additionalOptions: {
force: options.force || false,
packageManager,
packages: requests.map(p => p.toString()),
},
});
}
}
42 changes: 42 additions & 0 deletions packages/angular/cli/commands/update.json
Expand Up @@ -13,6 +13,48 @@
"allOf": [
{
"$ref": "./definitions.json#/definitions/base"
},
{
"type": "object",
"properties": {
"packages": {
"description": "The names of package(s) to update.",
"type": "array",
"items": {
"type": "string"
},
"$default": {
"$source": "argv"
}
},
"force": {
"description": "If false, will error out if installed packages are incompatible with the update.",
"default": false,
"type": "boolean"
},
"all": {
"description": "Whether to update all packages in package.json.",
"default": false,
"type": "boolean"
},
"next": {
"description": "Use the largest version, including beta and RCs.",
"default": false,
"type": "boolean"
},
"migrateOnly": {
"description": "Only perform a migration, does not update the installed version.",
"type": "boolean"
},
"from": {
"description": "Version from which to migrate from. Only available with a single package being updated, and only on migration only.",
"type": "string"
},
"to": {
"description": "Version up to which to apply migrations. Only available with a single package being updated, and only on migrations only. Requires from to be specified. Default to the installed version detected.",
"type": "string"
}
}
}
]
}
1 change: 1 addition & 0 deletions packages/angular/cli/package.json
Expand Up @@ -37,6 +37,7 @@
"npm-package-arg": "6.1.0",
"open": "6.3.0",
"pacote": "9.5.0",
"read-package-tree": "5.2.2",
"semver": "6.0.0",
"symbol-observable": "1.2.0",
"universal-analytics": "^0.4.20",
Expand Down
2 changes: 2 additions & 0 deletions packages/angular/cli/utilities/package-metadata.ts
Expand Up @@ -24,6 +24,8 @@ export interface PackageIdentifier {
scope: string | null;
registry: boolean;
raw: string;
fetchSpec: string;
rawSpec: string;
}

export interface PackageManifest {
Expand Down

0 comments on commit b7fc612

Please sign in to comment.