refactor: use require.resolve instead of custom resolve (#15906)

Author: alan-agius4

etc/api/angular_devkit/schematics/tools/index.d.ts

@@ -108,8 +108,6 @@ export declare class InvalidCollectionJsonException extends BaseException {
 export declare class NodeModulesEngineHost extends FileSystemEngineHostBase {
     constructor();
     protected _resolveCollectionPath(name: string): string;
-    protected _resolvePackageJson(name: string, basedir?: string): string;
-    protected _resolvePath(name: string, basedir?: string): string;
     protected _resolveReferenceString(refString: string, parentPath: string): {
         ref: RuleFactory<{}>;
         path: string;

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

@@ -139,7 +139,17 @@ export class AddCommand extends SchematicCommand<AddCommandSchema> {
     }
 
     if (savePackage === false) {
-      installTempPackage(packageIdentifier.raw, this.logger, packageManager);
+      // Temporary packages are located in a different directory
+      // Hence we need to resolve them using the temp path
+      const tempPath = installTempPackage(packageIdentifier.raw, this.logger, packageManager);
+      const resolvedCollectionPath = require.resolve(
+        join(collectionName, 'package.json'),
+        {
+          paths: [tempPath],
+        },
+      );
+
+      collectionName = dirname(resolvedCollectionPath);
     } else {
       installPackage(packageIdentifier.raw, this.logger, packageManager, savePackage);
     }

packages/angular/cli/tasks/install-package.ts

@@ -105,11 +105,6 @@ export function installTempPackage(
     tempNodeModules = join(tempPath, 'node_modules');
   }
 
-  // Needed to resolve schematics from this location since we use a custom
-  // resolve strategy in '@angular/devkit-core/node'
-  // todo: this should be removed when we change the resolutions to use require.resolve
-  process.env.NG_TEMP_MODULES_DIR = tempNodeModules;
-
   return tempNodeModules;
 }
 

packages/angular_devkit/schematics/tools/node-module-engine-host.ts

@@ -10,8 +10,7 @@ import {
   InvalidJsonCharacterException,
   UnexpectedEndOfInputException,
 } from '@angular-devkit/core';
-import * as core from '@angular-devkit/core/node';
-import { dirname, join, resolve as resolvePath } from 'path';
+import { dirname, extname, join, resolve } from 'path';
 import { RuleFactory } from '../src';
 import {
   FileSystemCollectionDesc,
@@ -27,7 +26,6 @@ import {
 } from './file-system-engine-host-base';
 import { readJsonFile } from './file-system-utility';
 
-
 export class NodePackageDoesNotSupportSchematics extends BaseException {
   constructor(name: string) {
     super(`Package ${JSON.stringify(name)} was found but does not support schematics.`);
@@ -41,71 +39,30 @@ export class NodePackageDoesNotSupportSchematics extends BaseException {
 export class NodeModulesEngineHost extends FileSystemEngineHostBase {
   constructor() { super(); }
 
-  protected _resolvePackageJson(name: string, basedir = process.cwd()) {
-    return core.resolve(name, {
-      basedir,
-      checkLocal: true,
-      checkGlobal: true,
-      resolvePackageJson: true,
-    });
-  }
-
-  protected _resolvePath(name: string, basedir = process.cwd()) {
-    // Allow relative / absolute paths.
-    if (name.startsWith('.') || name.startsWith('/')) {
-      return resolvePath(basedir, name);
-    } else {
-      // If it's a file inside a package, resolve the package then return the file...
-      if (name.split('/').length > (name[0] == '@' ? 2 : 1)) {
-        const rest = name.split('/');
-        const packageName = rest.shift() + (name[0] == '@' ? '/' + rest.shift() : '');
-
-        return resolvePath(core.resolve(packageName, {
-          basedir,
-          checkLocal: true,
-          checkGlobal: true,
-          resolvePackageJson: true,
-        }), '..', ...rest);
-      }
-
-      return core.resolve(name, {
-        basedir,
-        checkLocal: true,
-        checkGlobal: true,
-      });
-    }
-  }
-
   protected _resolveCollectionPath(name: string): string {
     let collectionPath: string | undefined = undefined;
-
-    if (name.replace(/\\/g, '/').split('/').length > (name[0] == '@' ? 2 : 1)) {
-      try {
-        collectionPath = this._resolvePath(name, process.cwd());
-      } catch {
-      }
+    if (name.startsWith('.') || name.startsWith('/')) {
+      name = resolve(name);
     }
 
-    if (!collectionPath) {
-      let packageJsonPath = this._resolvePackageJson(name, process.cwd());
-      // If it's a file, use it as is. Otherwise append package.json to it.
-      if (!core.fs.isFile(packageJsonPath)) {
-        packageJsonPath = join(packageJsonPath, 'package.json');
-      }
+    if (extname(name)) {
+      // When having an extension let's just resolve the provided path.
+      collectionPath = require.resolve(name);
+    } else {
+      const packageJsonPath = require.resolve(join(name, 'package.json'));
+      const { schematics } = require(packageJsonPath);
 
-      const pkgJsonSchematics = require(packageJsonPath)['schematics'];
-      if (!pkgJsonSchematics || typeof pkgJsonSchematics != 'string') {
+      if (!schematics || typeof schematics !== 'string') {
         throw new NodePackageDoesNotSupportSchematics(name);
       }
-      collectionPath = this._resolvePath(pkgJsonSchematics, dirname(packageJsonPath));
+
+      collectionPath = resolve(dirname(packageJsonPath), schematics);
     }
 
     try {
-      if (collectionPath) {
-        readJsonFile(collectionPath);
+      readJsonFile(collectionPath);
 
-        return collectionPath;
-      }
+      return collectionPath;
     } catch (e) {
       if (
         e instanceof InvalidJsonCharacterException || e instanceof UnexpectedEndOfInputException

packages/angular_devkit/schematics/tools/node-module-engine-host_spec.ts

@@ -14,7 +14,7 @@ import * as os from 'os';
 import * as path from 'path';
 import { NodeModulesEngineHost } from './node-module-engine-host';
 
-const TMP_DIR = process.env['$TEST_TMPDIR'] || os.tmpdir();
+const TMP_DIR = process.env['TEST_TMPDIR'] || os.tmpdir();
 
 describe('NodeModulesEngineHost', () => {
   let tmpDir!: string;
@@ -47,8 +47,16 @@ describe('NodeModulesEngineHost', () => {
     const engineHost = new NodeModulesEngineHost();
     const engine = new SchematicEngine(engineHost);
 
+    // Under Bazel 'require.resolve' is patched to use Bazel resolutions from the MANIFEST FILES.
+    // Adding a temporary file won't be enough to make Bazel aware of this file.
+    // We provide the full path here just to verify that the underlying logic works
+    let prefix = '';
+    if (process.env['BAZEL_TARGET']) {
+      prefix = path.join(process.cwd(), 'node_modules');
+    }
+
     expect(() => {
-      engine.createCollection(path.join('@angular/core', './schematics/migrations.json'));
+      engine.createCollection(path.join(prefix, '@angular/core', './schematics/migrations.json'));
     }).not.toThrow();
   });
 });