fix(compiler-cli): inline type checking instructions no longer prevent incremental reuse (#42759)

Source files that contain directives or components that need an inline
type constructor or inline template type-check block would always be
considered as affected in incremental rebuilds. The inline operations
cause the source file to be updated in the TypeScript program that is
created for template type-checking, which becomes the reuse program
in a subsequent incremental rebuild.

In an incremental rebuild, the source files from the new user program
are compared to those from the reuse program. The updated source files
are not the same as the original source file from the user program, so
the incremental engine would mark the file which needed inline
operations as affected. This prevents incremental reuse for these files,
causing sub-optimal rebuild performance.

This commit attaches the original source file for source files that have
been updated with inline operations, such that the incremental engine
is able to compare source files using the original source file.

Fixes #42543

PR Close #42759

Author: JoostK

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

@@ -20,7 +20,7 @@ import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer
 import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata';
 import {PartialEvaluator} from '../../partial_evaluator';
 import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf';
-import {ProgramDriver, UpdateMode} from '../../program_driver';
+import {FileUpdate, ProgramDriver, UpdateMode} from '../../program_driver';
 import {DeclarationNode, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
 import {AdapterResourceLoader} from '../../resource';
 import {entryPointKeyFor, NgModuleRouteAnalyzer} from '../../routing';
@@ -1180,7 +1180,7 @@ class NotifyingProgramDriverWrapper implements ProgramDriver {
     return this.delegate.getProgram();
   }
 
-  updateFiles(contents: Map<AbsoluteFsPath, string>, updateMode: UpdateMode): void {
+  updateFiles(contents: Map<AbsoluteFsPath, FileUpdate>, updateMode: UpdateMode): void {
     this.delegate.updateFiles(contents, updateMode);
     this.notifyNewProgram(this.delegate.getProgram());
   }

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

@@ -15,6 +15,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/metadata",
         "//packages/compiler-cli/src/ngtsc/partial_evaluator",
         "//packages/compiler-cli/src/ngtsc/perf",
+        "//packages/compiler-cli/src/ngtsc/program_driver",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/scope",
         "//packages/compiler-cli/src/ngtsc/transform",

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

@@ -10,11 +10,13 @@ import * as ts from 'typescript';
 
 import {absoluteFromSourceFile, AbsoluteFsPath, resolve} from '../../file_system';
 import {PerfPhase, PerfRecorder} from '../../perf';
+import {MaybeSourceFileWithOriginalFile, NgOriginalFile} from '../../program_driver';
 import {ClassRecord, TraitCompiler} from '../../transform';
 import {FileTypeCheckingData} from '../../typecheck';
 import {toUnredirectedSourceFile} from '../../util/src/typescript';
 import {IncrementalBuild} from '../api';
 import {SemanticDepGraphUpdater} from '../semantic_graph';
+
 import {FileDependencyGraph} from './dependency_tracking';
 import {AnalyzedIncrementalState, DeltaIncrementalState, IncrementalState, IncrementalStateKind} from './state';
 
@@ -134,12 +136,12 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil
 
       const oldVersions = priorAnalysis.versions;
 
-      const oldFilesArray = oldProgram.getSourceFiles().map(sf => toUnredirectedSourceFile(sf));
+      const oldFilesArray = oldProgram.getSourceFiles().map(toOriginalSourceFile);
       const oldFiles = new Set(oldFilesArray);
       const deletedTsFiles = new Set(oldFilesArray.map(sf => absoluteFromSourceFile(sf)));
 
       for (const possiblyRedirectedNewFile of program.getSourceFiles()) {
-        const sf = toUnredirectedSourceFile(possiblyRedirectedNewFile);
+        const sf = toOriginalSourceFile(possiblyRedirectedNewFile);
         const sfPath = absoluteFromSourceFile(sf);
         // Since we're seeing a file in the incoming program with this name, it can't have been
         // deleted.
@@ -373,3 +375,28 @@ export class IncrementalCompilation implements IncrementalBuild<ClassRecord, Fil
     return this.step.priorState.emitted.has(sfPath);
   }
 }
+
+/**
+ * To accurately detect whether a source file was affected during an incremental rebuild, the
+ * "original" source file needs to be consistently used.
+ *
+ * First, TypeScript may have created source file redirects when declaration files of the same
+ * version of a library are included multiple times. The non-redirected source file should be used
+ * to detect changes, as otherwise the redirected source files cause a mismatch when compared to
+ * a prior program.
+ *
+ * Second, the program that is used for template type checking may contain mutated source files, if
+ * inline type constructors or inline template type-check blocks had to be used. Such source files
+ * store their original, non-mutated source file from the original program in a symbol. For
+ * computing the affected files in an incremental build this original source file should be used, as
+ * the mutated source file would always be considered affected.
+ */
+function toOriginalSourceFile(sf: ts.SourceFile): ts.SourceFile {
+  const unredirectedSf = toUnredirectedSourceFile(sf);
+  const originalFile = (unredirectedSf as MaybeSourceFileWithOriginalFile)[NgOriginalFile];
+  if (originalFile !== undefined) {
+    return originalFile;
+  } else {
+    return unredirectedSf;
+  }
+}

packages/compiler-cli/src/ngtsc/program_driver/src/api.ts

@@ -9,6 +9,29 @@
 import * as ts from 'typescript';
 import {AbsoluteFsPath} from '../../file_system';
 
+export interface FileUpdate {
+  /**
+   * The source file text.
+   */
+  newText: string;
+
+  /**
+   * Represents the source file from the original program that is being updated. If the file update
+   * targets a shim file then this is null, as shim files do not have an associated original file.
+   */
+  originalFile: ts.SourceFile|null;
+}
+
+export const NgOriginalFile = Symbol('NgOriginalFile');
+
+/**
+ * If an updated file has an associated original source file, then the original source file
+ * is attached to the updated file using the `NgOriginalFile` symbol.
+ */
+export interface MaybeSourceFileWithOriginalFile extends ts.SourceFile {
+  [NgOriginalFile]?: ts.SourceFile;
+}
+
 export interface ProgramDriver {
   /**
    * Whether this strategy supports modifying user files (inline modifications) in addition to
@@ -25,7 +48,7 @@ export interface ProgramDriver {
    * Incorporate a set of changes to either augment or completely replace the type-checking code
    * included in the type-checking program.
    */
-  updateFiles(contents: Map<AbsoluteFsPath, string>, updateMode: UpdateMode): void;
+  updateFiles(contents: Map<AbsoluteFsPath, FileUpdate>, updateMode: UpdateMode): void;
 
   /**
    * Retrieve a string version for a given `ts.SourceFile`, which much change when the contents of

packages/compiler-cli/src/ngtsc/program_driver/src/ts_create_program_driver.ts

@@ -12,7 +12,7 @@ import {AbsoluteFsPath} from '../../file_system';
 import {copyFileShimData, retagAllTsFiles, ShimReferenceTagger, untagAllTsFiles} from '../../shims';
 import {RequiredDelegations, toUnredirectedSourceFile} from '../../util/src/typescript';
 
-import {ProgramDriver, UpdateMode} from './api';
+import {FileUpdate, MaybeSourceFileWithOriginalFile, NgOriginalFile, ProgramDriver, UpdateMode} from './api';
 
 /**
  * Delegates all methods of `ts.CompilerHost` to a delegate, with the exception of
@@ -153,7 +153,7 @@ export class TsCreateProgramDriver implements ProgramDriver {
     return this.program;
   }
 
-  updateFiles(contents: Map<AbsoluteFsPath, string>, updateMode: UpdateMode): void {
+  updateFiles(contents: Map<AbsoluteFsPath, FileUpdate>, updateMode: UpdateMode): void {
     if (contents.size === 0) {
       // No changes have been requested. Is it safe to skip updating entirely?
       // If UpdateMode is Incremental, then yes. If UpdateMode is Complete, then it's safe to skip
@@ -169,8 +169,12 @@ export class TsCreateProgramDriver implements ProgramDriver {
       this.sfMap.clear();
     }
 
-    for (const [filePath, text] of contents.entries()) {
-      this.sfMap.set(filePath, ts.createSourceFile(filePath, text, ts.ScriptTarget.Latest, true));
+    for (const [filePath, {newText, originalFile}] of contents.entries()) {
+      const sf = ts.createSourceFile(filePath, newText, ts.ScriptTarget.Latest, true);
+      if (originalFile !== null) {
+        (sf as MaybeSourceFileWithOriginalFile)[NgOriginalFile] = originalFile;
+      }
+      this.sfMap.set(filePath, sf);
     }
 
     const host = new UpdatedProgramHost(

packages/compiler-cli/src/ngtsc/typecheck/src/context.ts

@@ -13,6 +13,7 @@ import * as ts from 'typescript';
 import {absoluteFromSourceFile, AbsoluteFsPath} from '../../file_system';
 import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports';
 import {PerfEvent, PerfRecorder} from '../../perf';
+import {FileUpdate} from '../../program_driver';
 import {ClassDeclaration, ReflectionHost} from '../../reflection';
 import {ImportManager} from '../../translator';
 import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
@@ -376,13 +377,16 @@ export class TypeCheckContextImpl implements TypeCheckContext {
     return code;
   }
 
-  finalize(): Map<AbsoluteFsPath, string> {
+  finalize(): Map<AbsoluteFsPath, FileUpdate> {
     // First, build the map of updates to source files.
-    const updates = new Map<AbsoluteFsPath, string>();
+    const updates = new Map<AbsoluteFsPath, FileUpdate>();
     for (const originalSf of this.opMap.keys()) {
       const newText = this.transform(originalSf);
       if (newText !== null) {
-        updates.set(absoluteFromSourceFile(originalSf), newText);
+        updates.set(absoluteFromSourceFile(originalSf), {
+          newText,
+          originalFile: originalSf,
+        });
       }
     }
 
@@ -399,8 +403,13 @@ export class TypeCheckContextImpl implements TypeCheckContext {
           path: pendingShimData.file.fileName,
           templates: pendingShimData.templates,
         });
-        updates.set(
-            pendingShimData.file.fileName, pendingShimData.file.render(false /* removeComments */));
+        const sfText = pendingShimData.file.render(false /* removeComments */);
+        updates.set(pendingShimData.file.fileName, {
+          newText: sfText,
+
+          // Shim files do not have an associated original file.
+          originalFile: null,
+        });
       }
     }
 

