fix(ivy): ensure that changes to component resources trigger incremental builds (#30954)

Optimizations to skip compiling source files that had not changed
did not account for the case where only a resource file changes,
such as an external template or style file.

Now we track such dependencies and trigger a recompilation
if any of the previously tracked resources have changed.

This will require a change on the CLI side to provide the list of
resource files that changed to trigger the current compilation by
implementing `CompilerHost.getModifiedResourceFiles()`.

Closes #30947

PR Close #30954

packages/compiler-cli/src/main.ts

@@ -24,7 +24,8 @@ export function main(
     args: string[], consoleError: (s: string) => void = console.error,
     config?: NgcParsedConfiguration, customTransformers?: api.CustomTransformers, programReuse?: {
       program: api.Program | undefined,
-    }): number {
+    },
+    modifiedResourceFiles?: Set<string>): number {
   let {project, rootNames, options, errors: configErrors, watch, emitFlags} =
       config || readNgcCommandLineAndConfiguration(args);
   if (configErrors.length) {
@@ -45,7 +46,7 @@ export function main(
     options,
     emitFlags,
     oldProgram,
-    emitCallback: createEmitCallback(options), customTransformers
+    emitCallback: createEmitCallback(options), customTransformers, modifiedResourceFiles
   });
   if (programReuse !== undefined) {
     programReuse.program = program;

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -20,6 +20,7 @@ import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from
 import {LocalModuleScopeRegistry} from '../../scope';
 import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform';
 import {TypeCheckContext} from '../../typecheck';
+import {NoopResourceDependencyRecorder, ResourceDependencyRecorder} from '../../util/src/resource_recorder';
 import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
 
 import {ResourceLoader} from './api';
@@ -48,7 +49,9 @@ export class ComponentDecoratorHandler implements
       private resourceLoader: ResourceLoader, private rootDirs: string[],
       private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean,
       private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
-      private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder) {}
+      private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder,
+      private resourceDependencies:
+          ResourceDependencyRecorder = new NoopResourceDependencyRecorder()) {}
 
   private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
   private elementSchemaRegistry = new DomElementSchemaRegistry();
@@ -182,6 +185,7 @@ export class ComponentDecoratorHandler implements
         }
         const templateUrl = this.resourceLoader.resolve(evalTemplateUrl, containingFile);
         const templateStr = this.resourceLoader.load(templateUrl);
+        this.resourceDependencies.recordResourceDependency(node.getSourceFile(), templateUrl);
 
         template = this._parseTemplate(
             component, templateStr, sourceMapUrl(templateUrl), /* templateRange */ undefined,
@@ -236,7 +240,9 @@ export class ComponentDecoratorHandler implements
       }
       for (const styleUrl of styleUrls) {
         const resourceUrl = this.resourceLoader.resolve(styleUrl, containingFile);
-        styles.push(this.resourceLoader.load(resourceUrl));
+        const resourceStr = this.resourceLoader.load(resourceUrl);
+        styles.push(resourceStr);
+        this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl);
       }
     }
     if (component.has('styles')) {
@@ -506,6 +512,7 @@ export class ComponentDecoratorHandler implements
       if (templatePromise !== undefined) {
         return templatePromise.then(() => {
           const templateStr = this.resourceLoader.load(resourceUrl);
+          this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl);
           const template = this._parseTemplate(
               component, templateStr, sourceMapUrl(resourceUrl), /* templateRange */ undefined,
               /* escapedString */ false);

packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel

@@ -22,6 +22,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/scope",
         "//packages/compiler-cli/src/ngtsc/testing",
         "//packages/compiler-cli/src/ngtsc/translator",
+        "//packages/compiler-cli/src/ngtsc/util",
         "@npm//typescript",
     ],
 )

packages/compiler-cli/src/ngtsc/incremental/BUILD.bazel

@@ -12,6 +12,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/metadata",
         "//packages/compiler-cli/src/ngtsc/partial_evaluator",
         "//packages/compiler-cli/src/ngtsc/reflection",
+        "//packages/compiler-cli/src/ngtsc/util",
         "@npm//typescript",
     ],
 )

packages/compiler-cli/src/ngtsc/incremental/src/state.ts

@@ -7,21 +7,26 @@
  */
 
 import * as ts from 'typescript';
+
 import {Reference} from '../../imports';
 import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata';
 import {DependencyTracker} from '../../partial_evaluator';
 import {ClassDeclaration} from '../../reflection';
