refactor(@angular/build): isolate TypeScript diagnostics and AST caching in TypeScriptCompilation

packages/angular/build/src/tools/angular/compilation/angular-compilation.ts

@@ -8,9 +8,7 @@
 
 import type * as ng from '@angular/compiler-cli';
 import type { PartialMessage } from 'esbuild';
-import type ts from 'typescript';
-import { convertTypeScriptDiagnostic } from '../../esbuild/angular/diagnostics';
-import { profileAsync, profileSync } from '../../esbuild/profiling';
+import { profileSync } from '../../esbuild/profiling';
 import type { AngularHostOptions } from '../angular-host';
 
 export interface EmitFileResult {
@@ -51,20 +49,13 @@ export enum DiagnosticModes {
 
 export abstract class AngularCompilation {
   static #angularCompilerCliModule?: typeof ng;
-  static #typescriptModule?: typeof ts;
 
   static async loadCompilerCli(): Promise<typeof ng> {
     AngularCompilation.#angularCompilerCliModule ??= await import('@angular/compiler-cli');
 
     return AngularCompilation.#angularCompilerCliModule;
   }
 
-  static async loadTypescript(): Promise<typeof ts> {
-    AngularCompilation.#typescriptModule ??= await import('typescript');
-
-    return AngularCompilation.#typescriptModule;
-  }
-
   protected async loadConfiguration(tsconfig: string): Promise<ng.CompilerOptions> {
     const { readConfiguration } = await AngularCompilation.loadCompilerCli();
 
@@ -100,40 +91,10 @@ export abstract class AngularCompilation {
 
   transformFile?(filename: string, content: string): Promise<FileTransformResult | null>;
 
-  protected collectDiagnostics?(
-    modes: DiagnosticModes,
-  ): Iterable<ts.Diagnostic> | Promise<Iterable<ts.Diagnostic>>;
-
   async diagnoseFiles(
-    modes = DiagnosticModes.All,
+    modes?: DiagnosticModes,
   ): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {
-    if (!this.collectDiagnostics) {
-      return {};
-    }
-
-    const result: { errors?: PartialMessage[]; warnings?: PartialMessage[] } = {};
-
-    // Avoid loading typescript until actually needed.
-    // This allows for avoiding the load of typescript in the main thread when using the parallel compilation.
-    const typescript = await AngularCompilation.loadTypescript();
-
-    await profileAsync('NG_DIAGNOSTICS_TOTAL', async () => {
-      const diagnostics = await this.collectDiagnostics?.(modes);
-      if (!diagnostics) {
-        return;
-      }
-
-      for (const diagnostic of diagnostics) {
-        const message = convertTypeScriptDiagnostic(typescript, diagnostic);
-        if (diagnostic.category === typescript.DiagnosticCategory.Error) {
-          (result.errors ??= []).push(message);
-        } else {
-          (result.warnings ??= []).push(message);
-        }
-      }
-    });
-
-    return result;
+    return {};
   }
 
   update?(files: Set<string>): Promise<void>;

packages/angular/build/src/tools/angular/compilation/angular-compilation_spec.ts

@@ -6,11 +6,14 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
+import ts from 'typescript';
 import type { AngularHostOptions } from '../angular-host';
 import {
   AngularCompilation,
   AngularCompilationResult,
+  DiagnosticModes,
   NoopCompilation,
+  TypeScriptCompilation,
   createAngularCompilation,
 } from './index';
 
@@ -68,15 +71,90 @@ describe('AngularCompilation', () => {
       expect(result.compilerOptions['customOption']).toBe(true);
     });
 
-    it('throws when calling collectDiagnostics or emitAffectedFiles', () => {
+    it('throws when calling emitAffectedFiles', () => {
       const compilation = new NoopCompilation();
-      expect(() =>
-        (compilation as unknown as { collectDiagnostics(): unknown }).collectDiagnostics(),
-      ).toThrowError('Not available when using noop compilation.');
       expect(() => compilation.emitAffectedFiles()).toThrowError(
         'Not available when using noop compilation.',
       );
     });
+
+    it('returns empty diagnostics from diagnoseFiles', async () => {
+      const compilation = new NoopCompilation();
+      const diagnostics = await compilation.diagnoseFiles();
+      expect(diagnostics).toEqual({});
+    });
+  });
+
+  describe('TypeScriptCompilation', () => {
+    class MockTypeScriptCompilation extends TypeScriptCompilation {
+      async initialize(): Promise<AngularCompilationResult> {
+        return { compilerOptions: {}, referencedFiles: [] };
+      }
+
+      protected override *collectDiagnostics(modes: DiagnosticModes): Iterable<ts.Diagnostic> {
+        if (modes & DiagnosticModes.Option) {
+          yield {
+            category: ts.DiagnosticCategory.Error,
+            code: 1234,
+            messageText: 'Mock option error',
+            file: undefined,
+            start: undefined,
+            length: undefined,
+          };
+        }
+        if (modes & DiagnosticModes.Semantic) {
+          yield {
+            category: ts.DiagnosticCategory.Warning,
+            code: 5678,
+            messageText: 'Mock semantic warning',
+            file: undefined,
+            start: undefined,
+            length: undefined,
+          };
+        }
+      }
+
+      public getCachedSourceFiles(): Map<string, ts.SourceFile> {
+        return this.sourceFiles;
+      }
+    }
+
+    it('collects and converts diagnostics categorized by error and warning', async () => {
+      const compilation = new MockTypeScriptCompilation();
+      const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.All);
+
+      expect(diagnostics.errors?.length).toBe(1);
+      expect(diagnostics.errors?.[0].text).toContain('Mock option error');
+      expect(diagnostics.warnings?.length).toBe(1);
+      expect(diagnostics.warnings?.[0].text).toContain('Mock semantic warning');
+    });
+
+    it('filters diagnostics according to requested DiagnosticModes', async () => {
+      const compilation = new MockTypeScriptCompilation();
+      const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.Option);
+
+      expect(diagnostics.errors?.length).toBe(1);
+      expect(diagnostics.errors?.[0].text).toContain('Mock option error');
+      expect(diagnostics.warnings).toBeUndefined();
+    });
+
+    it('returns empty diagnostics immediately when mode is DiagnosticModes.None', async () => {
+      const compilation = new MockTypeScriptCompilation();
+      const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.None);
+
+      expect(diagnostics).toEqual({});
+    });
+
+    it('evicts files from AST cache on update and invalidateFiles', async () => {
+      const compilation = new MockTypeScriptCompilation();
+      const mockSourceFile = ts.createSourceFile('test.ts', '', ts.ScriptTarget.Latest);
+      compilation.getCachedSourceFiles().set('/src/test.ts', mockSourceFile);
+
+      expect(compilation.getCachedSourceFiles().has('/src/test.ts')).toBeTrue();
+
+      await compilation.update?.(new Set(['/src/test.ts']));
+      expect(compilation.getCachedSourceFiles().has('/src/test.ts')).toBeFalse();
+    });
   });
 
   describe('createAngularCompilation', () => {

packages/angular/build/src/tools/angular/compilation/aot-compilation.ts

@@ -11,7 +11,6 @@ import assert from 'node:assert';
 import { relative } from 'node:path';
 import ts from 'typescript';
 import { useTypeChecking } from '../../../utils/environment-options';
-import { toPosixPath } from '../../../utils/path';
 import { profileAsync, profileSync } from '../../esbuild/profiling';
 import {
   AngularHostOptions,
@@ -28,6 +27,7 @@ import {
   EmitFileResult,
 } from './angular-compilation';
 import { collectHmrCandidates } from './hmr-candidates';
+import { TypeScriptCompilation } from './typescript-compilation';
 import { printSourceFileWithMap } from './typescript-printer';
 
 /**
@@ -54,9 +54,8 @@ class AngularCompilationState {
   }
 }
 
-export class AotCompilation extends AngularCompilation {
+export class AotCompilation extends TypeScriptCompilation {
   #state?: AngularCompilationState;
-  readonly #sourceFiles = new Map<string, ts.SourceFile>();
 
   constructor(private readonly browserOnlyBuild: boolean) {
     super();
@@ -100,9 +99,9 @@ export class AotCompilation extends AngularCompilation {
     let staleSourceFiles;
     let clearPackageJsonCache = false;
     if (hostOptions.modifiedFiles) {
-      for (const modifiedFile of hostOptions.modifiedFiles) {
-        this.#sourceFiles.delete(toPosixPath(modifiedFile));
+      this.invalidateFiles(hostOptions.modifiedFiles);
 
+      for (const modifiedFile of hostOptions.modifiedFiles) {
         if (this.#state) {
           // Clear package.json cache if a node modules file was modified
           if (!clearPackageJsonCache && modifiedFile.includes('node_modules')) {
@@ -128,7 +127,7 @@ export class AotCompilation extends AngularCompilation {
       compilerOptions,
       hostOptions,
       packageJsonCache,
-      this.#sourceFiles,
+      this.sourceFiles,
     );
 
     // Create the Angular specific program that contains the Angular compiler
@@ -463,12 +462,6 @@ export class AotCompilation extends AngularCompilation {
 
     return emittedFiles.values();
   }
-
-  override async update(files: Set<string>): Promise<void> {
-    for (const file of files) {
-      this.#sourceFiles.delete(toPosixPath(file));
-    }
-  }
 }
 
 function findAffectedFiles(

packages/angular/build/src/tools/angular/compilation/index.ts

@@ -16,3 +16,4 @@ export {
 } from './angular-compilation';
 export { createAngularCompilation, type AngularCompilationMode } from './factory';
 export { NoopCompilation } from './noop-compilation';
+export { TypeScriptCompilation } from './typescript-compilation';

packages/angular/build/src/tools/angular/compilation/jit-compilation.ts

@@ -9,7 +9,6 @@
 import type * as ng from '@angular/compiler-cli';
 import assert from 'node:assert';
 import ts from 'typescript';
-import { toPosixPath } from '../../../utils/path';
 import { profileSync } from '../../esbuild/profiling';
 import { AngularHostOptions, createAngularCompilerHost } from '../angular-host';
 import { createJitResourceTransformer } from '../transformers/jit-resource-transformer';
@@ -21,6 +20,7 @@ import {
   DiagnosticModes,
   EmitFileResult,
 } from './angular-compilation';
+import { TypeScriptCompilation } from './typescript-compilation';
 
 class JitCompilationState {
   constructor(
@@ -32,9 +32,8 @@ class JitCompilationState {
   ) {}
 }
 
-export class JitCompilation extends AngularCompilation {
+export class JitCompilation extends TypeScriptCompilation {
   #state?: JitCompilationState;
-  readonly #sourceFiles = new Map<string, ts.SourceFile>();
 
   constructor(private readonly browserOnlyBuild: boolean) {
     super();
@@ -59,9 +58,7 @@ export class JitCompilation extends AngularCompilation {
       compilerOptionsTransformer?.(originalCompilerOptions) ?? originalCompilerOptions;
 
     if (hostOptions.modifiedFiles) {
-      for (const modifiedFile of hostOptions.modifiedFiles) {
-        this.#sourceFiles.delete(toPosixPath(modifiedFile));
-      }
+      this.invalidateFiles(hostOptions.modifiedFiles);
     }
 
     // Create Angular compiler host
@@ -70,7 +67,7 @@ export class JitCompilation extends AngularCompilation {
       compilerOptions,
       hostOptions,
       undefined,
-      this.#sourceFiles,
+      this.sourceFiles,
     );
 
     // Create the TypeScript Program
@@ -167,10 +164,4 @@ export class JitCompilation extends AngularCompilation {
 
     return emittedFiles;
   }
-
-  override async update(files: Set<string>): Promise<void> {
-    for (const file of files) {
-      this.#sourceFiles.delete(toPosixPath(file));
-    }
-  }
 }

packages/angular/build/src/tools/angular/compilation/noop-compilation.ts

@@ -24,10 +24,6 @@ export class NoopCompilation extends AngularCompilation {
     return { compilerOptions, referencedFiles: [] };
   }
 
-  protected override collectDiagnostics(): never {
-    throw new Error('Not available when using noop compilation.');
-  }
-
   override emitAffectedFiles(): never {
     throw new Error('Not available when using noop compilation.');
   }

packages/angular/build/src/tools/angular/compilation/parallel-compilation.ts

@@ -133,14 +133,6 @@ export class ParallelCompilation extends AngularCompilation {
     }
   }
 
-  /**
-   * This is not needed with this compilation type since the worker will already send a response
-   * with the serializable esbuild compatible diagnostics.
-   */
-  protected override collectDiagnostics(): never {
-    throw new Error('Not implemented in ParallelCompilation.');
-  }
-
   override async diagnoseFiles(
     modes = DiagnosticModes.All,
   ): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {

packages/angular/build/src/tools/angular/compilation/typescript-compilation.ts

@@ -0,0 +1,57 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import type { PartialMessage } from 'esbuild';
+import ts from 'typescript';
+import { toPosixPath } from '../../../utils/path';
+import { profileAsync } from '../../esbuild/profiling';
+import { AngularCompilation, DiagnosticModes } from './angular-compilation';
+import { convertTypeScriptDiagnostic } from './diagnostics';
+
+export abstract class TypeScriptCompilation extends AngularCompilation {
+  protected readonly sourceFiles = new Map<string, ts.SourceFile>();
+
+  protected invalidateFiles(files: Iterable<string>): void {
+    for (const file of files) {
+      this.sourceFiles.delete(toPosixPath(file));
+    }
+  }
+
+  override async update(files: Set<string>): Promise<void> {
+    this.invalidateFiles(files);
+  }
+
+  protected abstract collectDiagnostics(
+    modes: DiagnosticModes,
+  ): Iterable<ts.Diagnostic> | Promise<Iterable<ts.Diagnostic>>;
+
+  override async diagnoseFiles(
+    modes = DiagnosticModes.All,
+  ): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {
+    if (modes === DiagnosticModes.None) {
+      return {};
+    }
+
+    const result: { errors?: PartialMessage[]; warnings?: PartialMessage[] } = {};
+
+    await profileAsync('NG_DIAGNOSTICS_TOTAL', async () => {
+      const diagnostics = await this.collectDiagnostics(modes);
+
+      for (const diagnostic of diagnostics) {
+        const message = convertTypeScriptDiagnostic(ts, diagnostic);
+        if (diagnostic.category === ts.DiagnosticCategory.Error) {
+          (result.errors ??= []).push(message);
+        } else {
+          (result.warnings ??= []).push(message);
+        }
+      }
+    });
+
+    return result;
+  }
+}