packages/compiler-cli/src/ngtsc/typecheck/test/program_spec.ts

@@ -10,7 +10,7 @@ import * as ts from 'typescript';
 
 import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
 import {runInEachFileSystem} from '../../file_system/testing';
-import {TsCreateProgramDriver, UpdateMode} from '../../program_driver';
+import {FileUpdate, TsCreateProgramDriver, UpdateMode} from '../../program_driver';
 import {sfExtensionData, ShimReferenceTagger} from '../../shims';
 import {expectCompleteReuse, makeProgram} from '../../testing';
 import {OptimizeFor} from '../api';
@@ -42,7 +42,8 @@ runInEachFileSystem(() => {
       // Update /main.ngtypecheck.ts without changing its shape. Verify that the old program was
       // reused completely.
       programStrategy.updateFiles(
-          new Map([[typecheckPath, 'export const VERSION = 2;']]), UpdateMode.Complete);
+          new Map([[typecheckPath, createUpdate('export const VERSION = 2;')]]),
+          UpdateMode.Complete);
 
       expectCompleteReuse(programStrategy.getProgram());
     });
@@ -54,13 +55,21 @@ runInEachFileSystem(() => {
       // Update /main.ts without changing its shape. Verify that the old program was reused
       // completely.
       programStrategy.updateFiles(
-          new Map([[mainPath, 'export const STILL_NOT_A_COMPONENT = true;']]), UpdateMode.Complete);
+          new Map([[mainPath, createUpdate('export const STILL_NOT_A_COMPONENT = true;')]]),
+          UpdateMode.Complete);
 
       expectCompleteReuse(programStrategy.getProgram());
     });
   });
 });
 