+import {ResourceDependencyRecorder} from '../../util/src/resource_recorder';
 
 /**
  * Accumulates state between compilations.
  */
-export class IncrementalState implements DependencyTracker, MetadataReader, MetadataRegistry {
+export class IncrementalState implements DependencyTracker, MetadataReader, MetadataRegistry,
+    ResourceDependencyRecorder {
   private constructor(
       private unchangedFiles: Set<ts.SourceFile>,
-      private metadata: Map<ts.SourceFile, FileMetadata>) {}
+      private metadata: Map<ts.SourceFile, FileMetadata>,
+      private modifiedResourceFiles: Set<string>|null) {}
 
-  static reconcile(previousState: IncrementalState, oldProgram: ts.Program, newProgram: ts.Program):
+  static reconcile(
-      IncrementalState {
+      previousState: IncrementalState, oldProgram: ts.Program, newProgram: ts.Program,
+      modifiedResourceFiles: Set<string>|null): IncrementalState {
     const unchangedFiles = new Set<ts.SourceFile>();
     const metadata = new Map<ts.SourceFile, FileMetadata>();
     const oldFiles = new Set<ts.SourceFile>(oldProgram.getSourceFiles());
@@ -46,14 +51,17 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
       }
     }
 
-    return new IncrementalState(unchangedFiles, metadata);
+    return new IncrementalState(unchangedFiles, metadata, modifiedResourceFiles);
   }
 
   static fresh(): IncrementalState {
-    return new IncrementalState(new Set<ts.SourceFile>(), new Map<ts.SourceFile, FileMetadata>());
+    return new IncrementalState(
+        new Set<ts.SourceFile>(), new Map<ts.SourceFile, FileMetadata>(), null);
   }
 
-  safeToSkip(sf: ts.SourceFile): boolean { return this.unchangedFiles.has(sf); }
+  safeToSkip(sf: ts.SourceFile): boolean|Promise<boolean> {
+    return this.unchangedFiles.has(sf) && !this.hasChangedResourceDependencies(sf);
+  }
 
   trackFileDependency(dep: ts.SourceFile, src: ts.SourceFile) {
     const metadata = this.ensureMetadata(src);
@@ -92,11 +100,25 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
     metadata.pipeMeta.set(meta.ref.node, meta);
   }
 
+  recordResourceDependency(file: ts.SourceFile, resourcePath: string): void {
+    const metadata = this.ensureMetadata(file);
+    metadata.resourcePaths.add(resourcePath);
+  }
+
   private ensureMetadata(sf: ts.SourceFile): FileMetadata {
     const metadata = this.metadata.get(sf) || new FileMetadata();
     this.metadata.set(sf, metadata);
     return metadata;
   }
+
+  private hasChangedResourceDependencies(sf: ts.SourceFile): boolean {
+    if (this.modifiedResourceFiles === undefined || !this.metadata.has(sf)) {
+      return false;
+    }
+    const resourceDeps = this.metadata.get(sf) !.resourcePaths;
+    return Array.from(resourceDeps.keys())
+        .some(resourcePath => this.modifiedResourceFiles !.has(resourcePath));
+  }
 }
 
 /**
@@ -105,6 +127,7 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
 class FileMetadata {
   /** A set of source files that this file depends upon. */
   fileDependencies = new Set<ts.SourceFile>();
+  resourcePaths = new Set<string>();
   directiveMeta = new Map<ClassDeclaration, DirectiveMeta>();
   ngModuleMeta = new Map<ClassDeclaration, NgModuleMeta>();
   pipeMeta = new Map<ClassDeclaration, PipeMeta>();

packages/compiler-cli/src/ngtsc/program.ts

