fix(ngcc): handle bad path mappings when finding entry-points (#36331)

Previously, a bad baseUrl or path mapping passed to an `EntryPointFinder`
could cause the original `sourceDirectory` to be superceded by a higher
directory. This could result in none of the sourceDirectory entry-points being
processed.

Now missing basePaths computed from path-mappings are discarded with
a warning. Further, if the `baseUrl` is the root directory then a warning is
given as this is most likely an error in the tsconfig.json.

Resolves #36313
Resolves #36283

PR Close #36331

packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts

@@ -21,7 +21,7 @@ import {getBasePaths, trackDuration} from './utils';
  * `pathMappings`.
  */
 export class DirectoryWalkerEntryPointFinder implements EntryPointFinder {
-  private basePaths = getBasePaths(this.sourceDirectory, this.pathMappings);
+  private basePaths = getBasePaths(this.logger, this.sourceDirectory, this.pathMappings);
   constructor(
       private fs: FileSystem, private config: NgccConfiguration, private logger: Logger,
       private resolver: DependencyResolver, private entryPointManifest: EntryPointManifest,

packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts

@@ -25,7 +25,7 @@ import {getBasePaths} from './utils';
 export class TargetedEntryPointFinder implements EntryPointFinder {
   private unprocessedPaths: AbsoluteFsPath[] = [];
   private unsortedEntryPoints = new Map<AbsoluteFsPath, EntryPoint>();
-  private basePaths = getBasePaths(this.basePath, this.pathMappings);
+  private basePaths = getBasePaths(this.logger, this.basePath, this.pathMappings);
 
   constructor(
       private fs: FileSystem, private config: NgccConfiguration, private logger: Logger,

packages/compiler-cli/ngcc/src/entry_point_finder/utils.ts

@@ -5,7 +5,8 @@
  * 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 {AbsoluteFsPath, getFileSystem, join, relative, resolve} from '../../../src/ngtsc/file_system';
+import {AbsoluteFsPath, getFileSystem, relative, resolve} from '../../../src/ngtsc/file_system';
+import {Logger} from '../logging/logger';
 import {PathMappings} from '../utils';
 
 /**
@@ -28,22 +29,44 @@ import {PathMappings} from '../utils';
  * @param pathMappings Path mapping configuration, from which to extract additional base-paths.
  */
 export function getBasePaths(
-    sourceDirectory: AbsoluteFsPath, pathMappings: PathMappings | undefined): AbsoluteFsPath[] {
+    logger: Logger, sourceDirectory: AbsoluteFsPath,
+    pathMappings: PathMappings | undefined): AbsoluteFsPath[] {
   const fs = getFileSystem();
   const basePaths = [sourceDirectory];
   if (pathMappings) {
     const baseUrl = resolve(pathMappings.baseUrl);
+    if (fs.isRoot(baseUrl)) {
+      logger.warn(
+          `The provided pathMappings baseUrl is the root path ${baseUrl}.\n` +
+          `This is likely to mess up how ngcc finds entry-points and is probably not correct.\n` +
+          `Please check your path mappings configuration such as in the tsconfig.json file.`);
+    }
     Object.values(pathMappings.paths).forEach(paths => paths.forEach(path => {
       // We only want base paths that exist and are not files
-      let basePath = join(baseUrl, extractPathPrefix(path));
-      while (basePath !== baseUrl && (!fs.exists(basePath) || fs.stat(basePath).isFile())) {
+      let basePath = fs.resolve(baseUrl, extractPathPrefix(path));
+      if (fs.exists(basePath) && fs.stat(basePath).isFile()) {
         basePath = fs.dirname(basePath);
       }
-      basePaths.push(basePath);
+      if (fs.exists(basePath)) {
+        basePaths.push(basePath);
+      } else {
+        logger.warn(
+            `The basePath "${basePath}" computed from baseUrl "${baseUrl}" and path mapping "${path}" does not exist in the file-system.\n` +
+            `It will not be scanned for entry-points.`);
+      }
     }));
   }
   basePaths.sort().reverse();  // Get the paths in order with the longer ones first.
-  return basePaths.filter(removeContainedPaths);
+  const dedupedBasePaths = basePaths.filter(removeContainedPaths);
+
+  // We want to ensure that the `sourceDirectory` is included when it is a node_modules folder.
+  // Otherwise our entry-point finding algorithm would fail to walk that folder.
+  if (fs.basename(sourceDirectory) === 'node_modules' &&
+      !dedupedBasePaths.includes(sourceDirectory)) {
+    dedupedBasePaths.unshift(sourceDirectory);
+  }
+
+  return dedupedBasePaths;
 }
 
 /**

packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts

@@ -340,7 +340,7 @@ runInEachFileSystem(() => {
         const pathMappings: PathMappings = {
           baseUrl: '/path_mapped/dist',
           paths: {
-            '@test': ['pkg2/fesm2015/pkg2.js'],
+            '@test': ['pkg2/pkg2.d.ts'],
             '@missing': ['pkg3'],
           }
         };
@@ -371,6 +371,10 @@ runInEachFileSystem(() => {
               fesm2015: `./fesm2015/${packageName}.js`,
             })
           },
+          {
+            name: _Abs(`${basePath}/${packageName}/${packageName}.d.ts`),
+            contents: deps.map((dep, i) => `import * as i${i} from '${dep}';`).join('\n'),
+          },
           {
             name: _Abs(`${basePath}/${packageName}/fesm2015/${packageName}.js`),
             contents: deps.map((dep, i) => `import * as i${i} from '${dep}';`).join('\n'),

packages/compiler-cli/ngcc/test/entry_point_finder/utils_spec.ts

@@ -0,0 +1,142 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * 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 {absoluteFrom, getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system';
+
+import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
+import {getBasePaths} from '../../src/entry_point_finder/utils';
+import {MockLogger} from '../helpers/mock_logger';
+
+runInEachFileSystem(() => {
+  let _: typeof absoluteFrom;
+  let logger: MockLogger;
+
+  beforeEach(() => {
+    _ = absoluteFrom;
+    logger = new MockLogger();
+  });
+
+  describe('getBasePaths', () => {
+    it('should just return the `sourceDirectory if there are no `pathMappings', () => {
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const basePaths = getBasePaths(logger, sourceDirectory, undefined);
+      expect(basePaths).toEqual([sourceDirectory]);
+    });
+
+    it('should use each path mapping prefix and sort in descending order', () => {
+      const projectDirectory = _('/path/to/project');
+      const fs = getFileSystem();
+      fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
+      fs.ensureDir(fs.resolve(projectDirectory, 'sub-folder/dist-2'));
+      fs.ensureDir(fs.resolve(projectDirectory, 'libs'));
+
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const pathMappings = {
+        baseUrl: projectDirectory,
+        paths: {'@dist': ['dist-1', 'sub-folder/dist-2'], '@lib/*': ['libs/*']}
+      };
+      const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+      expect(basePaths).toEqual([
+        fs.resolve(projectDirectory, 'sub-folder/dist-2'),
+        sourceDirectory,
+        fs.resolve(projectDirectory, 'libs'),
+        fs.resolve(projectDirectory, 'dist-1'),
+      ]);
+    });
+
+    it('should discard paths that are already contained by another path', () => {
+      const projectDirectory = _('/path/to/project');
+      const fs = getFileSystem();
+      fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
+      fs.ensureDir(fs.resolve(projectDirectory, 'dist-1/sub-folder'));
+      fs.ensureDir(fs.resolve(projectDirectory, 'node_modules/libs'));
+
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const pathMappings = {
+        baseUrl: projectDirectory,
+        paths: {'@dist': ['dist-1', 'dist-1/sub-folder'], '@lib/*': ['node_modules/libs/*']}
+      };
+      const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+      expect(basePaths).toEqual([
+        sourceDirectory,
+        fs.resolve(projectDirectory, 'dist-1'),
+      ]);
+    });
+
+    it('should use the containing directory of path mapped files', () => {
+      const projectDirectory = _('/path/to/project');
+      const fs = getFileSystem();
+      fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
+      fs.writeFile(fs.resolve(projectDirectory, 'dist-1/file.js'), 'dummy content');
+
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const pathMappings = {baseUrl: projectDirectory, paths: {'@dist': ['dist-1/file.js']}};
+      const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+      expect(basePaths).toEqual([
+        sourceDirectory,
+        fs.resolve(projectDirectory, 'dist-1'),
+      ]);
+    });
+
+    it('should always include the `sourceDirectory` if it is a node_modules directory in the returned basePaths, even if it is contained by another basePath',
+       () => {
+         const projectDirectory = _('/path/to/project');
+         const sourceDirectory = _('/path/to/project/node_modules');
+         const fs = getFileSystem();
+         fs.ensureDir(fs.resolve(sourceDirectory));
+
+         const pathMappings = {baseUrl: projectDirectory, paths: {'*': ['./*']}};
+         const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+         expect(basePaths).toEqual([
+           sourceDirectory,
+           projectDirectory,
+         ]);
+       });
+
+    it('should log a warning if baseUrl is the root path', () => {
+      const fs = getFileSystem();
+      fs.ensureDir(fs.resolve('/dist'));
+
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const pathMappings = {baseUrl: _('/'), paths: {'@dist': ['dist']}};
+      const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+      expect(basePaths).toEqual([
+        sourceDirectory,
+        fs.resolve('/dist'),
+      ]);
+      expect(logger.logs.warn).toEqual([
+        [`The provided pathMappings baseUrl is the root path ${_('/')}.\n` +
+         `This is likely to mess up how ngcc finds entry-points and is probably not correct.\n` +
+         `Please check your path mappings configuration such as in the tsconfig.json file.`]
+      ]);
+    });
+
+    it('should discard basePaths that do not exists and log a warning', () => {
+      const projectDirectory = _('/path/to/project');
+      const fs = getFileSystem();
+      fs.ensureDir(fs.resolve(projectDirectory, 'dist-1'));
+      fs.ensureDir(fs.resolve(projectDirectory, 'sub-folder'));
+
+      const sourceDirectory = _('/path/to/project/node_modules');
+      const pathMappings = {
+        baseUrl: projectDirectory,
+        paths: {'@dist': ['dist-1', 'sub-folder/dist-2'], '@lib/*': ['libs/*']}
+      };
+      const basePaths = getBasePaths(logger, sourceDirectory, pathMappings);
+      expect(basePaths).toEqual([
+        sourceDirectory,
+        fs.resolve(projectDirectory, 'dist-1'),
+      ]);
+      expect(logger.logs.warn).toEqual([
+        [`The basePath "${fs.resolve(projectDirectory, 'sub-folder/dist-2')}" computed from baseUrl "${projectDirectory}" and path mapping "sub-folder/dist-2" does not exist in the file-system.\n` +
+         `It will not be scanned for entry-points.`],
+        [`The basePath "${fs.resolve(projectDirectory, 'libs')}" computed from baseUrl "${projectDirectory}" and path mapping "libs/*" does not exist in the file-system.\n` +
+         `It will not be scanned for entry-points.`],
+      ]);
+    });
+  });
+});