fix(ivy): ngtsc is unable to detect flat module entry-point on windows (

angular#29453)

Currently when building an Angular project with `ngtsc`
and `flatModuleOutFile` enabled, the Ngtsc build will fail
if there are multiple source files as root file names.

Ngtsc and NGC currently determine the entry-point for multiple
root file names by looking for files ending with `/index.ts`.

This functionality is technically deprecated, but still supported
and currently breaks on Windows as the root file names are not
guaranteed to be normalized POSIX-like paths.

In order to make this logic more reliable in the future, this commit
also switches the shim generators and entry-point logic to the branded
path types. This ensures that we don't break this in the future.

PR Close angular#29453

packages/compiler-cli/src/ngtsc/entry_point/BUILD.bazel

@@ -10,6 +10,7 @@ ts_library(
     module_name = "@angular/compiler-cli/src/ngtsc/entry_point",
     deps = [
         "//packages/compiler-cli/src/ngtsc/diagnostics",
+        "//packages/compiler-cli/src/ngtsc/path",
         "//packages/compiler-cli/src/ngtsc/shims",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//@types/node",

packages/compiler-cli/src/ngtsc/entry_point/src/logic.ts

@@ -6,10 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {normalizeSeparators} from '../../util/src/path';
+import {AbsoluteFsPath} from '../../path/src/types';
 import {isNonDeclarationTsPath} from '../../util/src/typescript';
 
-export function findFlatIndexEntryPoint(rootFiles: ReadonlyArray<string>): string|null {
+export function findFlatIndexEntryPoint(rootFiles: ReadonlyArray<AbsoluteFsPath>): string|null {
   // There are two ways for a file to be recognized as the flat module index:
   // 1) if it's the only file!!!!!!
   // 2) (deprecated) if it's named 'index.ts' and has the shortest path of all such files.
@@ -33,5 +33,5 @@ export function findFlatIndexEntryPoint(rootFiles: ReadonlyArray<string>): strin
     }
   }
 
-  return resolvedEntryPoint ? normalizeSeparators(resolvedEntryPoint) : null;
+  return resolvedEntryPoint;
 }

packages/compiler-cli/src/ngtsc/entry_point/test/BUILD.bazel

@@ -11,6 +11,7 @@ ts_library(
     deps = [
         "//packages:types",
         "//packages/compiler-cli/src/ngtsc/entry_point",
+        "//packages/compiler-cli/src/ngtsc/path",
         "@npm//typescript",
     ],
 )

packages/compiler-cli/src/ngtsc/entry_point/test/entry_point_spec.ts

@@ -7,22 +7,23 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {AbsoluteFsPath} from '../../path/src/types';
 import {findFlatIndexEntryPoint} from '../src/logic';
 
 describe('entry_point logic', () => {
 
   describe('findFlatIndexEntryPoint', () => {
 
-    it('should use the only source file if only a single one is specified',
-       () => { expect(findFlatIndexEntryPoint(['/src/index.ts'])).toBe('/src/index.ts'); });
+    it('should use the only source file if only a single one is specified', () => {
+      expect(findFlatIndexEntryPoint([AbsoluteFsPath.fromUnchecked('/src/index.ts')]))
+          .toBe('/src/index.ts');
+    });
 
     it('should use the shortest source file ending with "index.ts" for multiple files', () => {
       expect(findFlatIndexEntryPoint([
-        '/src/deep/index.ts', '/src/index.ts', '/index.ts'
+        AbsoluteFsPath.fromUnchecked('/src/deep/index.ts'),
+        AbsoluteFsPath.fromUnchecked('/src/index.ts'), AbsoluteFsPath.fromUnchecked('/index.ts')
       ])).toBe('/index.ts');
     });
-
-    it('should normalize the path separators for the found entry point',
-       () => { expect(findFlatIndexEntryPoint(['\\src\\index.ts'])).toBe('/src/index.ts'); });
   });
 });

packages/compiler-cli/src/ngtsc/program.ts

@@ -65,6 +65,7 @@ export class NgtscProgram implements api.Program {
     this.closureCompilerEnabled = !!options.annotateForClosureCompiler;
     this.resourceManager = new HostResourceLoader(host, options);
     const shouldGenerateShims = options.allowEmptyCodegenFiles || false;
+    const normalizedRootNames = rootNames.map(n => AbsoluteFsPath.from(n));
     this.host = host;
     if (host.fileNameToModuleName !== undefined) {
       this.fileToModuleHost = host as FileToModuleHost;
@@ -74,10 +75,10 @@ export class NgtscProgram implements api.Program {
     const generators: ShimGenerator[] = [];
     if (shouldGenerateShims) {
       // Summary generation.
-      const summaryGenerator = SummaryGenerator.forRootFiles(rootNames);
+      const summaryGenerator = SummaryGenerator.forRootFiles(normalizedRootNames);
 
       // Factory generation.
-      const factoryGenerator = FactoryGenerator.forRootFiles(rootNames);
+      const factoryGenerator = FactoryGenerator.forRootFiles(normalizedRootNames);
       const factoryFileMap = factoryGenerator.factoryFileMap;
       this.factoryToSourceInfo = new Map<string, FactoryInfo>();
       this.sourceToFactorySymbols = new Map<string, Set<string>>();
@@ -94,7 +95,7 @@ export class NgtscProgram implements api.Program {
 
     let entryPoint: string|null = null;
     if (options.flatModuleOutFile !== undefined) {
-      entryPoint = findFlatIndexEntryPoint(rootNames);
+      entryPoint = findFlatIndexEntryPoint(normalizedRootNames);
       if (entryPoint === null) {
         // This error message talks specifically about having a single .ts file in "files". However
         // the actual logic is a bit more permissive. If a single file exists, that will be taken,

packages/compiler-cli/src/ngtsc/shims/BUILD.bazel

@@ -10,6 +10,7 @@ ts_library(
     deps = [
         "//packages/compiler",
         "//packages/compiler-cli/src/ngtsc/imports",
+        "//packages/compiler-cli/src/ngtsc/path",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//@types/node",
         "@npm//typescript",

packages/compiler-cli/src/ngtsc/shims/src/factory_generator.ts

@@ -10,7 +10,7 @@ import * as path from 'path';
 import * as ts from 'typescript';
 
 import {ImportRewriter} from '../../imports';
-import {normalizeSeparators} from '../../util/src/path';
+import {AbsoluteFsPath} from '../../path/src/types';
 import {isNonDeclarationTsPath} from '../../util/src/typescript';
 
 import {ShimGenerator} from './host';
@@ -28,10 +28,10 @@ export class FactoryGenerator implements ShimGenerator {
 
   get factoryFileMap(): Map<string, string> { return this.map; }
 
-  recognize(fileName: string): boolean { return this.map.has(fileName); }
+  recognize(fileName: AbsoluteFsPath): boolean { return this.map.has(fileName); }
 
-  generate(genFilePath: string, readFile: (fileName: string) => ts.SourceFile | null): ts.SourceFile
-      |null {
+  generate(genFilePath: AbsoluteFsPath, readFile: (fileName: string) => ts.SourceFile | null):
+      ts.SourceFile|null {
     const originalPath = this.map.get(genFilePath) !;
     const original = readFile(originalPath);
     if (original === null) {
@@ -98,11 +98,13 @@ export class FactoryGenerator implements ShimGenerator {
     return genFile;
   }
 
-  static forRootFiles(files: ReadonlyArray<string>): FactoryGenerator {
-    const map = new Map<string, string>();
+  static forRootFiles(files: ReadonlyArray<AbsoluteFsPath>): FactoryGenerator {
+    const map = new Map<AbsoluteFsPath, string>();
     files.filter(sourceFile => isNonDeclarationTsPath(sourceFile))
-        .map(sourceFile => normalizeSeparators(sourceFile))
-        .forEach(sourceFile => map.set(sourceFile.replace(/\.ts$/, '.ngfactory.ts'), sourceFile));
+        .forEach(
+            sourceFile => map.set(
+                AbsoluteFsPath.fromUnchecked(sourceFile.replace(/\.ts$/, '.ngfactory.ts')),
+                sourceFile));
     return new FactoryGenerator(map);
   }
 }

packages/compiler-cli/src/ngtsc/shims/src/host.ts

@@ -7,12 +7,13 @@
  */
 
 import * as ts from 'typescript';
+import {AbsoluteFsPath} from '../../path/src/types';
 
 export interface ShimGenerator {
   /**
    * Returns `true` if this generator is intended to handle the given file.
    */
-  recognize(fileName: string): boolean;
+  recognize(fileName: AbsoluteFsPath): boolean;
 
   /**
    * Generate a shim's `ts.SourceFile` for the given original file.
@@ -22,8 +23,8 @@ export interface ShimGenerator {
    *
    * If `generate` returns null, then the shim generator declines to generate the file after all.
    */
-  generate(genFileName: string, readFile: (fileName: string) => ts.SourceFile | null): ts.SourceFile
-      |null;
+  generate(genFileName: AbsoluteFsPath, readFile: (fileName: string) => ts.SourceFile | null):
+      ts.SourceFile|null;
 }
 
 /**
@@ -60,14 +61,16 @@ export class GeneratedShimsHostWrapper implements ts.CompilerHost {
       shouldCreateNewSourceFile?: boolean|undefined): ts.SourceFile|undefined {
     for (let i = 0; i < this.shimGenerators.length; i++) {
       const generator = this.shimGenerators[i];
-      if (generator.recognize(fileName)) {
+      // TypeScript internal paths are guaranteed to be POSIX-like absolute file paths.
+      const absoluteFsPath = AbsoluteFsPath.fromUnchecked(fileName);
+      if (generator.recognize(absoluteFsPath)) {
         const readFile = (originalFile: string) => {
           return this.delegate.getSourceFile(
                      originalFile, languageVersion, onError, shouldCreateNewSourceFile) ||
               null;
         };
 
-        return generator.generate(fileName, readFile) || undefined;
+        return generator.generate(absoluteFsPath, readFile) || undefined;
       }
     }
     return this.delegate.getSourceFile(
@@ -98,10 +101,13 @@ export class GeneratedShimsHostWrapper implements ts.CompilerHost {
   getNewLine(): string { return this.delegate.getNewLine(); }
 
   fileExists(fileName: string): boolean {
-    // Consider the file as existing whenever 1) it really does exist in the delegate host, or
-    // 2) at least one of the shim generators recognizes it.
+    // Consider the file as existing whenever
+    //  1) it really does exist in the delegate host, or
+    //  2) at least one of the shim generators recognizes it
+    // Note that we can pass the file name as branded absolute fs path because TypeScript
+    // internally only passes POSIX-like paths.
     return this.delegate.fileExists(fileName) ||
-        this.shimGenerators.some(gen => gen.recognize(fileName));
+        this.shimGenerators.some(gen => gen.recognize(AbsoluteFsPath.fromUnchecked(fileName)));
   }
 
   readFile(fileName: string): string|undefined { return this.delegate.readFile(fileName); }

packages/compiler-cli/src/ngtsc/shims/src/summary_generator.ts

@@ -8,21 +8,21 @@
 
 import * as ts from 'typescript';
 
-import {normalizeSeparators} from '../../util/src/path';
+import {AbsoluteFsPath} from '../../path/src/types';
 import {isNonDeclarationTsPath} from '../../util/src/typescript';
 
 import {ShimGenerator} from './host';
 import {generatedModuleName} from './util';
 
 export class SummaryGenerator implements ShimGenerator {
-  private constructor(private map: Map<string, string>) {}
+  private constructor(private map: Map<AbsoluteFsPath, string>) {}
 
   getSummaryFileNames(): string[] { return Array.from(this.map.keys()); }
 
-  recognize(fileName: string): boolean { return this.map.has(fileName); }
+  recognize(fileName: AbsoluteFsPath): boolean { return this.map.has(fileName); }
 
-  generate(genFilePath: string, readFile: (fileName: string) => ts.SourceFile | null): ts.SourceFile
-      |null {
+  generate(genFilePath: AbsoluteFsPath, readFile: (fileName: string) => ts.SourceFile | null):
+      ts.SourceFile|null {
     const originalPath = this.map.get(genFilePath) !;
     const original = readFile(originalPath);
     if (original === null) {
@@ -77,11 +77,13 @@ export class SummaryGenerator implements ShimGenerator {
     return genFile;
   }
 
-  static forRootFiles(files: ReadonlyArray<string>): SummaryGenerator {
-    const map = new Map<string, string>();
+  static forRootFiles(files: ReadonlyArray<AbsoluteFsPath>): SummaryGenerator {
+    const map = new Map<AbsoluteFsPath, string>();
     files.filter(sourceFile => isNonDeclarationTsPath(sourceFile))
-        .map(sourceFile => normalizeSeparators(sourceFile))
-        .forEach(sourceFile => map.set(sourceFile.replace(/\.ts$/, '.ngsummary.ts'), sourceFile));
+        .forEach(
+            sourceFile => map.set(
+                AbsoluteFsPath.fromUnchecked(sourceFile.replace(/\.ts$/, '.ngsummary.ts')),
+                sourceFile));
     return new SummaryGenerator(map);
   }
 }

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -2669,6 +2669,21 @@ describe('ngtsc behavioral tests', () => {
       expect(jsContents).toContain('export * from \'./test\';');
     });
 
+    it('should determine the flat module entry-point within multiple root files', () => {
+      env.tsconfig({
+        'flatModuleOutFile': 'flat.js',
+      });
+      env.write('ignored.ts', 'export const TEST = "this is ignored";');
+      env.write('index.ts', 'export const ENTRY = "this is the entry";');
+
+      env.driveMain();
+      const jsContents = env.getContents('flat.js');
+      expect(jsContents)
+          .toContain(
+              'export * from \'./index\';',
+              'Should detect the "index.ts" file as flat module entry-point.');
+    });
+
     it('should generate a flat module with an id', () => {
       env.tsconfig({
         'flatModuleOutFile': 'flat.js',