@@ -43,7 +43,6 @@ export class NgtscProgram implements api.Program {
   private compilation: IvyCompilation|undefined = undefined;
   private factoryToSourceInfo: Map<string, FactoryInfo>|null = null;
   private sourceToFactorySymbols: Map<string, Set<string>>|null = null;
-  private host: ts.CompilerHost;
   private _coreImportsFrom: ts.SourceFile|null|undefined = undefined;
   private _importRewriter: ImportRewriter|undefined = undefined;
   private _reflector: TypeScriptReflectionHost|undefined = undefined;
@@ -68,20 +67,23 @@ export class NgtscProgram implements api.Program {
   private incrementalState: IncrementalState;
   private typeCheckFilePath: AbsoluteFsPath;
 
+  private modifiedResourceFiles: Set<string>|null;
+
   constructor(
       rootNames: ReadonlyArray<string>, private options: api.CompilerOptions,
-      host: api.CompilerHost, oldProgram?: NgtscProgram) {
+      private host: api.CompilerHost, oldProgram?: NgtscProgram) {
     if (shouldEnablePerfTracing(options)) {
       this.perfTracker = PerfTracker.zeroedToNow();
       this.perfRecorder = this.perfTracker;
     }
 
+    this.modifiedResourceFiles =
+        this.host.getModifiedResourceFiles && this.host.getModifiedResourceFiles() || null;
     this.rootDirs = getRootDirs(host, options);
     this.closureCompilerEnabled = !!options.annotateForClosureCompiler;
     this.resourceManager = new HostResourceLoader(host, options);
     const shouldGenerateShims = options.allowEmptyCodegenFiles || false;
     const normalizedRootNames = rootNames.map(n => AbsoluteFsPath.from(n));
-    this.host = host;
     if (host.fileNameToModuleName !== undefined) {
       this.fileToModuleHost = host as FileToModuleHost;
     }
@@ -159,7 +161,8 @@ export class NgtscProgram implements api.Program {
       this.incrementalState = IncrementalState.fresh();
     } else {
       this.incrementalState = IncrementalState.reconcile(
-          oldProgram.incrementalState, oldProgram.reuseTsProgram, this.tsProgram);
+          oldProgram.incrementalState, oldProgram.reuseTsProgram, this.tsProgram,
+          this.modifiedResourceFiles);
     }
   }
 
@@ -498,7 +501,7 @@ export class NgtscProgram implements api.Program {
           this.reflector, evaluator, metaRegistry, this.metaReader !, scopeRegistry, this.isCore,
           this.resourceManager, this.rootDirs, this.options.preserveWhitespaces || false,
           this.options.i18nUseExternalIds !== false, this.moduleResolver, this.cycleAnalyzer,
-          this.refEmitter, this.defaultImportTracker),
+          this.refEmitter, this.defaultImportTracker, this.incrementalState),
       new DirectiveDecoratorHandler(
           this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
       new InjectableDecoratorHandler(

packages/compiler-cli/src/ngtsc/util/src/resource_recorder.ts

@@ -0,0 +1,20 @@
+
+/**
+ * @license
+ * Copyright Google Inc. 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.io/license
+ */
+import * as ts from 'typescript';
+
+/**
+ * Implement this interface to record what resources a source file depends upon.
+ */
+export interface ResourceDependencyRecorder {
+  recordResourceDependency(file: ts.SourceFile, resourcePath: string): void;
+}
+
+export class NoopResourceDependencyRecorder implements ResourceDependencyRecorder {
+  recordResourceDependency(): void {}
+}

packages/compiler-cli/src/perform_compile.ts

@@ -220,10 +220,10 @@ export function exitCodeFromResult(diags: Diagnostics | undefined): number {
   return diags.some(d => d.source === 'angular' && d.code === api.UNKNOWN_ERROR_CODE) ? 2 : 1;
 }
 
-export function performCompilation({rootNames, options, host, oldProgram, emitCallback,
+export function performCompilation(
-                                    mergeEmitResultsCallback,
+    {rootNames, options, host, oldProgram, emitCallback, mergeEmitResultsCallback,
-                                    gatherDiagnostics = defaultGatherDiagnostics,
+     gatherDiagnostics = defaultGatherDiagnostics, customTransformers,
-                                    customTransformers, emitFlags = api.EmitFlags.Default}: {
+     emitFlags = api.EmitFlags.Default, modifiedResourceFiles}: {
       rootNames: string[],
       options: api.CompilerOptions,
       host?: api.CompilerHost,
@@ -232,7 +232,8 @@ export function performCompilation({rootNames, options, host, oldProgram, emitCa
       mergeEmitResultsCallback?: api.TsMergeEmitResultsCallback,
       gatherDiagnostics?: (program: api.Program) => Diagnostics,
       customTransformers?: api.CustomTransformers,
-  emitFlags?: api.EmitFlags
+      emitFlags?: api.EmitFlags,
+      modifiedResourceFiles?: Set<string>,
     }): PerformCompilationResult {
   let program: api.Program|undefined;
   let emitResult: ts.EmitResult|undefined;
@@ -241,6 +242,9 @@ export function performCompilation({rootNames, options, host, oldProgram, emitCa
     if (!host) {
       host = ng.createCompilerHost({options});
     }
+    if (modifiedResourceFiles) {
+      host.getModifiedResourceFiles = () => modifiedResourceFiles;
+    }
 
     program = ng.createProgram({rootNames, host, options, oldProgram});
 

packages/compiler-cli/src/perform_watch.ts

@@ -103,6 +103,11 @@ interface CacheEntry {
   content?: string;
 }
 
+interface QueuedCompilationInfo {
+  timerHandle: any;
+  modifiedResourceFiles: Set<string>;
+}
+
 /**
  * The logic in this function is adapted from `tsc.ts` from TypeScript.
  */
@@ -111,7 +116,8 @@ export function performWatchCompilation(host: PerformWatchHost):
   let cachedProgram: api.Program|undefined;            // Program cached from last compilation
   let cachedCompilerHost: api.CompilerHost|undefined;  // CompilerHost cached from last compilation
   let cachedOptions: ParsedConfiguration|undefined;  // CompilerOptions cached from last compilation
-  let timerHandleForRecompilation: any;  // Handle for 0.25s wait timer to trigger recompilation
+  let timerHandleForRecompilation: QueuedCompilationInfo|
+      undefined;  // Handle for 0.25s wait timer to trigger recompilation
 
   const ignoreFilesForWatch = new Set<string>();
   const fileCache = new Map<string, CacheEntry>();
@@ -141,13 +147,13 @@ export function performWatchCompilation(host: PerformWatchHost):
   function close() {
     fileWatcher.close();
     if (timerHandleForRecompilation) {
-      host.clearTimeout(timerHandleForRecompilation);
+      host.clearTimeout(timerHandleForRecompilation.timerHandle);
       timerHandleForRecompilation = undefined;
     }
   }
 
   // Invoked to perform initial compilation or re-compilation in watch mode
-  function doCompilation(): Diagnostics {
+  function doCompilation(modifiedResourceFiles?: Set<string>): Diagnostics {
     if (!cachedOptions) {
       cachedOptions = host.readConfiguration();
     }
@@ -190,6 +196,9 @@ export function performWatchCompilation(host: PerformWatchHost):
         }
         return ce.content !;
       };
+      // Provide access to the file paths that triggered this rebuild
+      cachedCompilerHost.getModifiedResourceFiles =
+          modifiedResourceFiles !== undefined ? () => modifiedResourceFiles : undefined;
     }
     ignoreFilesForWatch.clear();
     const oldProgram = cachedProgram;
@@ -255,24 +264,30 @@ export function performWatchCompilation(host: PerformWatchHost):
 
     if (!ignoreFilesForWatch.has(path.normalize(fileName))) {
       // Ignore the file if the file is one that was written by the compiler.
-      startTimerForRecompilation();
+      startTimerForRecompilation(fileName);
     }
   }
 
   // Upon detecting a file change, wait for 250ms and then perform a recompilation. This gives batch
   // operations (such as saving all modified files in an editor) a chance to complete before we kick
   // off a new compilation.
-  function startTimerForRecompilation() {
+  function startTimerForRecompilation(changedPath: string) {
     if (timerHandleForRecompilation) {
-      host.clearTimeout(timerHandleForRecompilation);
+      host.clearTimeout(timerHandleForRecompilation.timerHandle);
+    } else {
+      timerHandleForRecompilation = {
+        modifiedResourceFiles: new Set<string>(),
+        timerHandle: undefined
+      };
     }
-    timerHandleForRecompilation = host.setTimeout(recompile, 250);
+    timerHandleForRecompilation.timerHandle = host.setTimeout(recompile, 250);
+    timerHandleForRecompilation.modifiedResourceFiles.add(changedPath);
   }
 
   function recompile() {
-    timerHandleForRecompilation = undefined;
     host.reportDiagnostics(
         [createMessageDiagnostic('File change detected. Starting incremental compilation.')]);
-    doCompilation();
+    doCompilation(timerHandleForRecompilation !.modifiedResourceFiles);
+    timerHandleForRecompilation = undefined;
   }
 }

packages/compiler-cli/src/transformers/api.ts

@@ -281,6 +281,12 @@ export interface CompilerHost extends ts.CompilerHost {
    * rather than by path. See http://requirejs.org/docs/whyamd.html#namedmodules
    */
   amdModuleName?(sf: ts.SourceFile): string|undefined;
+
+  /**
+   * Get the absolute paths to the changed files that triggered the current compilation
+   * or `undefined` if this is not an incremental build.
+   */
+  getModifiedResourceFiles?(): Set<string>|undefined;
 }
 
 export enum EmitFlags {

packages/compiler-cli/test/ngtsc/BUILD.bazel

@@ -8,6 +8,7 @@ ts_library(
         "//packages/compiler",
         "//packages/compiler-cli",
         "//packages/compiler-cli/src/ngtsc/diagnostics",
+        "//packages/compiler-cli/src/ngtsc/path",
         "//packages/compiler-cli/src/ngtsc/routing",
         "//packages/compiler-cli/src/ngtsc/util",
         "//packages/compiler-cli/test:test_utils",

packages/compiler-cli/test/ngtsc/env.ts

@@ -39,6 +39,7 @@ function setupFakeCore(support: TestSupport): void {
 export class NgtscTestEnvironment {
   private multiCompileHostExt: MultiCompileHostExt|null = null;
   private oldProgram: Program|null = null;
+  private changedResources: Set<string>|undefined = undefined;
 
   private constructor(private support: TestSupport, readonly outDir: string) {}
 
@@ -106,6 +107,7 @@ export class NgtscTestEnvironment {
   }
 
   enableMultipleCompilations(): void {
+    this.changedResources = new Set();
     this.multiCompileHostExt = new MultiCompileHostExt();
     setWrapHostForTest(makeWrapHost(this.multiCompileHostExt));
   }
@@ -114,6 +116,7 @@ export class NgtscTestEnvironment {
     if (this.multiCompileHostExt === null) {
       throw new Error(`Not tracking written files - call enableMultipleCompilations()`);
     }
+    this.changedResources !.clear();
     this.multiCompileHostExt.flushWrittenFileTracking();
   }
 
@@ -133,8 +136,9 @@ export class NgtscTestEnvironment {
 
   write(fileName: string, content: string) {
     if (this.multiCompileHostExt !== null) {
-      const absFilePath = path.posix.resolve(this.support.basePath, fileName);
+      const absFilePath = path.resolve(this.support.basePath, fileName).replace(/\\/g, '/');
       this.multiCompileHostExt.invalidate(absFilePath);
+      this.changedResources !.add(absFilePath);
     }
     this.support.write(fileName, content);
   }
@@ -175,8 +179,9 @@ export class NgtscTestEnvironment {
         program: this.oldProgram || undefined,
       };
     }
-    const exitCode =
+    const exitCode = main(
-        main(['-p', this.basePath], errorSpy, undefined, customTransformers, reuseProgram);
+        ['-p', this.basePath], errorSpy, undefined, customTransformers, reuseProgram,
+        this.changedResources);
     expect(errorSpy).not.toHaveBeenCalled();
     expect(exitCode).toBe(0);
     if (this.multiCompileHostExt !== null) {

packages/compiler-cli/test/ngtsc/incremental_spec.ts

@@ -6,6 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/path';
+
 import {NgtscTestEnvironment} from './env';
 
 describe('ngtsc incremental compilation', () => {
@@ -69,6 +71,33 @@ describe('ngtsc incremental compilation', () => {
     expect(written).not.toContain('/component2.js');
   });
 
+  it('should rebuild components whose templates have changed', () => {
+    env.write('component1.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'cmp', templateUrl: './component1.template.html'})
+      export class Cmp1 {}
+    `);
+    env.write('component1.template.html', 'cmp1');
+    env.write('component2.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'cmp2', templateUrl: './component2.template.html'})
+      export class Cmp2 {}
+    `);
+    env.write('component2.template.html', 'cmp2');
+
+    env.driveMain();
+
+    // Make a change to Cmp1 template
+    env.flushWrittenFileTracking();
+    env.write('component1.template.html', 'changed');
+    env.driveMain();
+    const written = env.getFilesWrittenSinceLastFlush();
+    expect(written).toContain('/component1.js');
+    expect(written).not.toContain('/component2.js');
+  });
+
   it('should rebuild components whose partial-evaluation dependencies have changed', () => {
     env.write('component1.ts', `
       import {Component} from '@angular/core';