fix(@angular/cli): improve error handling of update command

clydin · Keen Yee Liau · commit e406f00909a7 · 2019-05-15T14:33:42.000-07:00
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.
diff --git a/packages/angular/cli/commands/update-impl.ts b/packages/angular/cli/commands/update-impl.ts
@@ -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()),
+      },
     });
   }
 }
diff --git a/packages/angular/cli/commands/update.json b/packages/angular/cli/commands/update.json
@@ -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"
+        }
+      }
     }
   ]
 }
diff --git a/packages/angular/cli/package.json b/packages/angular/cli/package.json
@@ -37,6 +37,7 @@
     "npm-package-arg": "6.1.0",
     "open": "6.2.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",
diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts
@@ -24,6 +24,8 @@ export interface PackageIdentifier {
   scope: string | null;
   registry: boolean;
   raw: string;
+  fetchSpec: string;
+  rawSpec: string;
 }
 
 export interface PackageManifest {
diff --git a/packages/angular/cli/utilities/package-tree.ts b/packages/angular/cli/utilities/package-tree.ts
diff --git a/yarn.lock b/yarn.lock

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,8 @@ export interface PackageIdentifier {`
`24`	`24`	`scope: string \| null;`
`25`	`25`	`registry: boolean;`
`26`	`26`	`raw: string;`
	`27`	`+ fetchSpec: string;`
	`28`	`+ rawSpec: string;`
`27`	`29`	`}`
`28`	`30`
`29`	`31`	`export interface PackageManifest {`