fix(ivy): ngcc - resolve `main` property paths correctly (#31509)

When determining if a `main` path points to a UMD or CommonJS
format, the contents of the file need to be loaded and parsed.

Previously, it was assumed that the path referred to the exact filename,
but did not account for normal module resolution semantics, where the
path may be missing an extension or refer to a directory containing an
`index.js` file.

// FW-1444

PR Close #31509

packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts

@@ -5,11 +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, FileSystem, absoluteFrom, dirname, isRoot, join, resolve} from '../../../src/ngtsc/file_system';
-import {PathMappings, isRelativePath} from '../utils';
-
-
+import {PathMappings, isRelativePath, resolveFileWithPostfixes} from '../utils';
 
 /**
  * This is a very cut-down implementation of the TypeScript module resolution strategy.
@@ -72,8 +69,8 @@ export class ModuleResolver {
    * If neither of these files exist then the method returns `null`.
    */
   private resolveAsRelativePath(moduleName: string, fromPath: AbsoluteFsPath): ResolvedModule|null {
-    const resolvedPath =
-        this.resolvePath(resolve(dirname(fromPath), moduleName), this.relativeExtensions);
+    const resolvedPath = resolveFileWithPostfixes(
+        this.fs, resolve(dirname(fromPath), moduleName), this.relativeExtensions);
     return resolvedPath && new ResolvedRelativeModule(resolvedPath);
   }
 
@@ -133,20 +130,6 @@ export class ModuleResolver {
     return null;
   }
 
-  /**
-   * Attempt to resolve a `path` to a file by appending the provided `postFixes`
-   * to the `path` and checking if the file exists on disk.
-   * @returns An absolute path to the first matching existing file, or `null` if none exist.
-   */
-  private resolvePath(path: AbsoluteFsPath, postFixes: string[]): AbsoluteFsPath|null {
-    for (const postFix of postFixes) {
-      const testPath = absoluteFrom(path + postFix);
-      if (this.fs.exists(testPath)) {
-        return testPath;
-      }
-    }
-    return null;
-  }
 
   /**
    * Can we consider the given path as an entry-point to a package?

packages/compiler-cli/ngcc/src/dependencies/umd_dependency_host.ts

@@ -6,11 +6,15 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as ts from 'typescript';
+
 import {AbsoluteFsPath, FileSystem, PathSegment} from '../../../src/ngtsc/file_system';
 import {getImportsOfUmdModule, parseStatementForUmdModule} from '../host/umd_host';
+import {resolveFileWithPostfixes} from '../utils';
+
 import {DependencyHost, DependencyInfo} from './dependency_host';
 import {ModuleResolver, ResolvedDeepImport, ResolvedRelativeModule} from './module_resolver';
 
+
 /**
  * Helper functions for computing dependencies.
  */
