fix(ivy): reuse compilation scope for incremental template changes. (#31932)

Previously if only a component template changed then we would know to
rebuild its component source file. But the compilation was incorrect if the
component was part of an NgModule, since we were not capturing the
compilation scope information that had a been acquired from the NgModule
and was not being regenerated since we were not needing to recompile
the NgModule.

Now we register compilation scope information for each component, via the
`ComponentScopeRegistry` interface, so that it is available for incremental
compilation.

The `ComponentDecoratorHandler` now reads the compilation scope from a
`ComponentScopeReader` interface which is implemented as a compound
reader composed of the original `LocalModuleScopeRegistry` and the
`IncrementalState`.

Fixes #31654

PR Close #31932

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -75,7 +75,7 @@ export class DecorationAnalyzer {
     new BaseDefDecoratorHandler(this.reflectionHost, this.evaluator, this.isCore),
     new ComponentDecoratorHandler(
         this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader,
-        this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs,
+        this.scopeRegistry, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs,
         /* defaultPreserveWhitespaces */ false,
         /* i18nUseExternalIds */ true, this.moduleResolver, this.cycleAnalyzer, this.refEmitter,
         NOOP_DEFAULT_IMPORT_RECORDER),

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

@@ -18,7 +18,7 @@ import {DirectiveMeta, MetadataReader, MetadataRegistry, extractDirectiveGuards}
 import {flattenInheritedDirectiveMetadata} from '../../metadata/src/inheritance';
 import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
 import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
-import {LocalModuleScopeRegistry} from '../../scope';
+import {ComponentScopeReader, LocalModuleScopeRegistry} from '../../scope';
 import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform';
 import {TypeCheckContext} from '../../typecheck';
 import {NoopResourceDependencyRecorder, ResourceDependencyRecorder} from '../../util/src/resource_recorder';
@@ -47,8 +47,8 @@ export class ComponentDecoratorHandler implements
   constructor(
       private reflector: ReflectionHost, private evaluator: PartialEvaluator,
       private metaRegistry: MetadataRegistry, private metaReader: MetadataReader,
-      private scopeRegistry: LocalModuleScopeRegistry, private isCore: boolean,
-      private resourceLoader: ResourceLoader, private rootDirs: string[],
+      private scopeReader: ComponentScopeReader, private scopeRegistry: LocalModuleScopeRegistry,
+      private isCore: boolean, private resourceLoader: ResourceLoader, private rootDirs: string[],
       private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean,
       private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
       private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder,
@@ -327,7 +327,7 @@ export class ComponentDecoratorHandler implements
       preserveWhitespaces: true,
       leadingTriviaChars: [],
     });
-    const scope = this.scopeRegistry.getScopeForComponent(node);
+    const scope = this.scopeReader.getScopeForComponent(node);
     const selector = analysis.meta.selector;
     const matcher = new SelectorMatcher<DirectiveMeta>();
     if (scope !== null) {
@@ -353,7 +353,7 @@ export class ComponentDecoratorHandler implements
     if (!ts.isClassDeclaration(node)) {
       return;
     }
-    const scope = this.scopeRegistry.getScopeForComponent(node);
+    const scope = this.scopeReader.getScopeForComponent(node);
     const matcher = new SelectorMatcher<DirectiveMeta>();
     if (scope !== null) {
       for (const meta of scope.compilation.directives) {
@@ -377,7 +377,7 @@ export class ComponentDecoratorHandler implements
     const context = node.getSourceFile();
     // Check whether this component was registered with an NgModule. If so, it should be compiled
     // under that module's compilation scope.
-    const scope = this.scopeRegistry.getScopeForComponent(node);
+    const scope = this.scopeReader.getScopeForComponent(node);
     let metadata = analysis.meta;
     if (scope !== null) {
       // Replace the empty components and directives from the analyze() step with a fully expanded

packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts

@@ -60,7 +60,7 @@ runInEachFileSystem(() => {
       const refEmitter = new ReferenceEmitter([]);
 
       const handler = new ComponentDecoratorHandler(
-          reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, false,
+          reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry, false,
           new NoopResourceLoader(), [''], false, true, moduleResolver, cycleAnalyzer, refEmitter,
           NOOP_DEFAULT_IMPORT_RECORDER);
       const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);

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/scope",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//typescript",
     ],

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

@@ -12,13 +12,14 @@ import {Reference} from '../../imports';
 import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata';
 import {DependencyTracker} from '../../partial_evaluator';
 import {ClassDeclaration} from '../../reflection';
+import {ComponentScopeReader, ComponentScopeRegistry, LocalModuleScope} from '../../scope';
 import {ResourceDependencyRecorder} from '../../util/src/resource_recorder';
 
 /**
  * Accumulates state between compilations.
  */
 export class IncrementalState implements DependencyTracker, MetadataReader, MetadataRegistry,
-    ResourceDependencyRecorder {
+    ResourceDependencyRecorder, ComponentScopeRegistry, ComponentScopeReader {
   private constructor(
       private unchangedFiles: Set<ts.SourceFile>,
       private metadata: Map<ts.SourceFile, FileMetadata>,
@@ -69,32 +70,56 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
   }
 
   getFileDependencies(file: ts.SourceFile): ts.SourceFile[] {
-    const meta = this.metadata.get(file);
-    return meta ? Array.from(meta.fileDependencies) : [];
+    if (!this.metadata.has(file)) {
+      return [];
+    }
+    const meta = this.metadata.get(file) !;
+    return Array.from(meta.fileDependencies);
   }
 
   getNgModuleMetadata(ref: Reference<ClassDeclaration>): NgModuleMeta|null {
-    const metadata = this.metadata.get(ref.node.getSourceFile()) || null;
-    return metadata && metadata.ngModuleMeta.get(ref.node) || null;
+    if (!this.metadata.has(ref.node.getSourceFile())) {
+      return null;
+    }
+    const metadata = this.metadata.get(ref.node.getSourceFile()) !;
+    if (!metadata.ngModuleMeta.has(ref.node)) {
+      return null;
+    }
+    return metadata.ngModuleMeta.get(ref.node) !;
   }
+
   registerNgModuleMetadata(meta: NgModuleMeta): void {
     const metadata = this.ensureMetadata(meta.ref.node.getSourceFile());
     metadata.ngModuleMeta.set(meta.ref.node, meta);
   }
 
   getDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta|null {
-    const metadata = this.metadata.get(ref.node.getSourceFile()) || null;
-    return metadata && metadata.directiveMeta.get(ref.node) || null;
+    if (!this.metadata.has(ref.node.getSourceFile())) {
+      return null;
+    }
+    const metadata = this.metadata.get(ref.node.getSourceFile()) !;
+    if (!metadata.directiveMeta.has(ref.node)) {
+      return null;
+    }
+    return metadata.directiveMeta.get(ref.node) !;
   }
+
   registerDirectiveMetadata(meta: DirectiveMeta): void {
     const metadata = this.ensureMetadata(meta.ref.node.getSourceFile());
     metadata.directiveMeta.set(meta.ref.node, meta);
   }
 
   getPipeMetadata(ref: Reference<ClassDeclaration>): PipeMeta|null {
-    const metadata = this.metadata.get(ref.node.getSourceFile()) || null;
-    return metadata && metadata.pipeMeta.get(ref.node) || null;
+    if (!this.metadata.has(ref.node.getSourceFile())) {
+      return null;
+    }
+    const metadata = this.metadata.get(ref.node.getSourceFile()) !;
+    if (!metadata.pipeMeta.has(ref.node)) {
+      return null;
+    }
+    return metadata.pipeMeta.get(ref.node) !;
   }
+
   registerPipeMetadata(meta: PipeMeta): void {
     const metadata = this.ensureMetadata(meta.ref.node.getSourceFile());
     metadata.pipeMeta.set(meta.ref.node, meta);
@@ -105,6 +130,40 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
     metadata.resourcePaths.add(resourcePath);
   }
 
+  registerComponentScope(clazz: ClassDeclaration, scope: LocalModuleScope): void {
+    const metadata = this.ensureMetadata(clazz.getSourceFile());
+    metadata.componentScope.set(clazz, scope);
+  }
+
+  getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null {
+    if (!this.metadata.has(clazz.getSourceFile())) {
+      return null;
+    }
+    const metadata = this.metadata.get(clazz.getSourceFile()) !;
+    if (!metadata.componentScope.has(clazz)) {
+      return null;
+    }
+    return metadata.componentScope.get(clazz) !;
+  }
+
+  setComponentAsRequiringRemoteScoping(clazz: ClassDeclaration): void {
+    const metadata = this.ensureMetadata(clazz.getSourceFile());
+    metadata.remoteScoping.add(clazz);
+  }
+
+  getRequiresRemoteScope(clazz: ClassDeclaration): boolean|null {
+    // TODO: https://angular-team.atlassian.net/browse/FW-1501
+    // Handle the incremental build case where a component requires remote scoping.
+    // This means that if the the component's template changes, it requires the module to be
+    // re-emitted.
+    // Also, we need to make sure the cycle detector works well across rebuilds.
+    if (!this.metadata.has(clazz.getSourceFile())) {
+      return null;
+    }
+    const metadata = this.metadata.get(clazz.getSourceFile()) !;
+    return metadata.remoteScoping.has(clazz);
+  }
+
   private ensureMetadata(sf: ts.SourceFile): FileMetadata {
     const metadata = this.metadata.get(sf) || new FileMetadata();
     this.metadata.set(sf, metadata);
@@ -131,4 +190,6 @@ class FileMetadata {
   directiveMeta = new Map<ClassDeclaration, DirectiveMeta>();
   ngModuleMeta = new Map<ClassDeclaration, NgModuleMeta>();
   pipeMeta = new Map<ClassDeclaration, PipeMeta>();
+  componentScope = new Map<ClassDeclaration, LocalModuleScope>();
+  remoteScoping = new Set<ClassDeclaration>();
 }

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

@@ -28,7 +28,7 @@ import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf';
 import {TypeScriptReflectionHost} from './reflection';
 import {HostResourceLoader} from './resource_loader';
 import {NgModuleRouteAnalyzer, entryPointKeyFor} from './routing';
-import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from './scope';
+import {CompoundComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from './scope';
 import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator, SummaryGenerator, TypeCheckShimGenerator, generatedFactoryTransform} from './shims';
 import {ivySwitchTransform} from './switch';
 import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform';
@@ -476,7 +476,8 @@ export class NgtscProgram implements api.Program {
     const localMetaReader = new CompoundMetadataReader([localMetaRegistry, this.incrementalState]);
     const depScopeReader = new MetadataDtsModuleScopeResolver(dtsReader, aliasGenerator);
     const scopeRegistry = new LocalModuleScopeRegistry(
-        localMetaReader, depScopeReader, this.refEmitter, aliasGenerator);
+        localMetaReader, depScopeReader, this.refEmitter, aliasGenerator, this.incrementalState);
+    const scopeReader = new CompoundComponentScopeReader([scopeRegistry, this.incrementalState]);
     const metaRegistry =
         new CompoundMetadataRegistry([localMetaRegistry, scopeRegistry, this.incrementalState]);
 
@@ -502,10 +503,11 @@ export class NgtscProgram implements api.Program {
     const handlers = [
       new BaseDefDecoratorHandler(this.reflector, evaluator, this.isCore),
       new ComponentDecoratorHandler(
-          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.incrementalState),
+          this.reflector, evaluator, metaRegistry, this.metaReader !, scopeReader, scopeRegistry,
+          this.isCore, this.resourceManager, this.rootDirs,
+          this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false,
+          this.moduleResolver, this.cycleAnalyzer, this.refEmitter, this.defaultImportTracker,
+          this.incrementalState),
       new DirectiveDecoratorHandler(
           this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
       new InjectableDecoratorHandler(

packages/compiler-cli/src/ngtsc/scope/index.ts

@@ -7,5 +7,6 @@
  */
 
 export {ExportScope, ScopeData} from './src/api';
+export {ComponentScopeReader, ComponentScopeRegistry, CompoundComponentScopeReader} from './src/component_scope';
 export {DtsModuleScopeResolver, MetadataDtsModuleScopeResolver} from './src/dependency';
 export {LocalModuleScope, LocalModuleScopeRegistry, LocalNgModuleData} from './src/local';

packages/compiler-cli/src/ngtsc/scope/src/component_scope.ts

@@ -0,0 +1,67 @@
+/**
+ * @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 {ClassDeclaration} from '../../reflection';
+import {LocalModuleScope} from './local';
+
+/**
+ * Register information about the compilation scope of components.
+ */
+export interface ComponentScopeRegistry {
+  registerComponentScope(clazz: ClassDeclaration, scope: LocalModuleScope): void;
+  setComponentAsRequiringRemoteScoping(clazz: ClassDeclaration): void;
+}
+
+/**
+ * Read information about the compilation scope of components.
+ */
+export interface ComponentScopeReader {
+  getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null;
+  getRequiresRemoteScope(clazz: ClassDeclaration): boolean|null;
+}
+
+/**
+ * A noop registry that doesn't do anything.
+ *
+ * This can be used in tests and cases where we don't care about the compilation scopes
+ * being registered.
+ */
+export class NoopComponentScopeRegistry implements ComponentScopeRegistry {
+  registerComponentScope(clazz: ClassDeclaration, scope: LocalModuleScope): void {}
+  setComponentAsRequiringRemoteScoping(clazz: ClassDeclaration): void {}
+}
+
+/**
+ * A `ComponentScopeReader` that reads from an ordered set of child readers until it obtains the
+ * requested scope.
+ *
+ * This is used to combine `ComponentScopeReader`s that read from different sources (e.g. from a
+ * registry and from the incremental state).
+ */
+export class CompoundComponentScopeReader implements ComponentScopeReader {
+  constructor(private readers: ComponentScopeReader[]) {}
+
+  getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null {
+    for (const reader of this.readers) {
+      const meta = reader.getScopeForComponent(clazz);
+      if (meta !== null) {
+        return meta;
+      }
+    }
+    return null;
+  }
+
+  getRequiresRemoteScope(clazz: ClassDeclaration): boolean|null {
+    for (const reader of this.readers) {
+      const requiredScoping = reader.getRequiresRemoteScope(clazz);
+      if (requiredScoping !== null) {
+        return requiredScoping;
+      }
+    }
+    return null;
+  }
+}

packages/compiler-cli/src/ngtsc/scope/src/local.ts

@@ -11,11 +11,12 @@ import * as ts from 'typescript';
 
 import {ErrorCode, makeDiagnostic} from '../../diagnostics';
 import {AliasGenerator, Reexport, Reference, ReferenceEmitter} from '../../imports';
-import {DirectiveMeta, LocalMetadataRegistry, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata';
+import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata';
 import {ClassDeclaration} from '../../reflection';
 import {identifierOfNode, nodeNameForError} from '../../util/src/typescript';
 
 import {ExportScope, ScopeData} from './api';
+import {ComponentScopeReader, ComponentScopeRegistry, NoopComponentScopeRegistry} from './component_scope';
 import {DtsModuleScopeResolver} from './dependency';
 
 export interface LocalNgModuleData {
@@ -58,7 +59,7 @@ export interface CompilationScope extends ScopeData {
  * The `LocalModuleScopeRegistry` is also capable of producing `ts.Diagnostic` errors when Angular
  * semantics are violated.
  */
-export class LocalModuleScopeRegistry implements MetadataRegistry {
+export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScopeReader {
   /**
    * Tracks whether the registry has been asked to produce scopes for a module or component. Once
    * this is true, the registry cannot accept registrations of new directives/pipes/modules as it
@@ -102,7 +103,8 @@ export class LocalModuleScopeRegistry implements MetadataRegistry {
 
   constructor(
       private localReader: MetadataReader, private dependencyScopeReader: DtsModuleScopeResolver,
-      private refEmitter: ReferenceEmitter, private aliasGenerator: AliasGenerator|null) {}
+      private refEmitter: ReferenceEmitter, private aliasGenerator: AliasGenerator|null,
+      private componentScopeRegistry: ComponentScopeRegistry = new NoopComponentScopeRegistry()) {}
 
   /**
    * Add an NgModule's data to the registry.
@@ -120,10 +122,13 @@ export class LocalModuleScopeRegistry implements MetadataRegistry {
   registerPipeMetadata(pipe: PipeMeta): void {}
 
   getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null {
-    if (!this.declarationToModule.has(clazz)) {
-      return null;
+    const scope = !this.declarationToModule.has(clazz) ?
+        null :
+        this.getScopeOfModule(this.declarationToModule.get(clazz) !);
+    if (scope !== null) {
+      this.componentScopeRegistry.registerComponentScope(clazz, scope);
     }
-    return this.getScopeOfModule(this.declarationToModule.get(clazz) !);
+    return scope;
   }
 
   /**
@@ -383,6 +388,7 @@ export class LocalModuleScopeRegistry implements MetadataRegistry {
    */
   setComponentAsRequiringRemoteScoping(node: ClassDeclaration): void {
     this.remoteScoping.add(node);
+    this.componentScopeRegistry.setComponentAsRequiringRemoteScoping(node);
   }
 
   /**

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

@@ -195,6 +195,25 @@ runInEachFileSystem(() => {
       expect(written).toContain('/foo_module.js');
     });
 
+    it('should rebuild only a Component (but with the correct CompilationScope) if its template has changed',
+       () => {
+         setupFooBarProgram(env);
+
+         // Make a change to the template of BarComponent.
+         env.write('bar_component.html', '<div bar>changed</div>');
+
+         env.driveMain();
+         const written = env.getFilesWrittenSinceLastFlush();
+         expect(written).not.toContain('/bar_directive.js');
+         expect(written).toContain('/bar_component.js');
+         expect(written).not.toContain('/bar_module.js');
+         expect(written).not.toContain('/foo_component.js');
+         expect(written).not.toContain('/foo_pipe.js');
+         expect(written).not.toContain('/foo_module.js');
+         // Ensure that the used directives are included in the component's generated template.
+         expect(env.getContents('/built/bar_component.js')).toMatch(/directives:\s*\[.+\.BarDir\]/);
+       });
+
     it('should rebuild everything if a typings file changes', () => {
       setupFooBarProgram(env);
 
@@ -280,9 +299,10 @@ runInEachFileSystem(() => {
     env.write('bar_component.ts', `
     import {Component} from '@angular/core';
 
-    @Component({selector: 'bar', template: 'bar'})
+    @Component({selector: 'bar', templateUrl: './bar_component.html'})
     export class BarCmp {}
   `);
+    env.write('bar_component.html', '<div bar></div>');
     env.write('bar_directive.ts', `
     import {Directive} from '@angular/core';