perf(@angular/build): optimize incremental watch rebuilds in AOT compilation

[alan-agius4]

Tip

Consider enabling Hide whitespace (view with ?w=1) when reviewing this PR for a cleaner diff.

Optimizes incremental watch rebuilds in AotCompilation:

• 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 Angular's template type-checking program (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.

Benchmarks

Tested on a large library reproduction (ngwr) with 228 entry points. Median timings across 3 independent runs:

Step	Scenario	main	This PR	Speedup
Initial Build	Cold library compilation	10.53s (228 entry points)	10.36s (228 entry points)	Parity
Rebuild 1 (First Edit)	Template edit (alert.html)	4.59s (110 entry points)	0.38s (1 entry point)	12.0x faster (-91.7%)
Rebuild 2 (Subsequent)	Template edit (alert.html)	0.29s (1 entry point)	0.31s (1 entry point)	Parity
Rebuild 3	Component TS edit (alert.ts)	0.30s (1 entry point)	0.32s (1 entry point)	Parity
Rebuild 4	Downstream utility edit (clamp.ts)	2.30s (8 entry points)	0.17s (1 entry point)	13.2x faster (-92.4%)

[gemini-code-assist]

Code Review

This pull request optimizes incremental builds in the Angular compilation pipeline by caching the tsconfig configuration and managing the internal state of the TypeScript BuilderProgram to prevent false-positive .d.ts cascades during watch rebuilds. It also refactors file reference collection to be more efficient. The feedback highlights a critical issue where the newly added builder program synchronization and state-committing logic is placed at the end of a generator function. Since generator execution can terminate early, this logic should be wrapped in a try...finally block to guarantee it runs.

[gemini-code-assist]

Code Review

This pull request optimizes incremental builds in the Angular compilation pipeline by caching the parsed TypeScript configuration and introducing state synchronization to prevent false-positive .d.ts cascades. It also refactors resource dependency handling during file reference collection. The review feedback suggests making the .json file extension check case-insensitive to ensure robust cache invalidation across different filesystems.

[dgp1130]

I'm not terribly familiar with this code, so left some thoughts about what immediately stood out to me, but feel free to ignore if this feedback isn't relevant / useful.

[dgp1130]

Consider: Agent suggests there's a potential casing bug here.

On case-insensitive filesystems or platforms like Windows where file watcher paths in modifiedFiles may contain backslashes or non-canonical casing, filesWithoutDtsSignature (which stores sourceFile.fileName) might not match modifiedFile directly.

Retrieving sf first via oldProgram.getSourceFile(modifiedFile) allows using sf.fileName (or sf.path) to ensure filesWithoutDtsSignature is reliably checked and cleared regardless of path formatting:

Suggested change

	for (const modifiedFile of modifiedFiles) {
	for (const modifiedFile of modifiedFiles) {
	const sf = oldProgram.getSourceFile(modifiedFile);
	if (sf && !sf.isDeclarationFile) {
	if (
	filesWithoutDtsSignature.delete(sf.fileName) ||
	filesWithoutDtsSignature.delete(modifiedFile)
	) {
	filesToUpdate.push(sf);
	}
	}
	}

[dgp1130]

Consider: Is this configuring the initial state? Would it be equivalent and simpler to do something like:

Suggested change

	const filesWithoutDtsSignature = this.#state?.filesWithoutDtsSignature ?? new Set<string>();
	if (!this.#state) {
	for (const sourceFile of typeScriptProgram.getSourceFiles()) {
	if (
	!sourceFile.isDeclarationFile &&
	!angularCompiler.ignoreForDiagnostics.has(sourceFile)
	) {
	filesWithoutDtsSignature.add(sourceFile.fileName);
	}
	}
	}
	const filesWithoutDtsSignature = this.#state?.filesWithoutDtsSignature ?? new Set<string>(
	typeScriptProgram.getSourceFiles()
	.filter((file) => !file.isDeclarationFile && angularCompiler.ignoreForDiagnostics.has(file))
	.map((file) => file.fileName)
	);

[dgp1130]

Consider: I'm not super familiar with this API, but from a drive-by review the pattern of:

while (builder.getSemanticDiagnosticsOfNextAffectedFile(undefined, () => {
  // Non-trivial implementation.
})) {
  // Empty
}

Is a bit awkward. Is there a way where we can just emit all the files in a generator or something like:

function* affectedFiles() {
  while (true) {
    const result = builder.getSemanticDiagnosticsOfNextAffectedFile(undefined, (sourceFile) => {
      // Yeah, `yield` doesn't work in a closure like this, so you might need to have fun with
      // iterators directly.
      yield sourceFile;
    });

    if (!result) break;
  }
}

// Then...

for (const file of affectedFiles()) {
  doSomethingWith(file);
}

[dgp1130]

Consider: It seems like we're using this approach of:

try {
  program.prop = newValue;
  // Do something interesting...
} finally {
  program.prop = originalValue;
}

I worry about the side effects of this kind of mutation. Would it be cleaner to just create a new program based on the previous one with the relevant values overwritten and use it in this function? That way we don't need to worry about side effects or remembering to restore values like this?