fix(ivy): track changes across failed builds (#33971)

Previously, our incremental build system kept track of the changes between
the current compilation and the previous one, and used its knowledge of
inter-file dependencies to evaluate the impact of each change and emit the
right set of output files.

However, a problem arose if the compiler was not able to extract a
dependency graph successfully. This typically happens if the input program
contains errors. In this case the Angular analysis part of compilation is
never executed.

If a file changed in one of these failed builds, in the next build it
appears unchanged. This means that the compiler "forgets" to emit it!

To fix this problem, the compiler needs to know the set of changes made
_since the last successful build_, not simply since the last invocation.

This commit changes the incremental state system to much more explicitly
pass information from the previous to the next compilation, and in the
process to keep track of changes across multiple failed builds, until the
program can be analyzed successfully and the results of those changes
incorporated into the emit plan.

Fixes #32214

PR Close #33971

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

@@ -6,4 +6,4 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export {IncrementalState} from './src/state';
+export {IncrementalDriver} from './src/state';

packages/compiler-cli/src/ngtsc/incremental/src/README.md

@@ -2,7 +2,19 @@
 
 This package contains logic related to incremental compilation in ngtsc.
 
-In particular, it tracks dependencies between `ts.SourceFile`s, so the compiler can make intelligent decisions about when it's safe to skip certain operations.
+In particular, it tracks dependencies between `ts.SourceFile`s, so the compiler can make intelligent decisions about when it's safe to skip certain operations. The main class performing this task is the `IncrementalDriver`.
+
+# What optimizations are made?
+
+ngtsc makes a decision to skip the emit of a file if it can prove that the contents of the file will not have changed. To prove this, two conditions must be true.
+
+* The input file itself must not have changed since the previous compilation.
+
+* None of the files on which the input file is dependent have changed since the previous compilation.
+
+The second condition is challenging to prove, as Angular allows statically evaluated expressions in lots of contexts that could result in changes from file to file. For example, the `name` of an `@Pipe` could be a reference to a constant in a different file. As part of analyzing the program, the compiler keeps track of such dependencies in order to answer this question.
+
+The emit of a file is the most expensive part of TypeScript/Angular compilation, so skipping emits when they are not necessary is one of the most valuable things the compiler can do to improve incremental build performance.
 
 # How does incremental compilation work?
 
@@ -14,21 +26,54 @@ This information is leveraged in two major ways:
 
 1) The previous `ts.Program` itself is used to create the next `ts.Program`, allowing TypeScript internally to leverage information from the previous compile in much the same way.
 
-2) An `IncrementalState` instance is constructed from the old and new `ts.Program`s.
+2) An `IncrementalDriver` instance is constructed from the old and new `ts.Program`s, and the previous program's `IncrementalDriver`.
 
-The compiler then proceeds normally, analyzing all of the Angular code within the program. As a part of this process, the compiler maps out all of the dependencies between files in the `IncrementalState`.
+The compiler then proceeds normally, analyzing all of the Angular code within the program. As a part of this process, the compiler maps out all of the dependencies between files in the `IncrementalDriver`.
 
-# What optimizations are made?
+## Determination of files to emit
 
-ngtsc makes a decision to skip the emit of a file if it can prove that the contents of the file will not have changed. To prove this, two conditions must be true.
+The principle question the incremental build system must answer is "which TS files need to be emitted for a given compilation?"
 
-* The input file itself must not have changed since the previous compilation.
+To determine whether an individual TS file needs to be emitted, the compiler must determine 3 things about the file:
 
-* None of the files on which the input file is dependent have changed since the previous compilation.
+1. Have its contents changed since the last time it was emitted?
+2. Has any resource file that the TS file depends on (like an HTML template) changed since the last time it was emitted?
+3. Have any of the dependencies of the TS file changed since the last time it was emitted?
 
-The second condition is challenging to prove, as Angular allows statically evaluated expressions in lots of contexts that could result in changes from file to file. For example, the `name` of an `@Pipe` could be a reference to a constant in a different file. As part of analyzing the program, the compiler keeps track of such dependencies in order to answer this question.
+If the answer to any of these questions is yes, then the TS file needs to be re-emitted.
 
