refactor(@angular/cli): improve update package discovery

Author: andreialecu

package.json

@@ -115,6 +115,7 @@
     "@types/pidusage": "^2.0.1",
     "@types/progress": "^2.0.3",
     "@types/request": "^2.47.1",
+    "@types/resolve": "^1.17.1",
     "@types/rimraf": "^3.0.0",
     "@types/semver": "^7.0.0",
     "@types/universal-analytics": "^0.4.2",

packages/angular/cli/BUILD.bazel

@@ -72,6 +72,7 @@ ts_library(
         "@npm//@types/debug",
         "@npm//@types/inquirer",
         "@npm//@types/node",
+        "@npm//@types/resolve",
         "@npm//@types/rimraf",
         "@npm//@types/semver",
         "@npm//@types/universal-analytics",

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

@@ -27,7 +27,12 @@ import {
   fetchPackageManifest,
   fetchPackageMetadata,
 } from '../utilities/package-metadata';
-import { PackageTreeNode, findNodeDependencies, readPackageTree } from '../utilities/package-tree';
+import {
+  PackageTreeNode,
+  findPackageJson,
+  getProjectDependencies,
+  readPackageJson,
+} from '../utilities/package-tree';
 import { Schema as UpdateCommandSchema } from './update';
 
 const npa = require('npm-package-arg') as (selector: string) => PackageIdentifier;
@@ -377,15 +382,14 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
 
     this.logger.info('Collecting installed dependencies...');
 
-    const packageTree = await readPackageTree(this.workspace.root);
-    const rootDependencies = findNodeDependencies(packageTree);
+    const rootDependencies = await getProjectDependencies(this.workspace.root);
 
-    this.logger.info(`Found ${Object.keys(rootDependencies).length} dependencies.`);
+    this.logger.info(`Found ${rootDependencies.size} dependencies.`);
 
     if (options.all) {
       // 'all' option and a zero length packages have already been checked.
       // Add all direct dependencies to be updated
-      for (const dep of Object.keys(rootDependencies)) {
+      for (const dep of rootDependencies.keys()) {
         const packageIdentifier = npa(dep);
         if (options.next) {
           packageIdentifier.fetchSpec = 'next';
@@ -400,7 +404,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
         next: options.next || false,
         verbose: options.verbose || false,
         packageManager: this.packageManager,
-        packages: options.all ? Object.keys(rootDependencies) : [],
+        packages: options.all ? rootDependencies.keys() : [],
       });
 
       return success ? 0 : 1;
@@ -424,8 +428,9 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
       }
 
       const packageName = packages[0].name;
-      const packageDependency = rootDependencies[packageName];
-      let packageNode = packageDependency && packageDependency.node;
+      const packageDependency = rootDependencies.get(packageName);
+      let packagePath = packageDependency?.path;
+      let packageNode = packageDependency?.package;
       if (packageDependency && !packageNode) {
         this.logger.error('Package found in package.json but is not installed.');
 
@@ -434,19 +439,20 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
         // 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) {
-          packageNode = child;
+        const packageJson = findPackageJson(this.workspace.root, packageName);
+        if (packageJson) {
+          packagePath = path.dirname(packageJson);
+          packageNode = await readPackageJson(packagePath);
         }
       }
 
-      if (!packageNode) {
+      if (!packageNode || !packagePath) {
         this.logger.error('Package is not installed.');
 
         return 1;
       }
 
-      const updateMetadata = packageNode.package['ng-update'];
+      const updateMetadata = packageNode['ng-update'];
       let migrations = updateMetadata && updateMetadata.migrations;
       if (migrations === undefined) {
         this.logger.error('Package does not provide migrations.');
@@ -477,14 +483,14 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
       }
 
       // Check if it is a package-local location
-      const localMigrations = path.join(packageNode.path, migrations);
+      const localMigrations = path.join(packagePath, migrations);
       if (fs.existsSync(localMigrations)) {
         migrations = localMigrations;
       } else {
         // Try to resolve from package location.
         // This avoids issues with package hoisting.
         try {
-          migrations = require.resolve(migrations, { paths: [packageNode.path] });
+          migrations = require.resolve(migrations, { paths: [packagePath] });
         } catch (e) {
           if (e.code === 'MODULE_NOT_FOUND') {
             this.logger.error('Migrations for package were not found.');
@@ -513,7 +519,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
         }
 
         const migrationRange = new semver.Range(
-          '>' + from + ' <=' + (options.to || packageNode.package.version),
+          '>' + from + ' <=' + (options.to || packageNode.version),
         );
 
         success = await this.executeMigrations(
@@ -529,7 +535,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
           packageName === '@angular/core'
           && options.from
           && +options.from.split('.')[0] < 9
-          && (options.to || packageNode.package.version).split('.')[0] === '9'
+          && (options.to || packageNode.version).split('.')[0] === '9'
         ) {
           this.logger.info(NG_VERSION_9_POST_MSG);
         }
@@ -547,8 +553,8 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
 
     // Validate packages actually are part of the workspace
     for (const pkg of packages) {
-      const node = rootDependencies[pkg.name] && rootDependencies[pkg.name].node;
-      if (!node) {
+      const node = rootDependencies.get(pkg.name);
+      if (!node?.package) {
         this.logger.error(`Package '${pkg.name}' is not a dependency.`);
 
         return 1;
@@ -627,7 +633,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
         return 1;
       }
 
-      if (manifest.version === node.package.version) {
+      if (manifest.version === node.package?.version) {
         this.logger.info(`Package '${packageName}' is already up to date.`);
         continue;
       }

packages/angular/cli/lib/init.ts

@@ -11,6 +11,7 @@ import 'symbol-observable';
 import { tags } from '@angular-devkit/core';
 import * as fs from 'fs';
 import * as path from 'path';
+import * as resolve from 'resolve';
 import { SemVer } from 'semver';
 import { Duplex } from 'stream';
 import { colors } from '../utilities/color';
@@ -20,7 +21,7 @@ const packageJson = require('../package.json');
 
 function _fromPackageJson(cwd = process.cwd()): SemVer | null {
   do {
-    const packageJsonPath = path.join(cwd, 'node_modules/@angular/cli/package.json');
+    const packageJsonPath = resolve.sync('@angular/cli/package.json', { paths: [process.cwd()] });
     if (fs.existsSync(packageJsonPath)) {
       const content = fs.readFileSync(packageJsonPath, 'utf-8');
       if (content) {

packages/angular/cli/package.json

@@ -39,7 +39,7 @@
     "npm-pick-manifest": "6.1.0",
     "open": "7.2.0",
     "pacote": "9.5.12",
-    "read-package-tree": "5.3.1",
+    "resolve": "1.17.0",
     "rimraf": "3.0.2",
     "semver": "7.3.2",
     "symbol-observable": "1.2.0",
@@ -50,7 +50,7 @@
     "migrations": "@schematics/angular/migrations/migration-collection.json",
     "packageGroup": {
       "@angular/cli": "0.0.0",
-      "@angular-devkit/build-angular":  "0.0.0",
+      "@angular-devkit/build-angular": "0.0.0",
       "@angular-devkit/build-ng-packagr": "0.0.0",
       "@angular-devkit/build-webpack": "0.0.0",
       "@angular-devkit/core": "0.0.0",

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

@@ -6,88 +6,94 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-
+import * as fs from 'fs';
+import { dirname, join } from 'path';
+import * as resolve from 'resolve';
+import { promisify } from 'util';
 import { NgAddSaveDepedency } from './package-metadata';
 
-export interface PackageTreeNodeBase {
+const readFile = promisify(fs.readFile);
+
+interface PackageJson {
   name: string;
-  path: string;
-  realpath: string;
-  error?: Error;
-  id: number;
-  isLink: boolean;
-  package: {
-    name: string;
-    version: string;
-    dependencies?: Record<string, string>;
-    devDependencies?: Record<string, string>;
-    peerDependencies?: Record<string, string>;
-    optionalDependencies?: Record<string, string>;
-    'ng-update'?: {
-      migrations?: string;
-    };
-    'ng-add'?: {
-      save?: NgAddSaveDepedency;
-    };
+  version: string;
+  dependencies?: Record<string, string>;
+  devDependencies?: Record<string, string>;
+  peerDependencies?: Record<string, string>;
+  optionalDependencies?: Record<string, string>;
+  'ng-update'?: {
+    migrations?: string;
+  };
+  'ng-add'?: {
+    save?: NgAddSaveDepedency;
   };
-  parent?: PackageTreeNode;
-  children: PackageTreeNode[];
 }
 
-export interface PackageTreeActual extends PackageTreeNodeBase {
-  isLink: false;
-}
+async function readJSON(file: string) {
+  const buffer = await readFile(file);
 
-export interface PackageTreeLink extends PackageTreeNodeBase {
-  isLink: true;
-  target: PackageTreeActual;
+  return JSON.parse(buffer.toString());
 }
 
-export type PackageTreeNode = PackageTreeActual | PackageTreeLink;
+function getAllDependencies(pkg: PackageJson) {
+  return new Set([
+    ...Object.entries(pkg.dependencies || []),
+    ...Object.entries(pkg.devDependencies || []),
+    ...Object.entries(pkg.peerDependencies || []),
+    ...Object.entries(pkg.optionalDependencies || []),
+  ]);
+}
 
-export function readPackageTree(path: string): Promise<PackageTreeNode> {
-  const rpt = require('read-package-tree');
+export interface PackageTreeNode {
+  name: string;
+  version: string;
+  path: string;
+  package: PackageJson | undefined;
+}
 
-  return new Promise((resolve, reject) => {
-    rpt(path, (e: Error | undefined, data: PackageTreeNode) => {
-      if (e) {
-        reject(e);
-      } else {
-        resolve(data);
-      }
-    });
-  });
+export async function readPackageJson(packageJsonPath: string): Promise<PackageJson | undefined> {
+  try {
+    return await readJSON(packageJsonPath);
+  } catch (err) {
+    return undefined;
+  }
 }
 
-export interface NodeDependency {
-  version: string;
-  node?: PackageTreeNode;
+export function findPackageJson(workspaceDir: string, packageName: string) {
+  try {
+    // avoid require.resolve here, see: https://github.com/angular/angular-cli/pull/18610#issuecomment-681980185
+    const packageJsonPath = resolve.sync(`${packageName}/package.json`, { paths: [workspaceDir] });
+
+    return packageJsonPath;
+  } catch (err) {
+    return undefined;
+  }
 }
 
-export function findNodeDependencies(node: PackageTreeNode) {
-  const rawDeps: Record<string, string> = {
-    ...node.package.dependencies,
-    ...node.package.devDependencies,
-    ...node.package.peerDependencies,
-    ...node.package.optionalDependencies,
-  };
+export async function getProjectDependencies(dir: string) {
+  const pkgJsonPath = resolve.sync(join(dir, `package.json`));
+  if (!pkgJsonPath) {
+    throw new Error('Could not find package.json');
+  }
 
-  return Object.entries(rawDeps).reduce(
-    (deps, [name, version]) => {
-      let dependencyNode;
-      let parent: PackageTreeNode | undefined | null = node;
-      while (!dependencyNode && parent) {
-        dependencyNode = parent.children.find(child => child.name === name);
-        parent = parent.parent;
-      }
+  const pkg: PackageJson = await readJSON(pkgJsonPath);
 
-      deps[name] = {
-        node: dependencyNode,
-        version,
-      };
+  const results = new Map<string, PackageTreeNode>();
+  await Promise.all(
+    Array.from(getAllDependencies(pkg)).map(async ([name, version]) => {
+      const packageJsonPath = findPackageJson(dir, name);
+      if (packageJsonPath) {
+        const currentDependency = {
+          name,
+          version,
+          path: dirname(packageJsonPath),
+          package: await readPackageJson(packageJsonPath),
+        };
 
-      return deps;
-    },
-    Object.create(null) as Record<string, NodeDependency>,
+        results.set(currentDependency.name, currentDependency);
+      }
+    }),
   );
+
+  return results;
 }

tests/legacy-cli/e2e/assets/1.0-yarn-workspace-project/package.json

@@ -0,0 +1,14 @@
+{
+  "private": true,
+  "workspaces": {
+    "packages": [
+      "packages/*"
+    ],
+    "nohoist": [
+      "**/codelyzer",
+      "**/codelyzer/**",
+      "**/@angular*",
+      "**/@angular*/**"
+    ]
+  }
+}