From fbc12d521e6e516a2838d25ddbfc42b5873e6911 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:56:46 -0400 Subject: [PATCH] refactor(@angular/build): isolate TypeScript diagnostics and AST caching in TypeScriptCompilation Decouple the AngularCompilation base class from TypeScript diagnostic structures and types, and encapsulate TypeScript-specific compilation logic in an intermediate TypeScriptCompilation class. TypeScriptCompilation extends AngularCompilation to manage the ts.SourceFile AST cache, file invalidation, and TypeScript diagnostic collection and conversion. This allows AngularCompilation.diagnoseFiles to return an empty diagnostics result by default, removing collectDiagnostics, static loadTypescript, and typescript imports from AngularCompilation entirely. AotCompilation and JitCompilation now extend TypeScriptCompilation, sharing unified AST caching and file invalidation logic, while NoopCompilation and ParallelCompilation no longer require dummy collectDiagnostics stubs. In addition, diagnostics.ts is relocated from tools/esbuild/angular/ into tools/angular/compilation/ so TypeScript diagnostic formatting remains strictly encapsulated within the compilation subsystem. --- .../compilation/angular-compilation.ts | 45 +--------- .../compilation/angular-compilation_spec.ts | 86 ++++++++++++++++++- .../angular/compilation/aot-compilation.ts | 17 ++-- .../compilation}/diagnostics.ts | 0 .../src/tools/angular/compilation/index.ts | 1 + .../angular/compilation/jit-compilation.ts | 17 +--- .../angular/compilation/noop-compilation.ts | 4 - .../compilation/parallel-compilation.ts | 8 -- .../compilation/typescript-compilation.ts | 57 ++++++++++++ 9 files changed, 152 insertions(+), 83 deletions(-) rename packages/angular/build/src/tools/{esbuild/angular => angular/compilation}/diagnostics.ts (100%) create mode 100644 packages/angular/build/src/tools/angular/compilation/typescript-compilation.ts diff --git a/packages/angular/build/src/tools/angular/compilation/angular-compilation.ts b/packages/angular/build/src/tools/angular/compilation/angular-compilation.ts index 4c17bb6e60de..308b0d255df3 100644 --- a/packages/angular/build/src/tools/angular/compilation/angular-compilation.ts +++ b/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,7 +49,6 @@ export enum DiagnosticModes { export abstract class AngularCompilation { static #angularCompilerCliModule?: typeof ng; - static #typescriptModule?: typeof ts; static async loadCompilerCli(): Promise { AngularCompilation.#angularCompilerCliModule ??= await import('@angular/compiler-cli'); @@ -59,12 +56,6 @@ export abstract class AngularCompilation { return AngularCompilation.#angularCompilerCliModule; } - static async loadTypescript(): Promise { - AngularCompilation.#typescriptModule ??= await import('typescript'); - - return AngularCompilation.#typescriptModule; - } - protected async loadConfiguration(tsconfig: string): Promise { const { readConfiguration } = await AngularCompilation.loadCompilerCli(); @@ -100,40 +91,10 @@ export abstract class AngularCompilation { transformFile?(filename: string, content: string): Promise; - protected collectDiagnostics?( - modes: DiagnosticModes, - ): Iterable | Promise>; - 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): Promise; diff --git a/packages/angular/build/src/tools/angular/compilation/angular-compilation_spec.ts b/packages/angular/build/src/tools/angular/compilation/angular-compilation_spec.ts index a9e93b701e5f..66d21cb52f51 100644 --- a/packages/angular/build/src/tools/angular/compilation/angular-compilation_spec.ts +++ b/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 { + return { compilerOptions: {}, referencedFiles: [] }; + } + + protected override *collectDiagnostics(modes: DiagnosticModes): Iterable { + 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 { + 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', () => { diff --git a/packages/angular/build/src/tools/angular/compilation/aot-compilation.ts b/packages/angular/build/src/tools/angular/compilation/aot-compilation.ts index 42df6a40e778..99f6da756930 100644 --- a/packages/angular/build/src/tools/angular/compilation/aot-compilation.ts +++ b/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(); 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): Promise { - for (const file of files) { - this.#sourceFiles.delete(toPosixPath(file)); - } - } } function findAffectedFiles( diff --git a/packages/angular/build/src/tools/esbuild/angular/diagnostics.ts b/packages/angular/build/src/tools/angular/compilation/diagnostics.ts similarity index 100% rename from packages/angular/build/src/tools/esbuild/angular/diagnostics.ts rename to packages/angular/build/src/tools/angular/compilation/diagnostics.ts diff --git a/packages/angular/build/src/tools/angular/compilation/index.ts b/packages/angular/build/src/tools/angular/compilation/index.ts index 213f55cac326..736adea60682 100644 --- a/packages/angular/build/src/tools/angular/compilation/index.ts +++ b/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'; diff --git a/packages/angular/build/src/tools/angular/compilation/jit-compilation.ts b/packages/angular/build/src/tools/angular/compilation/jit-compilation.ts index 955c90502cb0..e4e371a05df8 100644 --- a/packages/angular/build/src/tools/angular/compilation/jit-compilation.ts +++ b/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(); 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): Promise { - for (const file of files) { - this.#sourceFiles.delete(toPosixPath(file)); - } - } } diff --git a/packages/angular/build/src/tools/angular/compilation/noop-compilation.ts b/packages/angular/build/src/tools/angular/compilation/noop-compilation.ts index a2bb722d10a8..55c5913dbad4 100644 --- a/packages/angular/build/src/tools/angular/compilation/noop-compilation.ts +++ b/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.'); } diff --git a/packages/angular/build/src/tools/angular/compilation/parallel-compilation.ts b/packages/angular/build/src/tools/angular/compilation/parallel-compilation.ts index f9a504ee6fc0..7fc530161789 100644 --- a/packages/angular/build/src/tools/angular/compilation/parallel-compilation.ts +++ b/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[] }> { diff --git a/packages/angular/build/src/tools/angular/compilation/typescript-compilation.ts b/packages/angular/build/src/tools/angular/compilation/typescript-compilation.ts new file mode 100644 index 000000000000..7591e2028c36 --- /dev/null +++ b/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(); + + protected invalidateFiles(files: Iterable): void { + for (const file of files) { + this.sourceFiles.delete(toPosixPath(file)); + } + } + + override async update(files: Set): Promise { + this.invalidateFiles(files); + } + + protected abstract collectDiagnostics( + modes: DiagnosticModes, + ): Iterable | Promise>; + + 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; + } +}