+function createUpdate(text: string): FileUpdate {
+  return {
+    newText: text,
+    originalFile: null,
+  };
+}
+
 function makeSingleFileProgramWithTypecheckShim(): {
   program: ts.Program,
   host: ts.CompilerHost,

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

@@ -895,6 +895,53 @@ runInEachFileSystem(() => {
           expect(diags[0].messageText)
               .toContain(`Type '"gamma"' is not assignable to type 'keyof Keys'.`);
         });
+
+        it('should not re-emit files that need inline type constructors', () => {
+          // Setup a directive that requires an inline type constructor, as it has a generic type
+          // parameter that refer an interface that has not been exported. The inline operation
+          // causes an updated dir.ts to be used in the type-check program, which should not
+          // confuse the incremental engine in undesirably considering dir.ts as affected in
+          // incremental rebuilds.
+          env.write('dir.ts', `
+            import {Directive, Input} from '@angular/core';
+            interface Keys {
+              alpha: string;
+              beta: string;
+            }
+            @Directive({
+              selector: '[dir]'
+            })
+            export class Dir<T extends keyof Keys> {
+              @Input() dir: T;
+            }
+          `);
+
+          env.write('cmp.ts', `
+            import {Component, NgModule} from '@angular/core';
+            import {Dir} from './dir';
+            @Component({
+              selector: 'test-cmp',
+              template: '<div dir="alpha"></div>',
+            })
+            export class Cmp {}
+            @NgModule({
+              declarations: [Cmp, Dir],
+            })
+            export class Module {}
+          `);
+          env.driveMain();
+
+          // Trigger a recompile by changing cmp.ts.
+          env.invalidateCachedFile('cmp.ts');
+
+          env.flushWrittenFileTracking();
+          env.driveMain();
+
+          // Verify that only cmp.ts was emitted, but not dir.ts as it was not affected.
+          const written = env.getFilesWrittenSinceLastFlush();
+          expect(written).toContain('/cmp.js');
+          expect(written).not.toContain('/dir.js');
+        });
       });
     });
   });

packages/language-service/ivy/language_service.ts

@@ -12,7 +12,7 @@ import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
 import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics';
 import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {PerfPhase} from '@angular/compiler-cli/src/ngtsc/perf';
-import {ProgramDriver} from '@angular/compiler-cli/src/ngtsc/program_driver';
+import {FileUpdate, ProgramDriver} from '@angular/compiler-cli/src/ngtsc/program_driver';
 import {isNamedClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection';
 import {TypeCheckShimGenerator} from '@angular/compiler-cli/src/ngtsc/typecheck';
 import {OptimizeFor} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
@@ -483,8 +483,8 @@ function createProgramDriver(project: ts.server.Project): ProgramDriver {
       }
       return program;
     },
-    updateFiles(contents: Map<AbsoluteFsPath, string>) {
-      for (const [fileName, newText] of contents) {
+    updateFiles(contents: Map<AbsoluteFsPath, FileUpdate>) {
+      for (const [fileName, {newText}] of contents) {
         const scriptInfo = getOrCreateTypeCheckScriptInfo(project, fileName);
         const snapshot = scriptInfo.getSnapshot();
         const length = snapshot.getLength();