perf(@ngtools/webpack): only check affected files for Angular semantic diagnostics

This change improves the performance of incremental type checking of Angular templates by reducing the number of calls to retrieve the diagnostics.
Only the set of affected files will be queried on a rebuild. The affected set includes files TypeScript deems affected, files that
are required to be emitted by the Angular compiler, and the original file for any TTC shim file that TypeScript deems affected.

packages/angular_devkit/build_angular/src/browser/tests/behavior/rebuild-errors_spec.ts

@@ -5,6 +5,7 @@
  * 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
  */
+// tslint:disable: no-big-function
 import { logging } from '@angular-devkit/core';
 import { concatMap, count, take, timeout } from 'rxjs/operators';
 import { buildWebpackBrowser } from '../../index';
@@ -12,6 +13,270 @@ import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup';
 
 describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => {
   describe('Behavior: "Rebuild Error"', () => {
+
+    it('detects template errors with no AOT codegen or TS emit differences', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        aot: true,
+        watch: true,
+      });
+
+      const goodDirectiveContents = `
+        import { Directive, Input } from '@angular/core';
+        @Directive({ selector: 'dir' })
+        export class Dir {
+          @Input() foo: number;
+        }
+      `;
+
+      const typeErrorText = `Type 'number' is not assignable to type 'string'.`;
+
+      // Create a directive and add to application
+      await harness.writeFile('src/app/dir.ts', goodDirectiveContents);
+      await harness.writeFile('src/app/app.module.ts', `
+        import { NgModule } from '@angular/core';
+        import { BrowserModule } from '@angular/platform-browser';
+        import { AppComponent } from './app.component';
+        import { Dir } from './dir';
+        @NgModule({
+          declarations: [
+            AppComponent,
+            Dir,
+          ],
+          imports: [
+            BrowserModule
+          ],
+          providers: [],
+          bootstrap: [AppComponent]
+        })
+        export class AppModule { }
+      `);
+
+      // Create app component that uses the directive
+      await harness.writeFile('src/app/app.component.ts', `
+        import { Component } from '@angular/core'
+        @Component({
+          selector: 'app-root',
+          template: '<dir [foo]="123">',
+        })
+        export class AppComponent { }
+      `);
+
+      const buildCount = await harness
+        .execute({ outputLogsOnFailure: false })
+        .pipe(
+          timeout(60000),
+          concatMap(async ({ result, logs }, index) => {
+            switch (index) {
+              case 0:
+                expect(result?.success).toBeTrue();
+
+                // Update directive to use a different input type for 'foo' (number -> string)
+                // Should cause a template error
+                await harness.writeFile('src/app/dir.ts', `
+                  import { Directive, Input } from '@angular/core';
+                  @Directive({ selector: 'dir' })
+                  export class Dir {
+                    @Input() foo: string;
+                  }
+                `);
+
+                break;
+              case 1:
+                expect(result?.success).toBeFalse();
+                expect(logs).toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Make an unrelated change to verify error cache was updated
+                // Should persist error in the next rebuild
+                await harness.modifyFile('src/main.ts', (content) => content + '\n');
+
+                break;
+              case 2:
+                expect(result?.success).toBeFalse();
+                expect(logs).toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Revert the directive change that caused the error
+                // Should remove the error
+                await harness.writeFile('src/app/dir.ts', goodDirectiveContents);
+
+                break;
+              case 3:
+                expect(result?.success).toBeTrue();
+                expect(logs).not.toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Make an unrelated change to verify error cache was updated
+                // Should continue showing no error
+                await harness.modifyFile('src/main.ts', (content) => content + '\n');
+
+                break;
+              case 4:
+                expect(result?.success).toBeTrue();
+                expect(logs).not.toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                break;
+            }
+          }),
+          take(5),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(5);
+    });
+
+    it('detects template errors with AOT codegen differences', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        aot: true,
+        watch: true,
+      });
+
+      const typeErrorText = `Type 'number' is not assignable to type 'string'.`;
+
+      // Create two directives and add to application
+      await harness.writeFile('src/app/dir.ts', `
+        import { Directive, Input } from '@angular/core';
+        @Directive({ selector: 'dir' })
+        export class Dir {
+          @Input() foo: number;
+        }
+      `);
+
+      // Same selector with a different type on the `foo` property but initially no `@Input`
+      const goodDirectiveContents = `
+        import { Directive } from '@angular/core';
+        @Directive({ selector: 'dir' })
+        export class Dir2 {
+          foo: string;
+        }
+      `;
+      await harness.writeFile('src/app/dir2.ts', goodDirectiveContents);
+
+      await harness.writeFile('src/app/app.module.ts', `
+        import { NgModule } from '@angular/core';
+        import { BrowserModule } from '@angular/platform-browser';
+        import { AppComponent } from './app.component';
+        import { Dir } from './dir';
+        import { Dir2 } from './dir2';
+        @NgModule({
+          declarations: [
+            AppComponent,
+            Dir,
+            Dir2,
+          ],
+          imports: [
+            BrowserModule
+          ],
+          providers: [],
+          bootstrap: [AppComponent]
+        })
+        export class AppModule { }
+      `);
+
+      // Create app component that uses the directive
+      await harness.writeFile('src/app/app.component.ts', `
+        import { Component } from '@angular/core'
+        @Component({
+          selector: 'app-root',
+          template: '<dir [foo]="123">',
+        })
+        export class AppComponent { }
+      `);
+
+      const buildCount = await harness
+        .execute({ outputLogsOnFailure: false })
+        .pipe(
+          timeout(60000),
+          concatMap(async ({ result, logs }, index) => {
+            switch (index) {
+              case 0:
+                expect(result?.success).toBeTrue();
+
+                // Update second directive to use string property `foo` as an Input
+                // Should cause a template error
+                await harness.writeFile('src/app/dir2.ts', `
+                  import { Directive, Input } from '@angular/core';
+                  @Directive({ selector: 'dir' })
+                  export class Dir2 {
+                    @Input() foo: string;
+                  }
+                `);
+
+                break;
+              case 1:
+                expect(result?.success).toBeFalse();
+                expect(logs).toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Make an unrelated change to verify error cache was updated
+                // Should persist error in the next rebuild
+                await harness.modifyFile('src/main.ts', (content) => content + '\n');
+
+                break;
+              case 2:
+                expect(result?.success).toBeFalse();
+                expect(logs).toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Revert the directive change that caused the error
+                // Should remove the error
+                await harness.writeFile('src/app/dir2.ts', goodDirectiveContents);
+
+                break;
+              case 3:
+                expect(result?.success).toBeTrue();
+                expect(logs).not.toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                // Make an unrelated change to verify error cache was updated
+                // Should continue showing no error
+                await harness.modifyFile('src/main.ts', (content) => content + '\n');
+
+                break;
+              case 4:
+                expect(result?.success).toBeTrue();
+                expect(logs).not.toContain(
+                  jasmine.objectContaining<logging.LogEntry>({
+                    message: jasmine.stringMatching(typeErrorText),
+                  }),
+                );
+
+                break;
+            }
+          }),
+          take(5),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(5);
+    });
+
     it('recovers from component stylesheet error', async () => {
       harness.useTarget('build', {
         ...BASE_OPTIONS,

packages/angular_devkit/build_angular/test/hello-world-app/tsconfig.json

@@ -20,6 +20,7 @@
   },
   "angularCompilerOptions": {
     "enableIvy": true,
-    "disableTypeScriptVersionCheck": true
+    "disableTypeScriptVersionCheck": true,
+    "strictTemplates": true
   }
 }

packages/ngtools/webpack/src/ivy/cache.ts

@@ -9,6 +9,8 @@ import * as ts from 'typescript';
 import { normalizePath } from './paths';
 
 export class SourceFileCache extends Map<string, ts.SourceFile> {
+  private readonly angularDiagnostics = new Map<ts.SourceFile, ts.Diagnostic[]>();
+
   invalidate(
     fileTimestamps: Map<string, 'ignore' | number | { safeTime: number } | null>,
     buildTimestamp: number,
@@ -29,11 +31,27 @@ export class SourceFileCache extends Map<string, ts.SourceFile> {
       if (!time || time >= buildTimestamp) {
         // Cache stores paths using the POSIX directory separator
         const normalizedFile = normalizePath(file);
-        this.delete(normalizedFile);
+        const sourceFile = this.get(normalizedFile);
+        if (sourceFile) {
+          this.delete(normalizedFile);
+          this.angularDiagnostics.delete(sourceFile);
+        }
         changedFiles.add(normalizedFile);
       }
     }
 
     return changedFiles;
   }
+
+  updateAngularDiagnostics(sourceFile: ts.SourceFile, diagnostics: ts.Diagnostic[]): void {
+    if (diagnostics.length > 0) {
+      this.angularDiagnostics.set(sourceFile, diagnostics);
+    } else {
+      this.angularDiagnostics.delete(sourceFile);
+    }
+  }
+
+  getAngularDiagnostics(sourceFile: ts.SourceFile): ts.Diagnostic[] | undefined {
+    return this.angularDiagnostics.get(sourceFile);
+  }
 }

packages/ngtools/webpack/src/ivy/plugin.ts

@@ -9,12 +9,7 @@ import { CompilerHost, CompilerOptions, readConfiguration } from '@angular/compi
 import { NgtscProgram } from '@angular/compiler-cli/src/ngtsc/program';
 import { createHash } from 'crypto';
 import * as ts from 'typescript';
-import {
-  Compiler,
-  NormalModuleReplacementPlugin,
-  WebpackFourCompiler,
-  compilation,
-} from 'webpack';
+import { Compiler, NormalModuleReplacementPlugin, WebpackFourCompiler, compilation } from 'webpack';
 import { NgccProcessor } from '../ngcc_processor';
 import { TypeScriptPathsPlugin } from '../paths-plugin';
 import { WebpackResourceLoader } from '../resource_loader';
@@ -36,6 +31,13 @@ import { AngularPluginSymbol, EmitFileResult, FileEmitter } from './symbol';
 import { InputFileSystemSync, createWebpackSystem } from './system';
 import { createAotTransformers, createJitTransformers, mergeTransformers } from './transformation';
 
+/**
+ * The threshold used to determine whether Angular file diagnostics should optimize for full programs
+ * or single files. If the number of affected files for a build is more than the threshold, full
+ * program optimization will be used.
+ */
+const DIAGNOSTICS_AFFECTED_THRESHOLD = 1;
+
 export interface AngularPluginOptions {
   tsconfig: string;
   compilerOptions?: CompilerOptions;
@@ -296,10 +298,7 @@ export class AngularWebpackPlugin {
     });
   }
 
-  private markResourceUsed(
-    normalizedResourcePath: string,
-    currentUnused: Set<string>,
-  ): void {
+  private markResourceUsed(normalizedResourcePath: string, currentUnused: Set<string>): void {
     if (!currentUnused.has(normalizedResourcePath)) {
       return;
     }
@@ -422,13 +421,37 @@ export class AngularWebpackPlugin {
     }
 
     // Update semantic diagnostics cache
+    const affectedFiles = new Set<ts.SourceFile>();
     while (true) {
-      const result = builder.getSemanticDiagnosticsOfNextAffectedFile(undefined, (sourceFile) =>
-        ignoreForDiagnostics.has(sourceFile),
-      );
+      const result = builder.getSemanticDiagnosticsOfNextAffectedFile(undefined, (sourceFile) => {
+        // If the affected file is a TTC shim, add the shim's original source file.
+        // This ensures that changes that affect TTC are typechecked even when the changes
+        // are otherwise unrelated from a TS perspective and do not result in Ivy codegen changes.
+        // For example, changing @Input property types of a directive used in another component's
+        // template.
+        if (
+          ignoreForDiagnostics.has(sourceFile) &&
+          sourceFile.fileName.endsWith('.ngtypecheck.ts')
+        ) {
+          // This file name conversion relies on internal compiler logic and should be converted
+          // to an official method when available. 15 is length of `.ngtypecheck.ts`
+          const originalFilename = sourceFile.fileName.slice(0, -15) + '.ts';
+          const originalSourceFile = builder.getSourceFile(originalFilename);
+          if (originalSourceFile) {
+            affectedFiles.add(originalSourceFile);
+          }
+
+          return true;
+        }
+
+        return false;
+      });
+
       if (!result) {
         break;
       }
+
+      affectedFiles.add(result.affected as ts.SourceFile);
     }
 
     // Collect non-semantic diagnostics
@@ -468,35 +491,55 @@ export class AngularWebpackPlugin {
       this.requiredFilesToEmit.clear();
 
       for (const sourceFile of builder.getSourceFiles()) {
-        // Collect Angular template diagnostics
-        if (!ignoreForDiagnostics.has(sourceFile)) {
-          // The below check should be removed once support for compiler 11.0 is dropped.
-          // Also, the below require should be changed to an ES6 import.
-          if (angularCompiler.getDiagnosticsForFile) {
-            // @angular/compiler-cli 11.1+
-            const { OptimizeFor } = require('@angular/compiler-cli/src/ngtsc/typecheck/api');
-            diagnosticsReporter(
-              angularCompiler.getDiagnosticsForFile(sourceFile, OptimizeFor.WholeProgram),
-            );
-          } else {
-            // @angular/compiler-cli 11.0+
-            const getDiagnostics = angularCompiler.getDiagnostics as (
-              sourceFile: ts.SourceFile,
-            ) => ts.Diagnostic[];
-            diagnosticsReporter(getDiagnostics.call(angularCompiler, sourceFile));
-          }
+        if (sourceFile.isDeclarationFile) {
+          continue;
         }
 
         // Collect sources that are required to be emitted
         if (
-          !sourceFile.isDeclarationFile &&
           !ignoreForEmit.has(sourceFile) &&
           !angularCompiler.incrementalDriver.safeToSkipEmit(sourceFile)
         ) {
           this.requiredFilesToEmit.add(normalizePath(sourceFile.fileName));
+
+          // If required to emit, diagnostics may have also changed
+          if (!ignoreForDiagnostics.has(sourceFile)) {
+            affectedFiles.add(sourceFile);
+          }
+        } else if (
+          this.sourceFileCache &&
+          !affectedFiles.has(sourceFile) &&
+          !ignoreForDiagnostics.has(sourceFile)
+        ) {
+          // Use cached Angular diagnostics for unchanged and unaffected files
+          const angularDiagnostics = this.sourceFileCache.getAngularDiagnostics(sourceFile);
+          if (angularDiagnostics) {
+            diagnosticsReporter(angularDiagnostics);
+          }
         }
       }
 
+      // Collect new Angular diagnostics for files affected by changes
+      const { OptimizeFor } = require('@angular/compiler-cli/src/ngtsc/typecheck/api');
+      const optimizeDiagnosticsFor =
+        affectedFiles.size <= DIAGNOSTICS_AFFECTED_THRESHOLD
+          ? OptimizeFor.SingleFile
+          : OptimizeFor.WholeProgram;
+      for (const affectedFile of affectedFiles) {
+        const angularDiagnostics = angularCompiler.getDiagnosticsForFile(
+          affectedFile,
+          optimizeDiagnosticsFor,
+        );
+        diagnosticsReporter(angularDiagnostics);
+        this.sourceFileCache?.updateAngularDiagnostics(affectedFile, angularDiagnostics);
+      }
+
+      // NOTE: Workaround to fix stale reuse program. Can be removed once fixed upstream.
+      // tslint:disable-next-line: no-any
+      (angularProgram as any).reuseTsProgram =
+        // tslint:disable-next-line: no-any
+        angularCompiler?.getNextProgram() || (angularCompiler as any)?.getCurrentProgram();
+
       return this.createFileEmitter(
         builder,
         mergeTransformers(angularCompiler.prepareEmit().transformers, transformers),