-The emit of a file is the most expensive part of TypeScript/Angular compilation, so skipping emits when they are not necessary is one of the most valuable things the compiler can do to improve incremental build performance.
+## Tracking of changes
+
+On every invocation, the compiler receives (or can easily determine) several pieces of information:
+
+* The set of `ts.SourceFile`s that have changed since the last invocation.
+* The set of resources (`.html` files) that have changed since the last invocation.
+
+With this information, the compiler can perform rebuild optimizations:
+
+1. The compiler analyzes the full program and generates a dependency graph, which describes the relationships between files in the program.
+2. Based on this graph, the compiler can make a determination for each TS file whether it needs to be re-emitted or can safely be skipped. This produces a set called `pendingEmit` of every file which requires a re-emit.
+3. The compiler cycles through the files and emits those which are necessary, removing them from `pendingEmit`.
+
+Theoretically, after this process `pendingEmit` should be empty. As a precaution against errors which might happen in the future, `pendingEmit` is also passed into future compilations, so any files which previously were determined to need an emit (but have not been successfully produced yet) will be retried on subsequent compilations. This is mostly relevant if a client of `ngtsc` attempts to implement emit-on-error functionality.
+
+However, normally the execution of these steps requires a correct input program. In the presence of TypeScript errors, the compiler cannot perform this process. It might take many invocations for the user to fix all their TypeScript errors and reach a compilation that can be analyzed.
+
+As a result, the compiler must accumulate the set of these changes (to source files and resource files) from build to build until analysis can succeed.
+
+This accumulation happens via a type called `BuildState`. This type is a union of two possible states.
+
+### `PendingBuildState`
+
+This is the initial state of any build, and the final state of any unsuccessful build. This state tracks both `pendingEmit` files from the previous program as well as any source or resource files which have changed since the last successful analysis.
+
+If a new build starts and inherits from a failed build, it will merge the failed build's `PendingBuildState` into its own, including the sets of changed files.
+
+### `AnalyzedBuildState`
+
+After analysis is successfully performed, the compiler uses its dependency graph to evaluate the impact of any accumulated changes from the `PendingBuildState`, and updates `pendingEmit` with all of the pending files. At this point, the compiler transitions from a `PendingBuildState` to an `AnalyzedBuildState`, which only tracks `pendingEmit`. In `AnalyzedBuildState` this set is complete, and the raw changes can be forgotten.
+
+If a new build is started after a successful build, only `pendingEmit` from the `AnalyzedBuildState` needs to be merged into the new build's `PendingBuildState`.
 
 # What optimizations are possible in the future?
 

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

@@ -12,49 +12,156 @@ import {DependencyTracker} from '../../partial_evaluator';
 import {ResourceDependencyRecorder} from '../../util/src/resource_recorder';
 
 /**
- * Accumulates state between compilations.
+ * Drives an incremental build, by tracking changes and determining which files need to be emitted.
  */
