fix(@angular/cli): correctly account for linked packages in update

Fixes #15511
Fixes #15294

Author: clydin

packages/angular/cli/commands/update-impl.ts

@@ -20,13 +20,10 @@ import { getPackageManager } from '../utilities/package-manager';
 import {
   PackageIdentifier,
   PackageManifest,
+  PackageMetadata,
   fetchPackageMetadata,
 } from '../utilities/package-metadata';
-import {
-  PackageTreeActual,
-  findNodeDependencies,
-  readPackageTree,
-} from '../utilities/package-tree';
+import { PackageTreeNode, findNodeDependencies, readPackageTree } from '../utilities/package-tree';
 import { Schema as UpdateCommandSchema } from './update';
 
 const npa = require('npm-package-arg');
@@ -311,26 +308,26 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
       }
 
       const packageName = packages[0].name;
-      let packageNode = rootDependencies[packageName];
-      if (typeof packageNode === 'string') {
+      const packageDependency = rootDependencies[packageName];
+      let packageNode = packageDependency && packageDependency.node;
+      if (packageDependency && !packageNode) {
         this.logger.error('Package found in package.json but is not installed.');
 
         return 1;
-      } else if (!packageNode) {
+      } else if (!packageDependency) {
         // 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;
+          packageNode = child;
         }
+      }
 
-        if (!packageNode) {
-          this.logger.error('Package is not installed.');
+      if (!packageNode) {
+        this.logger.error('Package is not installed.');
 
-          return 1;
-        }
+        return 1;
       }
 
       const updateMetadata = packageNode.package['ng-update'];
@@ -399,12 +396,12 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
 
     const requests: {
       identifier: PackageIdentifier;
-      node: PackageTreeActual | string;
+      node: PackageTreeNode | string;
     }[] = [];
 
     // Validate packages actually are part of the workspace
     for (const pkg of packages) {
-      const node = rootDependencies[pkg.name];
+      const node = rootDependencies[pkg.name] && rootDependencies[pkg.name].node;
       if (!node) {
         this.logger.error(`Package '${pkg.name}' is not a dependency.`);
 

packages/angular/cli/utilities/package-metadata.ts

@@ -40,12 +40,10 @@ export interface PackageManifest {
   peerDependencies: PackageDependencies;
   optionalDependencies: PackageDependencies;
 
-  'ng-add'?: {
-
-  };
+  'ng-add'?: {};
   'ng-update'?: {
-    migrations: string,
-    packageGroup: { [name: string]: string },
+    migrations: string;
+    packageGroup: { [name: string]: string };
   };
 }
 
@@ -61,12 +59,12 @@ function ensureNpmrc(logger: logging.LoggerApi, usingYarn: boolean, verbose: boo
   if (!npmrc) {
     try {
       npmrc = readOptions(logger, false, verbose);
-    } catch { }
+    } catch {}
 
     if (usingYarn) {
       try {
         npmrc = { ...npmrc, ...readOptions(logger, true, verbose) };
-      } catch { }
+      } catch {}
     }
   }
 }
@@ -95,9 +93,7 @@ function readOptions(
     (!yarn && process.env.NPM_CONFIG_USERCONFIG) || path.join(homedir(), dotFilename),
   ];
 
-  const projectConfigLocations: string[] = [
-    path.join(cwd, dotFilename),
-  ];
+  const projectConfigLocations: string[] = [path.join(cwd, dotFilename)];
   const root = path.parse(cwd).root;
   for (let curDir = path.dirname(cwd); curDir && curDir !== root; curDir = path.dirname(curDir)) {
     projectConfigLocations.unshift(path.join(curDir, dotFilename));
@@ -125,7 +121,7 @@ function readOptions(
         delete options.cafile;
         try {
           options.ca = readFileSync(cafile, 'utf8').replace(/\r?\n/, '\\n');
-        } catch { }
+        } catch {}
       }
     } else if (showPotentials) {
       logger.info(`Trying '${location}'...not found.`);
@@ -151,7 +147,7 @@ function normalizeManifest(rawManifest: {}): PackageManifest {
     peerDependencies: {},
     optionalDependencies: {},
     // tslint:disable-next-line:no-any
-    ...rawManifest as any,
+    ...(rawManifest as any),
   };
 }
 
@@ -173,14 +169,11 @@ export async function fetchPackageMetadata(
 
   ensureNpmrc(logger, usingYarn, verbose);
 
-  const response = await pacote.packument(
-    name,
-    {
-      'full-metadata': true,
-      ...npmrc,
-      ...(registry ? { registry } : {}),
-    },
-  );
+  const response = await pacote.packument(name, {
+    'full-metadata': true,
+    ...npmrc,
+    ...(registry ? { registry } : {}),
+  });
 
   // Normalize the response
   const metadata: PackageMetadata = {
@@ -227,14 +220,11 @@ export async function fetchPackageManifest(
 
   ensureNpmrc(logger, usingYarn, verbose);
 
-  const response = await pacote.manifest(
-    name,
-    {
-      'full-metadata': true,
-      ...npmrc,
-      ...(registry ? { registry } : {}),
-    },
-  );
+  const response = await pacote.manifest(name, {
+    'full-metadata': true,
+    ...npmrc,
+    ...(registry ? { registry } : {}),
+  });
 
   return normalizeManifest(response);
 }

packages/angular/cli/utilities/package-tree.ts

@@ -18,21 +18,21 @@ export interface PackageTreeNodeBase {
     dependencies?: Record<string, string>;
     devDependencies?: Record<string, string>;
     peerDependencies?: Record<string, string>;
+    optionalDependencies?: Record<string, string>;
     'ng-update'?: {
       migrations?: string;
     };
   };
+  parent?: PackageTreeNode;
   children: PackageTreeNode[];
 }
 
 export interface PackageTreeActual extends PackageTreeNodeBase {
   isLink: false;
-  parent?: PackageTreeActual;
 }
 
 export interface PackageTreeLink extends PackageTreeNodeBase {
   isLink: true;
-  parent: null;
   target: PackageTreeActual;
 }
 
@@ -52,25 +52,35 @@ export function readPackageTree(path: string): Promise<PackageTreeNode> {
   });
 }
 
-export function findNodeDependencies(root: PackageTreeNode, node = root) {
-  const actual = node.isLink ? node.target : node;
+export interface NodeDependency {
+  version: string;
+  node?: PackageTreeNode;
+}
 
+export function findNodeDependencies(node: PackageTreeNode) {
   const rawDeps: Record<string, string> = {
-    ...actual.package.dependencies,
-    ...actual.package.devDependencies,
-    ...actual.package.peerDependencies,
+    ...node.package.dependencies,
+    ...node.package.devDependencies,
+    ...node.package.peerDependencies,
+    ...node.package.optionalDependencies,
   };
 
   return Object.entries(rawDeps).reduce(
     (deps, [name, version]) => {
-      const depNode = root.children.find(child => {
-        return child.name === name && !child.isLink && child.parent === node;
-      }) as PackageTreeActual | undefined;
+      let dependencyNode;
+      let parent: PackageTreeNode | undefined | null = node;
+      while (!dependencyNode && parent) {
+        dependencyNode = parent.children.find(child => child.name === name);
+        parent = parent.parent;
+      }
 
-      deps[name] = depNode || version;
+      deps[name] = {
+        node: dependencyNode,
+        version,
+      };
 
       return deps;
     },
-    {} as Record<string, string | PackageTreeActual | undefined>,
+    Object.create(null) as Record<string, NodeDependency>,
   );
 }