@@ -50,15 +54,20 @@ export class UmdDependencyHost implements DependencyHost {
   private recursivelyFindDependencies(
       file: AbsoluteFsPath, dependencies: Set<AbsoluteFsPath>, missing: Set<string>,
       deepImports: Set<string>, alreadySeen: Set<AbsoluteFsPath>): void {
-    const fromContents = this.fs.readFile(file);
+    const resolvedFile = resolveFileWithPostfixes(this.fs, file, ['', '.js', '/index.js']);
+    if (resolvedFile === null) {
+      return;
+    }
+
+    const fromContents = this.fs.readFile(resolvedFile);
     if (!this.hasRequireCalls(fromContents)) {
       // Avoid parsing the source file as there are no require calls.
       return;
     }
 
     // Parse the source into a TypeScript AST and then walk it looking for imports and re-exports.
-    const sf =
-        ts.createSourceFile(file, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
+    const sf = ts.createSourceFile(
+        resolvedFile, fromContents, ts.ScriptTarget.ES2015, false, ts.ScriptKind.JS);
     if (sf.statements.length !== 1) {
       return;
     }
@@ -70,7 +79,7 @@ export class UmdDependencyHost implements DependencyHost {
     }
 
     umdImports.forEach(umdImport => {
-      const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, file);
+      const resolvedModule = this.moduleResolver.resolveModuleImport(umdImport.path, resolvedFile);
       if (resolvedModule) {
         if (resolvedModule instanceof ResolvedRelativeModule) {
           const internalDependency = resolvedModule.modulePath;

packages/compiler-cli/ngcc/src/packages/entry_point.ts

@@ -11,6 +11,7 @@ import * as ts from 'typescript';
 import {AbsoluteFsPath, FileSystem, join, resolve} from '../../../src/ngtsc/file_system';
 import {parseStatementForUmdModule} from '../host/umd_host';
 import {Logger} from '../logging/logger';
+import {resolveFileWithPostfixes} from '../utils';
 import {NgccConfiguration, NgccEntryPointConfig} from './configuration';
 
 /**
@@ -167,8 +168,12 @@ function loadEntryPointPackage(
 }
 
 function isUmdModule(fs: FileSystem, sourceFilePath: AbsoluteFsPath): boolean {
+  const resolvedPath = resolveFileWithPostfixes(fs, sourceFilePath, ['', '.js', '/index.js']);
+  if (resolvedPath === null) {
+    return false;
+  }
   const sourceFile =
-      ts.createSourceFile(sourceFilePath, fs.readFile(sourceFilePath), ts.ScriptTarget.ES5);
+      ts.createSourceFile(sourceFilePath, fs.readFile(resolvedPath), ts.ScriptTarget.ES5);
   return sourceFile.statements.length > 0 &&
       parseStatementForUmdModule(sourceFile.statements[0]) !== null;
 }

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

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import * as ts from 'typescript';
+import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../src/ngtsc/file_system';
 
 export function getOriginalSymbol(checker: ts.TypeChecker): (symbol: ts.Symbol) => ts.Symbol {
   return function(symbol: ts.Symbol) {
@@ -65,3 +66,19 @@ export type PathMappings = {
 export function isRelativePath(path: string): boolean {
   return /^\/|^\.\.?($|\/)/.test(path);
 }
+
+/**
+ * Attempt to resolve a `path` to a file by appending the provided `postFixes`
+ * to the `path` and checking if the file exists on disk.
+ * @returns An absolute path to the first matching existing file, or `null` if none exist.
+ */
+export function resolveFileWithPostfixes(
+    fs: FileSystem, path: AbsoluteFsPath, postFixes: string[]): AbsoluteFsPath|null {
+  for (const postFix of postFixes) {
+    const testPath = absoluteFrom(path + postFix);
+    if (fs.exists(testPath) && fs.stat(testPath).isFile()) {
+      return testPath;
+    }
+  }
+  return null;
+}

packages/compiler-cli/ngcc/test/dependencies/umd_dependency_host_spec.ts

@@ -115,6 +115,16 @@ runInEachFileSystem(() => {
         expect(missing.size).toBe(0);
         expect(deepImports.size).toBe(0);
       });
+
+      it('should resolve path to the file if it has no extension', () => {
+        const {dependencies, missing, deepImports} =
+            host.findDependencies(_('/external/imports/index'));
+        expect(dependencies.size).toBe(2);
+        expect(missing.size).toBe(0);
+        expect(deepImports.size).toBe(0);
+        expect(dependencies.has(_('/node_modules/lib_1'))).toBe(true);
+        expect(dependencies.has(_('/node_modules/lib_1/sub_1'))).toBe(true);
+      });
     });
 
     function setupMockFileSystem(): void {

packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts

@@ -10,7 +10,7 @@ import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem} from '../../../
 import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
 import {loadTestFiles} from '../../../test/helpers';
 import {NgccConfiguration} from '../../src/packages/configuration';
-import {SUPPORTED_FORMAT_PROPERTIES, getEntryPointInfo} from '../../src/packages/entry_point';
+import {EntryPoint, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat, getEntryPointInfo} from '../../src/packages/entry_point';
 import {MockLogger} from '../helpers/mock_logger';
 
 runInEachFileSystem(() => {
@@ -340,6 +340,92 @@ runInEachFileSystem(() => {
     });
   });
 
+  describe('getEntryPointFormat', () => {
+    let SOME_PACKAGE: AbsoluteFsPath;
+    let _: typeof absoluteFrom;
+    let fs: FileSystem;
+    let entryPoint: EntryPoint;
+
+    beforeEach(() => {
+      _ = absoluteFrom;
+      SOME_PACKAGE = _('/project/node_modules/some_package');
+      fs = getFileSystem();
+      loadTestFiles([{
+        name: _('/project/node_modules/some_package/valid_entry_point/package.json'),
+        contents: createPackageJson('valid_entry_point')
+      }]);
+      const config = new NgccConfiguration(fs, _('/project'));
+      entryPoint = getEntryPointInfo(
+          fs, config, new MockLogger(), SOME_PACKAGE,
+          _('/project/node_modules/some_package/valid_entry_point')) !;
+    });
+
+    it('should return `esm2015` format for `fesm2015` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'fesm2015')).toBe('esm2015'); });
+
+    it('should return `esm5` format for `fesm5` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'fesm5')).toBe('esm5'); });
+
+    it('should return `esm2015` format for `es2015` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'es2015')).toBe('esm2015'); });
+
+    it('should return `esm2015` format for `esm2015` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'esm2015')).toBe('esm2015'); });
+
+    it('should return `esm5` format for `esm5` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'esm5')).toBe('esm5'); });
+
+    it('should return `esm5` format for `module` property',
+       () => { expect(getEntryPointFormat(fs, entryPoint, 'module')).toBe('esm5'); });
+
+    it('should return `umd` for `main` if the file contains a UMD wrapper function', () => {
+      loadTestFiles([{
+        name: _(
+            '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'),
+        contents: `
+        (function (global, factory) {
+          typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) :
+          typeof define === 'function' && define.amd ? define('@angular/common', ['exports', '@angular/core'], factory) :
+          (global = global || self, factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core));
+        }(this, function (exports, core) { 'use strict'; }));
+      `
+      }]);
+      expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd');
+    });
+
+    it('should return `commonjs` for `main` if the file does not contain a UMD wrapper function', () => {
+      loadTestFiles([{
+        name: _(
+            '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'),
+        contents: `
+          const core = require('@angular/core);
+          module.exports = {};
+        `
+      }]);
+      expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('commonjs');
+    });
+
+    it('should resolve the format path with suitable postfixes', () => {
+      loadTestFiles([{
+        name: _(
+            '/project/node_modules/some_package/valid_entry_point/bundles/valid_entry_point/index.js'),
+        contents: `
+        (function (global, factory) {
+          typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) :
+          typeof define === 'function' && define.amd ? define('@angular/common', ['exports', '@angular/core'], factory) :
+          (global = global || self, factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core));
+        }(this, function (exports, core) { 'use strict'; }));
+      `
+      }]);
+
+      entryPoint.packageJson.main = './bundles/valid_entry_point/index';
+      expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd');
+
+      entryPoint.packageJson.main = './bundles/valid_entry_point';
+      expect(getEntryPointFormat(fs, entryPoint, 'main')).toBe('umd');
+    });
+  });
+
   function createPackageJson(
       packageName: string, {excludes}: {excludes?: string[]} = {},
       typingsProp: string = 'typings'): string {
@@ -351,7 +437,7 @@ runInEachFileSystem(() => {
       es2015: `./es2015/${packageName}.js`,
       fesm5: `./fesm5/${packageName}.js`,
       esm5: `./esm5/${packageName}.js`,
-      main: `./bundles/${packageName}.umd.js`,
+      main: `./bundles/${packageName}/index.js`,
       module: './index.js',
     };
     if (excludes) {