-export class IncrementalState implements DependencyTracker, ResourceDependencyRecorder {
-  private constructor(
-      private unchangedFiles: Set<ts.SourceFile>,
-      private metadata: Map<ts.SourceFile, FileMetadata>,
-      private modifiedResourceFiles: Set<string>|null) {}
+export class IncrementalDriver implements DependencyTracker, ResourceDependencyRecorder {
+  /**
+   * State of the current build.
+   *
+   * This transitions as the compilation progresses.
+   */
+  private state: BuildState;
 
+  /**
+   * Tracks metadata related to each `ts.SourceFile` in the program.
+   */
+  private metadata = new Map<ts.SourceFile, FileMetadata>();
+
+  private constructor(state: PendingBuildState, private allTsFiles: Set<ts.SourceFile>) {
+    this.state = state;
+  }
+
+  /**
+   * Construct an `IncrementalDriver` with a starting state that incorporates the results of a
+   * previous build.
+   *
+   * The previous build's `BuildState` is reconciled with the new program's changes, and the results
+   * are merged into the new build's `PendingBuildState`.
+   */
   static reconcile(
-      oldProgram: ts.Program, newProgram: ts.Program,
-      modifiedResourceFiles: Set<string>|null): IncrementalState {
-    const unchangedFiles = new Set<ts.SourceFile>();
-    const metadata = new Map<ts.SourceFile, FileMetadata>();
+      oldProgram: ts.Program, oldDriver: IncrementalDriver, newProgram: ts.Program,
+      modifiedResourceFiles: Set<string>|null): IncrementalDriver {
+    // Initialize the state of the current build based on the previous one.
+    let state: PendingBuildState;
+    if (oldDriver.state.kind === BuildStateKind.Pending) {
+      // The previous build never made it past the pending state. Reuse it as the starting state for
+      // this build.
+      state = oldDriver.state;
+    } else {
+      // The previous build was successfully analyzed. `pendingEmit` is the only state carried
+      // forward into this build.
+      state = {
+        kind: BuildStateKind.Pending,
+        pendingEmit: oldDriver.state.pendingEmit,
+        changedResourcePaths: new Set<string>(),
+        changedTsPaths: new Set<string>(),
+      };
+    }
+
+    // Merge the freshly modified resource files with any prior ones.
+    if (modifiedResourceFiles !== null) {
+      for (const resFile of modifiedResourceFiles) {
+        state.changedResourcePaths.add(resFile);
+      }
+    }
+
+    // Next, process the files in the new program, with a couple of goals:
+    // 1) Determine which TS files have changed, if any, and merge them into `changedTsFiles`.
+    // 2) Produce a list of TS files which no longer exist in the program (they've been deleted
+    //    since the previous compilation). These need to be removed from the state tracking to avoid
+    //    leaking memory.
+
+    // All files in the old program, for easy detection of changes.
     const oldFiles = new Set<ts.SourceFile>(oldProgram.getSourceFiles());
 
-    // Compute the set of files that are unchanged (both in themselves and their dependencies).
+    // Assume all the old files were deleted to begin with. Only TS files are tracked.
+    const deletedTsPaths = new Set<string>(tsOnlyFiles(oldProgram).map(sf => sf.fileName));
+
     for (const newFile of newProgram.getSourceFiles()) {
-      if (newFile.isDeclarationFile && !oldFiles.has(newFile)) {
-        // Bail out and re-emit everything if a .d.ts file has changed - currently the compiler does
-        // not track dependencies into .d.ts files very well.
-        return IncrementalState.fresh();
-      } else if (oldFiles.has(newFile)) {
-        unchangedFiles.add(newFile);
+      if (!newFile.isDeclarationFile) {
+        // This file exists in the new program, so remove it from `deletedTsPaths`.
+        deletedTsPaths.delete(newFile.fileName);
+      }
+
+      if (oldFiles.has(newFile)) {
+        // This file hasn't changed; no need to look at it further.
+        continue;
       }
+
+      // The file has changed since the last successful build. The appropriate reaction depends on
+      // what kind of file it is.
+      if (!newFile.isDeclarationFile) {
+        // It's a .ts file, so track it as a change.
+        state.changedTsPaths.add(newFile.fileName);
+      } else {
+        // It's a .d.ts file. Currently the compiler does not do a great job of tracking
+        // dependencies on .d.ts files, so bail out of incremental builds here and do a full build.
+        // This usually only happens if something in node_modules changes.
+        return IncrementalDriver.fresh(newProgram);
+      }
+    }
+
+    // The last step is to remove any deleted files from the state.
+    for (const filePath of deletedTsPaths) {
+      state.pendingEmit.delete(filePath);
+
+      // Even if the file doesn't exist in the current compilation, it still might have been changed
+      // in a previous one, so delete it from the set of changed TS files, just in case.
+      state.changedTsPaths.delete(filePath);
     }
 
-    return new IncrementalState(unchangedFiles, metadata, modifiedResourceFiles);
+    // `state` now reflects the initial compilation state of the current
+    return new IncrementalDriver(state, new Set<ts.SourceFile>(tsOnlyFiles(newProgram)));
   }
 
-  static fresh(): IncrementalState {
-    return new IncrementalState(
-        new Set<ts.SourceFile>(), new Map<ts.SourceFile, FileMetadata>(), null);
+  static fresh(program: ts.Program): IncrementalDriver {
+    // Initialize the set of files which need to be emitted to the set of all TS files in the
+    // program.
+    const tsFiles = tsOnlyFiles(program);
+
+    const state: PendingBuildState = {
+      kind: BuildStateKind.Pending,
+      pendingEmit: new Set<string>(tsFiles.map(sf => sf.fileName)),
+      changedResourcePaths: new Set<string>(),
+      changedTsPaths: new Set<string>(),
+    };
+
+    return new IncrementalDriver(state, new Set(tsFiles));
   }
 
-  safeToSkip(sf: ts.SourceFile): boolean {
-    // It's safe to skip emitting a file if:
-    // 1) it hasn't changed
-    // 2) none if its resource dependencies have changed
-    // 3) none of its source dependencies have changed
-    return this.unchangedFiles.has(sf) && !this.hasChangedResourceDependencies(sf) &&
-        this.getFileDependencies(sf).every(dep => this.unchangedFiles.has(dep));
+  recordSuccessfulAnalysis(): void {
+    if (this.state.kind !== BuildStateKind.Pending) {
+      // Changes have already been incorporated.
+      return;
+    }
+
+    const pendingEmit = this.state.pendingEmit;
+
+    const state: PendingBuildState = this.state;
+
+    for (const sf of this.allTsFiles) {
+      // It's safe to skip emitting a file if:
+      // 1) it hasn't changed
+      // 2) none if its resource dependencies have changed
+      // 3) none of its source dependencies have changed
+      if (state.changedTsPaths.has(sf.fileName) || this.hasChangedResourceDependencies(sf) ||
+          this.getFileDependencies(sf).some(dep => state.changedTsPaths.has(dep.fileName))) {
+        // Something has changed which requires this file be re-emitted.
+        pendingEmit.add(sf.fileName);
+      }
+    }
+
+    // Update the state to an `AnalyzedBuildState`.
+    this.state = {
+      kind: BuildStateKind.Analyzed,
+      pendingEmit,
+    };
   }
 
+  recordSuccessfulEmit(sf: ts.SourceFile): void { this.state.pendingEmit.delete(sf.fileName); }
+
+  safeToSkipEmit(sf: ts.SourceFile): boolean { return !this.state.pendingEmit.has(sf.fileName); }
+
   trackFileDependency(dep: ts.SourceFile, src: ts.SourceFile) {
     const metadata = this.ensureMetadata(src);
     metadata.fileDependencies.add(dep);
@@ -87,12 +194,14 @@ export class IncrementalState implements DependencyTracker, ResourceDependencyRe
   }
 
   private hasChangedResourceDependencies(sf: ts.SourceFile): boolean {
-    if (this.modifiedResourceFiles === null || !this.metadata.has(sf)) {
+    if (!this.metadata.has(sf)) {
       return false;
     }
     const resourceDeps = this.metadata.get(sf) !.resourcePaths;
     return Array.from(resourceDeps.keys())
-        .some(resourcePath => this.modifiedResourceFiles !.has(resourcePath));
+        .some(
+            resourcePath => this.state.kind === BuildStateKind.Pending &&
+                this.state.changedResourcePaths.has(resourcePath));
   }
 }
 
@@ -104,3 +213,80 @@ class FileMetadata {
   fileDependencies = new Set<ts.SourceFile>();
   resourcePaths = new Set<string>();
 }
+
+
+type BuildState = PendingBuildState | AnalyzedBuildState;
+
+enum BuildStateKind {
+  Pending,
+  Analyzed,
+}
+
+interface BaseBuildState {
+  kind: BuildStateKind;
+
+  /**
+   * The heart of incremental builds. This `Set` tracks the set of files which need to be emitted
+   * during the current compilation.
+   *
+   * This starts out as the set of files which are still pending from the previous program (or the
+   * full set of .ts files on a fresh build).
+   *
+   * After analysis, it's updated to include any files which might have changed and need a re-emit
+   * as a result of incremental changes.
+   *
+   * If an emit happens, any written files are removed from the `Set`, as they're no longer pending.
+   *
+   * Thus, after compilation `pendingEmit` should be empty (on a successful build) or contain the
+   * files which still need to be emitted but have not yet been (due to errors).
+   *
+   * `pendingEmit` is tracked as as `Set<string>` instead of a `Set<ts.SourceFile>`, because the
+   * contents of the file are not important here, only whether or not the current version of it
+   * needs to be emitted. The `string`s here are TS file paths.
+   *
+   * See the README.md for more information on this algorithm.
+   */
+  pendingEmit: Set<string>;
+}
+
+/**
+ * State of a build before the Angular analysis phase completes.
+ */
+interface PendingBuildState extends BaseBuildState {
+  kind: BuildStateKind.Pending;
+
+  /**
+   * Set of files which are known to need an emit.
+   *
+   * Before the compiler's analysis phase completes, `pendingEmit` only contains files that were
+   * still pending after the previous build.
+   */
+  pendingEmit: Set<string>;
+
+  /**
+   * Set of TypeScript file paths which have changed since the last successfully analyzed build.
+   */
+  changedTsPaths: Set<string>;
+
+  /**
+   * Set of resource file paths which have changed since the last successfully analyzed build.
+   */
+  changedResourcePaths: Set<string>;
+}
+
+interface AnalyzedBuildState extends BaseBuildState {
+  kind: BuildStateKind.Analyzed;
+
+  /**
+   * Set of files which are known to need an emit.
+   *
+   * After analysis completes (that is, the state transitions to `AnalyzedBuildState`), the
+   * `pendingEmit` set takes into account any on-disk changes made since the last successfully
+   * analyzed build.
+   */
+  pendingEmit: Set<string>;
+}
+
+function tsOnlyFiles(program: ts.Program): ReadonlyArray<ts.SourceFile> {
+  return program.getSourceFiles().filter(sf => !sf.isDeclarationFile);
+}