From 8e343ea8d19cc2bdd873ae32f6f672c3438763d5 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:11:40 +0000 Subject: [PATCH] perf(@angular/build): optimize incremental watch rebuilds in AOT compilation - Pre-populate `.d.ts` shape signatures on the previous `BuilderProgram` for modified files prior to diffing when type-checking or TypeScript transpilation is active, preventing false-positive `.d.ts` cascades on incremental watch edits. - Synchronize `typeScriptProgram` with `angularCompiler.getCurrentProgram()` after `collectDiagnostics()` and clear uncommitted `changedFilesSet` and `oldSignatures` entries so subsequent incremental builds preserve `.d.ts` signatures and do not treat `.ngtypecheck.ts` shims as newly added files. - Detect affected dependent `.ngtypecheck.ts` shims in `isolatedModules` / `verbatimModuleSyntax` mode via TypeScript's `state.hasCalledUpdateShapeSignature`, ensuring cross-component template diagnostics are re-evaluated when a dependency's `.d.ts` shape changes without cascading on private edits. - Avoid `filter().flatMap()` allocations and skip `getResourceDependencies()` for declaration files when collecting `referencedFiles`. --- .../tests/behavior/rebuild-errors_spec.ts | 118 ++++++- .../angular/compilation/aot-compilation.ts | 331 ++++++++++++++---- 2 files changed, 373 insertions(+), 76 deletions(-) diff --git a/packages/angular/build/src/builders/application/tests/behavior/rebuild-errors_spec.ts b/packages/angular/build/src/builders/application/tests/behavior/rebuild-errors_spec.ts index de53c7223fff..f78a41d3d14e 100644 --- a/packages/angular/build/src/builders/application/tests/behavior/rebuild-errors_spec.ts +++ b/packages/angular/build/src/builders/application/tests/behavior/rebuild-errors_spec.ts @@ -15,12 +15,6 @@ import { expectNoLog, } from '../setup'; -/** - * Maximum time in milliseconds for single build/rebuild - * This accounts for CI variability. - */ -export const BUILD_TIMEOUT = 30_000; - describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { describe('Behavior: "Rebuild Error Detection"', () => { it('detects template errors with no AOT codegen or TS emit differences', async () => { @@ -128,6 +122,116 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { ); }); + it('detects template errors across components on watch rebuild when isolatedModules is enabled', async () => { + harness.useTarget('build', { + ...BASE_OPTIONS, + watch: true, + }); + + await harness.modifyFile('tsconfig.json', (content) => { + const tsconfig = JSON.parse(content); + tsconfig.compilerOptions = { + ...tsconfig.compilerOptions, + isolatedModules: true, + }; + + return JSON.stringify(tsconfig); + }); + + const goodChildComponentContents = ` + import { Component, Input } from '@angular/core'; + @Component({ + selector: 'child', + standalone: true, + template: '

{{ value }}

', + }) + export class ChildComponent { + @Input() value!: number; + } + `; + + const typeErrorText = `Type 'number' is not assignable to type 'string'.`; + + await harness.writeFiles({ + 'src/app/child.component.ts': goodChildComponentContents, + 'src/app/app.module.ts': ` + import { NgModule } from '@angular/core'; + import { BrowserModule } from '@angular/platform-browser'; + import { AppComponent } from './app.component'; + @NgModule({ + imports: [ + BrowserModule, + AppComponent, + ], + providers: [], + bootstrap: [AppComponent] + }) + export class AppModule { } + `, + 'src/app/app.component.ts': ` + import { Component } from '@angular/core'; + import { ChildComponent } from './child.component'; + @Component({ + selector: 'app-root', + standalone: true, + imports: [ChildComponent], + template: '', + }) + export class AppComponent {} + `, + }); + + await harness.executeWithCases( + [ + async ({ result }) => { + expect(result?.success).toBeTrue(); + + // Update child component to change input type from number to string + await harness.writeFile( + 'src/app/child.component.ts', + ` + import { Component, Input } from '@angular/core'; + @Component({ + selector: 'child', + standalone: true, + template: '

{{ value }}

', + }) + export class ChildComponent { + @Input() value!: string; + } + `, + ); + }, + async ({ result, logs }) => { + expect(result?.success).toBeFalse(); + expectLog(logs, typeErrorText); + + // Make an unrelated change to verify error persists + await harness.modifyFile('src/main.ts', (content) => content + '\n'); + }, + async ({ result, logs }) => { + expect(result?.success).toBeFalse(); + expectLog(logs, typeErrorText); + + // Revert back to number + await harness.writeFile('src/app/child.component.ts', goodChildComponentContents); + }, + async ({ result, logs }) => { + expect(result?.success).toBeTrue(); + expectNoLog(logs, typeErrorText); + + // Make an unrelated change to verify error cache cleared + await harness.modifyFile('src/main.ts', (content) => content + '\n'); + }, + ({ result, logs }) => { + expect(result?.success).toBeTrue(); + expectNoLog(logs, typeErrorText); + }, + ], + { outputLogsOnFailure: false }, + ); + }); + it('detects cumulative block syntax errors', async () => { harness.useTarget('build', { ...BASE_OPTIONS, @@ -234,7 +338,7 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { await harness.appendToFile('src/app/app.component.html', '
Guten Tag
'); }, ({ logs }) => { - expectNoLog(logs, 'invalid-css-content'); + expectNoLog(logs, 'Unexpected character "EOF"'); harness.expectFile('dist/browser/main.js').content.toContain('Hello, world!'); harness.expectFile('dist/browser/main.js').content.toContain('Guten Tag'); 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 e4047a3e3e1b..7a14135838d6 100644 --- a/packages/angular/build/src/tools/angular/compilation/aot-compilation.ts +++ b/packages/angular/build/src/tools/angular/compilation/aot-compilation.ts @@ -37,11 +37,35 @@ import { printSourceFileWithMap } from './typescript-printer'; */ const HMR_MODIFIED_FILE_LIMIT = 32; +/** + * Extension for Angular type-checking files. + */ +const TTC_EXTENSION = '.ngtypecheck.ts'; + +/** + * Length of the `.ngtypecheck.ts` extension. + */ +const TTC_EXTENSION_LENGTH = TTC_EXTENSION.length; + +/** + * Checks if a file is an Angular type-checking shim file. + */ +function isNgTypeCheckFile(fileName: string): boolean { + return fileName.endsWith(TTC_EXTENSION); +} + +/** + * Returns the original filename for a type-checking shim file. + */ +function getOriginalFilename(fileName: string): string { + return fileName.slice(0, -TTC_EXTENSION_LENGTH) + '.ts'; +} + class AngularCompilationState { constructor( public readonly angularProgram: ng.NgtscProgram, public readonly compilerHost: ng.CompilerHost, - public readonly typeScriptProgram: ts.EmitAndSemanticDiagnosticsBuilderProgram, + public typeScriptProgram: ts.EmitAndSemanticDiagnosticsBuilderProgram, public readonly affectedFiles: ReadonlySet, public readonly templateDiagnosticsOptimization: ng.OptimizeFor, public readonly webWorkerTransform: ts.TransformerFactory, @@ -102,8 +126,8 @@ export class AotCompilation extends TypeScriptCompilation { if (hostOptions.modifiedFiles) { this.invalidateFiles(hostOptions.modifiedFiles); - for (const modifiedFile of hostOptions.modifiedFiles) { - if (this.#state) { + if (this.#state && hostOptions.modifiedFiles.size > 0) { + for (const modifiedFile of hostOptions.modifiedFiles) { // Clear package.json cache if a node modules file was modified if (!clearPackageJsonCache && modifiedFile.includes('node_modules')) { clearPackageJsonCache = true; @@ -119,6 +143,14 @@ export class AotCompilation extends TypeScriptCompilation { } } } + + if (useTypeChecking || useTypeScriptTranspilation) { + populatePreviousDtsSignatures( + this.#state.typeScriptProgram, + this.#state.compilerHost, + hostOptions.modifiedFiles, + ); + } } } @@ -209,25 +241,46 @@ export class AotCompilation extends TypeScriptCompilation { const componentResourcesDependencies = new Map(); // Get all files referenced in the TypeScript/Angular program including component resources - const referencedFiles = typeScriptProgram - .getSourceFiles() - .filter((sourceFile) => !angularCompiler.ignoreForEmit.has(sourceFile)) - .flatMap((sourceFile) => { - const resourceDependencies = angularCompiler.getResourceDependencies(sourceFile); - componentResourcesDependencies.set(sourceFile.fileName, resourceDependencies); - // Also invalidate Angular diagnostics for a source file if component resources are modified - if (this.#state && hostOptions.modifiedFiles?.size) { - for (const resourceDependency of resourceDependencies) { - if (hostOptions.modifiedFiles.has(resourceDependency)) { - this.#state.diagnosticCache.delete(sourceFile); - // Also mark as affected in case changed template affects diagnostics - affectedFiles.add(sourceFile); - } + const referencedFiles: string[] = []; + for (const sourceFile of typeScriptProgram.getSourceFiles()) { + if (angularCompiler.ignoreForEmit.has(sourceFile)) { + continue; + } + + referencedFiles.push(sourceFile.fileName); + if (sourceFile.isDeclarationFile) { + continue; + } + + const resourceDependencies = angularCompiler.getResourceDependencies(sourceFile); + componentResourcesDependencies.set(sourceFile.fileName, resourceDependencies); + + if (resourceDependencies.length === 0) { + continue; + } + + referencedFiles.push(...resourceDependencies); + + // Also invalidate Angular diagnostics for a source file if component resources are modified + if (this.#state && hostOptions.modifiedFiles?.size) { + for (const resourceDependency of resourceDependencies) { + if (!hostOptions.modifiedFiles.has(resourceDependency)) { + continue; + } + + this.#state.diagnosticCache.delete(sourceFile); + + const oldSourceFile = this.#state.typeScriptProgram.getSourceFile(sourceFile.fileName); + if (oldSourceFile) { + this.#state.diagnosticCache.delete(oldSourceFile); } - } - return [sourceFile.fileName, ...resourceDependencies]; - }); + // Also mark as affected in case changed template affects diagnostics + affectedFiles.add(sourceFile); + break; + } + } + } this.#state = new AngularCompilationState( angularProgram, @@ -263,63 +316,81 @@ export class AotCompilation extends TypeScriptCompilation { const syntactic = modes & DiagnosticModes.Syntactic; const semantic = modes & DiagnosticModes.Semantic; - // Collect program level diagnostics - if (modes & DiagnosticModes.Option) { - yield* typeScriptProgram.getConfigFileParsingDiagnostics(); - yield* angularCompiler.getOptionDiagnostics(); - yield* typeScriptProgram.getOptionsDiagnostics(); - } - if (syntactic) { - yield* typeScriptProgram.getGlobalDiagnostics(); - } - - // Collect source file specific diagnostics - for (const sourceFile of typeScriptProgram.getSourceFiles()) { - if (angularCompiler.ignoreForDiagnostics.has(sourceFile)) { - continue; + try { + // Collect program level diagnostics + if (modes & DiagnosticModes.Option) { + yield* typeScriptProgram.getConfigFileParsingDiagnostics(); + yield* angularCompiler.getOptionDiagnostics(); + yield* typeScriptProgram.getOptionsDiagnostics(); } - if (syntactic) { - // TypeScript will use cached diagnostics for files that have not been - // changed or affected for this build when using incremental building. - yield* profileSync( - 'NG_DIAGNOSTICS_SYNTACTIC', - () => typeScriptProgram.getSyntacticDiagnostics(sourceFile), - true, - ); + yield* typeScriptProgram.getGlobalDiagnostics(); } - if (!semantic) { - continue; - } + // Collect source file specific diagnostics + for (const sourceFile of typeScriptProgram.getSourceFiles()) { + if (angularCompiler.ignoreForDiagnostics.has(sourceFile)) { + continue; + } - yield* profileSync( - 'NG_DIAGNOSTICS_SEMANTIC', - () => typeScriptProgram.getSemanticDiagnostics(sourceFile), - true, - ); + if (syntactic) { + // TypeScript will use cached diagnostics for files that have not been + // changed or affected for this build when using incremental building. + yield* profileSync( + 'NG_DIAGNOSTICS_SYNTACTIC', + () => typeScriptProgram.getSyntacticDiagnostics(sourceFile), + true, + ); + } - // Declaration files cannot have template diagnostics - if (sourceFile.isDeclarationFile) { - continue; - } + if (!semantic) { + continue; + } - // Only request Angular template diagnostics for affected files to avoid - // overhead of template diagnostics for unchanged files. - if (affectedFiles.has(sourceFile)) { - const angularDiagnostics = profileSync( - 'NG_DIAGNOSTICS_TEMPLATE', - () => angularCompiler.getDiagnosticsForFile(sourceFile, templateDiagnosticsOptimization), + yield* profileSync( + 'NG_DIAGNOSTICS_SEMANTIC', + () => typeScriptProgram.getSemanticDiagnostics(sourceFile), true, ); - diagnosticCache.set(sourceFile, angularDiagnostics); - yield* angularDiagnostics; - } else { - const angularDiagnostics = diagnosticCache.get(sourceFile); - if (angularDiagnostics) { + + // Declaration files cannot have template diagnostics + if (sourceFile.isDeclarationFile) { + continue; + } + + // Only request Angular template diagnostics for affected files to avoid + // overhead of template diagnostics for unchanged files. + if (affectedFiles.has(sourceFile)) { + const angularDiagnostics = profileSync( + 'NG_DIAGNOSTICS_TEMPLATE', + () => + angularCompiler.getDiagnosticsForFile(sourceFile, templateDiagnosticsOptimization), + true, + ); + diagnosticCache.set(sourceFile, angularDiagnostics); yield* angularDiagnostics; + } else { + const angularDiagnostics = diagnosticCache.get(sourceFile); + if (angularDiagnostics) { + yield* angularDiagnostics; + } } } + } finally { + // Angular's template typechecker lazily creates `.ngtypecheck.ts` shims in a new `ts.Program` + // during `getDiagnosticsForFile`. Sync the builder program with `angularCompiler.getCurrentProgram()` + // so the next incremental build does not treat all `.ngtypecheck.ts` files as newly added files. + const currentTsProgram = angularCompiler.getCurrentProgram(); + if (currentTsProgram !== typeScriptProgram.getProgram()) { + ensureSourceFileVersions(currentTsProgram); + this.#state.typeScriptProgram = ts.createEmitAndSemanticDiagnosticsBuilderProgram( + currentTsProgram, + this.#state.compilerHost, + typeScriptProgram, + ); + } + + commitBuilderProgramState(this.#state.typeScriptProgram); } } @@ -490,10 +561,10 @@ function findAffectedFiles( // For example, changing @Input property types of a directive used in another component's // template. // A TTC shim is a file that has been ignored for diagnostics and has a filename ending in `.ngtypecheck.ts`. - if (ignoreForDiagnostics.has(sourceFile) && sourceFile.fileName.endsWith('.ngtypecheck.ts')) { + if (ignoreForDiagnostics.has(sourceFile) && isNgTypeCheckFile(sourceFile.fileName)) { // 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 originalFilename = getOriginalFilename(sourceFile.fileName); const originalSourceFile = builder.getSourceFile(originalFilename); if (originalSourceFile) { affectedFiles.add(originalSourceFile); @@ -518,10 +589,10 @@ function findAffectedFiles( // info is present. if (includeTTC) { for (const sourceFile of builder.getSourceFiles()) { - if (ignoreForDiagnostics.has(sourceFile) && sourceFile.fileName.endsWith('.ngtypecheck.ts')) { + if (ignoreForDiagnostics.has(sourceFile) && isNgTypeCheckFile(sourceFile.fileName)) { // 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 originalFilename = getOriginalFilename(sourceFile.fileName); const originalSourceFile = builder.getSourceFile(originalFilename); if (originalSourceFile) { affectedFiles.add(originalSourceFile); @@ -530,5 +601,127 @@ function findAffectedFiles( } } + // When `isolatedModules` or `verbatimModuleSyntax` is enabled, TypeScript's `getFilesAffectedByUpdatedShapeWhenModuleEmit` + // only returns the modified file itself, so dependent `.ngtypecheck.ts` shims are not yielded by + // `getSemanticDiagnosticsOfNextAffectedFile`. However, TypeScript's `handleDtsMayChangeOfReferencingExportOfAffectedFile` + // still walks `referencedMap` when a modified file's `.d.ts` shape signature changes and calls + // `BuilderState.updateShapeSignature` on all affected dependents, recording them in `state.hasCalledUpdateShapeSignature`. + const compilerOptions = builder.getCompilerOptions(); + const hasCalledUpdateShapeSignature = (builder as InternalBuilderProgram).state + ?.hasCalledUpdateShapeSignature; + if ( + (compilerOptions.isolatedModules || compilerOptions.verbatimModuleSyntax) && + affectedFiles.size && + hasCalledUpdateShapeSignature?.size + ) { + for (const path of hasCalledUpdateShapeSignature) { + if (!isNgTypeCheckFile(path)) { + continue; + } + + const originalFilename = getOriginalFilename(path); + const originalSourceFile = builder.getSourceFile(originalFilename); + if (originalSourceFile) { + affectedFiles.add(originalSourceFile); + } + } + } + return affectedFiles; } + +interface InternalBuilderProgram extends ts.EmitAndSemanticDiagnosticsBuilderProgram { + state?: { + fileInfos?: Map; + changedFilesSet?: Set; + oldSignatures?: Map; + hasCalledUpdateShapeSignature?: Set; + }; +} + +interface InternalTypeScriptWithBuilderState { + BuilderState?: { + updateShapeSignature?( + state: unknown, + program: ts.Program, + sourceFile: ts.SourceFile, + cancellationToken: ts.CancellationToken | undefined, + host: ts.CompilerHost, + useFileVersionAsSignature: boolean, + ): boolean; + }; +} + +/** + * Commits the internal state of a TypeScript `BuilderProgram` by clearing uncommitted + * `oldSignatures` and `changedFilesSet` entries. + * + * Why this is needed: + * TypeScript's `BuilderProgram` only clears `state.oldSignatures` and `state.changedFilesSet` + * when `getSemanticDiagnostics()` (whole-program, no arguments) is called. Because Angular's + * `collectDiagnostics()` queries `getSemanticDiagnostics(sourceFile)` per-file (skipping + * internal `ignoreForDiagnostics` shims), TypeScript leaves `oldSignatures` populated with `false`. + * When `BuilderState.create` creates the next `BuilderProgram`, any `false` entry in `oldSignatures` + * resets that file's `info.signature` back to `undefined`. Clearing `oldSignatures` and `changedFilesSet` + * commits the current build's state so subsequent incremental builds preserve `.d.ts` signatures. + */ +function commitBuilderProgramState(program: InternalBuilderProgram): void { + program.state?.changedFilesSet?.clear(); + program.state?.oldSignatures?.clear(); +} + +/** + * Ensures `.d.ts` shape signatures (`info.signature`) are computed on the previous `BuilderProgram` + * for modified files before the next `BuilderProgram` is created. + * + * Why this is needed: + * 1. On the initial build (`oldProgram === undefined`), TypeScript's `BuilderState.create` sets + * `state.useFileVersionAsSignature = true`, which initializes `info.signature = sourceFile.version` + * (the SHA-256 hash of the `.ts` source text). + * 2. Normally, during `emitNextAffectedFile()`, TypeScript's `getWriteFileCallback` replaces + * `info.signature` with the hash of the emitted `.d.ts` declaration text. However, TypeScript + * explicitly guards this behind `if (!customTransformers)` (in `ts.createBuilderProgram`). + * Because Angular passes `angularCompiler.prepareEmit().transformers` (`customTransformers`) to + * `emitNextAffectedFile()`, TypeScript skips updating `info.signature`, leaving it equal to the + * `.ts` file hash (`sourceFile.version`). + * 3. On the first incremental watch rebuild (`oldProgram !== undefined`), `useFileVersionAsSignature` + * becomes `false` and TypeScript calls `BuilderState.updateShapeSignature()` (`computeDtsSignature`) + * for each modified file, comparing the newly computed `.d.ts` hash against the previous build's + * `info.signature` (which is still the `.ts` hash). Because a `.d.ts` hash never matches a `.ts` + * hash, TypeScript falsely concludes that the file's public `.d.ts` shape changed and cascades + * re-analysis and re-emission across all transitive importers in `referencedMap`. + * 4. Calling `BuilderState.updateShapeSignature(..., false)` on the previous program when a modified + * file's `signature` is still equal to its `version` replaces the initial `.ts` version hash with + * its true `.d.ts` signature prior to diffing, preventing false-positive `.d.ts` cascades on watch edits. + */ +function populatePreviousDtsSignatures( + oldProgram: InternalBuilderProgram, + oldHost: ts.CompilerHost, + modifiedFiles: ReadonlySet, +): void { + const updateShapeSignature = (ts as InternalTypeScriptWithBuilderState).BuilderState + ?.updateShapeSignature; + const oldState = oldProgram.state; + if (!updateShapeSignature || !oldState?.fileInfos) { + return; + } + + const program = oldProgram.getProgram(); + for (const modifiedFile of modifiedFiles) { + const sf = program.getSourceFile(modifiedFile); + if (!sf || sf.isDeclarationFile) { + continue; + } + + const resolvedPath = + (sf as ts.SourceFile & { resolvedPath?: string }).resolvedPath ?? modifiedFile; + const fileInfo = oldState.fileInfos.get(resolvedPath); + if (!fileInfo || fileInfo.signature !== fileInfo.version) { + continue; + } + + oldState.hasCalledUpdateShapeSignature?.delete(resolvedPath); + updateShapeSignature(oldState, program, sf, undefined, oldHost, false); + } + oldState.oldSignatures?.clear(); +}