feat(ivy): skip analysis of unchanged components (angular#30238)

Now that the dependent files and compilation scopes are being tracked in
the incremental state, we can skip analysing and emitting source files if
none of their dependent files have changed since the last compile.

The computation of what files (and their dependencies) are unchanged is
computed during reconciliation.

This commit also removes the previous emission skipping logic, since this
approach covers those cases already.

PR Close angular#30238

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

@@ -50,8 +50,6 @@ export class InjectableDecoratorHandler implements
 
   analyze(node: ClassDeclaration, decorator: Decorator): AnalysisOutput<InjectableHandlerData> {
     return {
-      // @Injectable()s cannot depend on other files for their compilation output.
-      allowSkipAnalysisAndEmit: true,
       analysis: {
         meta: extractInjectableMetadata(
             node, decorator, this.reflector, this.defaultImportRecorder, this.isCore,

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

@@ -24,24 +24,25 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
       IncrementalState {
     const unchangedFiles = new Set<ts.SourceFile>();
     const metadata = new Map<ts.SourceFile, FileMetadata>();
+    const oldFiles = new Set<ts.SourceFile>(oldProgram.getSourceFiles());
+    const newFiles = new Set<ts.SourceFile>(newProgram.getSourceFiles());
 
-    // Compute the set of files that's unchanged.
-    const oldFiles = new Set<ts.SourceFile>();
-    for (const oldFile of oldProgram.getSourceFiles()) {
-      if (!oldFile.isDeclarationFile) {
-        oldFiles.add(oldFile);
-      }
-    }
-
-    // Look for files in the new program which haven't changed.
+    // Compute the set of files that are unchanged (both in themselves and their dependencies).
     for (const newFile of newProgram.getSourceFiles()) {
       if (oldFiles.has(newFile)) {
-        unchangedFiles.add(newFile);
-
-        // Copy over metadata for the unchanged file if available.
-        if (previousState.metadata.has(newFile)) {
-          metadata.set(newFile, previousState.metadata.get(newFile) !);
+        const oldDeps = previousState.getFileDependencies(newFile);
+        if (oldDeps.every(oldDep => newFiles.has(oldDep))) {
+          // The file and its dependencies are unchanged.
+          unchangedFiles.add(newFile);
+          // Copy over its metadata too
+          const meta = previousState.metadata.get(newFile);
+          if (meta) {
+            metadata.set(newFile, meta);
+          }
         }
+      } else if (newFile.isDeclarationFile) {
+        // A typings file has changed so trigger a full rebuild of the Angular analyses
+        return IncrementalState.fresh();
       }
     }
 
@@ -52,42 +53,18 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
     return new IncrementalState(new Set<ts.SourceFile>(), new Map<ts.SourceFile, FileMetadata>());
   }
 
-  safeToSkipEmit(sf: ts.SourceFile): boolean {
-    if (!this.unchangedFiles.has(sf)) {
-      // The file has changed since the last run, and must be re-emitted.
-      return false;
-    }
-
-    // The file hasn't changed since the last emit. Whether or not it's safe to emit depends on
-    // what metadata was gathered about the file.
-
-    if (!this.metadata.has(sf)) {
-      // The file has no metadata from the previous or current compilations, so it must be emitted.
-      return false;
-    }
-
-    const meta = this.metadata.get(sf) !;
-
-    // Check if this file was explicitly marked as safe. This would only be done if every
-    // `DecoratorHandler` agreed that the file didn't depend on any other file's contents.
-    if (meta.safeToSkipEmitIfUnchanged) {
-      return true;
-    }
-
-    // The file wasn't explicitly marked as safe to skip emitting, so require an emit.
-    return false;
-  }
-
-  markFileAsSafeToSkipEmitIfUnchanged(sf: ts.SourceFile): void {
-    const metadata = this.ensureMetadata(sf);
-    metadata.safeToSkipEmitIfUnchanged = true;
-  }
+  safeToSkip(sf: ts.SourceFile): boolean { return this.unchangedFiles.has(sf); }
 
   trackFileDependency(dep: ts.SourceFile, src: ts.SourceFile) {
     const metadata = this.ensureMetadata(src);
     metadata.fileDependencies.add(dep);
   }
 
+  getFileDependencies(file: ts.SourceFile): ts.SourceFile[] {
+    const meta = this.metadata.get(file);
+    return meta ? 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;
@@ -126,8 +103,6 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
  * Information about the whether a source file can have analysis or emission can be skipped.
  */
 class FileMetadata {
-  /** True if this file has no dependency changes that require it to be re-emitted. */
-  safeToSkipEmitIfUnchanged = false;
   /** A set of source files that this file depends upon. */
   fileDependencies = new Set<ts.SourceFile>();
   directiveMeta = new Map<ClassDeclaration, DirectiveMeta>();

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

@@ -347,7 +347,7 @@ export class NgtscProgram implements api.Program {
         continue;
       }
 
-      if (this.incrementalState.safeToSkipEmit(targetSourceFile)) {
+      if (this.incrementalState.safeToSkip(targetSourceFile)) {
         continue;
       }
 

packages/compiler-cli/src/ngtsc/transform/src/api.ts

@@ -110,7 +110,6 @@ export interface AnalysisOutput<A> {
   diagnostics?: ts.Diagnostic[];
   factorySymbolName?: string;
   typeCheck?: boolean;
-  allowSkipAnalysisAndEmit?: boolean;
 }
 
 /**

packages/compiler-cli/src/ngtsc/transform/src/compilation.ts

@@ -172,11 +172,9 @@ export class IvyCompilation {
   private analyze(sf: ts.SourceFile, preanalyze: true): Promise<void>|undefined;
   private analyze(sf: ts.SourceFile, preanalyze: boolean): Promise<void>|undefined {
     const promises: Promise<void>[] = [];
-
-    // This flag begins as true for the file. If even one handler is matched and does not explicitly
-    // state that analysis/emit can be skipped, then the flag will be set to false.
-    let allowSkipAnalysisAndEmit = true;
-
+    if (this.incrementalState.safeToSkip(sf)) {
+      return;
+    }
     const analyzeClass = (node: ClassDeclaration): void => {
       const ivyClass = this.detectHandlersForClass(node);
 
@@ -203,12 +201,6 @@ export class IvyCompilation {
                 this.sourceToFactorySymbols.has(sf.fileName)) {
               this.sourceToFactorySymbols.get(sf.fileName) !.add(match.analyzed.factorySymbolName);
             }
-
-            // Update the allowSkipAnalysisAndEmit flag - it will only remain true if match.analyzed
-            // also explicitly specifies a value of true for the flag.
-            allowSkipAnalysisAndEmit =
-                allowSkipAnalysisAndEmit && (!!match.analyzed.allowSkipAnalysisAndEmit);
-
           } catch (err) {
             if (err instanceof FatalDiagnosticError) {
               this._diagnostics.push(err.toDiagnostic());
@@ -251,19 +243,9 @@ export class IvyCompilation {
 
     visit(sf);
 
-    const updateIncrementalState = () => {
-      if (allowSkipAnalysisAndEmit) {
-        this.incrementalState.markFileAsSafeToSkipEmitIfUnchanged(sf);
-      }
-    };
-
     if (preanalyze && promises.length > 0) {
-      return Promise.all(promises).then(() => {
-        updateIncrementalState();
-        return undefined;
-      });
+      return Promise.all(promises).then(() => undefined);
     } else {
-      updateIncrementalState();
       return undefined;
     }
   }

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

@@ -16,7 +16,7 @@ describe('ngtsc incremental compilation', () => {
     env.tsconfig();
   });
 
-  it('should compile incrementally', () => {
+  it('should skip unchanged services', () => {
     env.write('service.ts', `
       import {Injectable} from '@angular/core';
 
@@ -40,11 +40,125 @@ describe('ngtsc incremental compilation', () => {
     env.driveMain();
     const written = env.getFilesWrittenSinceLastFlush();
 
-    // The component should be recompiled, but not the service.
+    // The changed file should be recompiled, but not the service.
     expect(written).toContain('/test.js');
     expect(written).not.toContain('/service.js');
   });
 
+  it('should rebuild components that have changed', () => {
+    env.write('component1.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'cmp', template: 'cmp'})
+      export class Cmp1 {}
+    `);
+    env.write('component2.ts', `
+      import {Component} from '@angular/core';
+
+      @Component({selector: 'cmp2', template: 'cmp'})
+      export class Cmp2 {}
+    `);
+    env.driveMain();
+
+    // Pretend a change was made to Cmp1
+    env.flushWrittenFileTracking();
+    env.invalidateCachedFile('component1.ts');
+    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';
+
+      @Component({selector: 'cmp', template: 'cmp'})
+      export class Cmp1 {}
+    `);
+    env.write('component2.ts', `
+      import {Component} from '@angular/core';
+      import {SELECTOR} from './constants';
+
+      @Component({selector: SELECTOR, template: 'cmp'})
+      export class Cmp2 {}
+    `);
+    env.write('constants.ts', `
+      export const SELECTOR = 'cmp';
+    `);
+    env.driveMain();
+
+    // Pretend a change was made to SELECTOR
+    env.flushWrittenFileTracking();
+    env.invalidateCachedFile('constants.ts');
+    env.driveMain();
+    const written = env.getFilesWrittenSinceLastFlush();
+    expect(written).toContain('/constants.js');
+    expect(written).not.toContain('/component1.js');
+    expect(written).toContain('/component2.js');
+  });
+
+  it('should rebuild components whose imported dependencies have changed', () => {
+    setupFooBarProgram(env);
+
+    // Pretend a change was made to BarDir.
+    env.invalidateCachedFile('bar_directive.ts');
+    env.driveMain();
+
+    let written = env.getFilesWrittenSinceLastFlush();
+    expect(written).toContain('/bar_directive.js');
+    expect(written).toContain('/bar_component.js');
+    expect(written).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');
+  });
+
+  it('should rebuild components where their NgModule declared dependencies have changed', () => {
+    setupFooBarProgram(env);
+
+    // Pretend a change was made to FooPipe.
+    env.invalidateCachedFile('foo_pipe.ts');
+    env.driveMain();
+    const written = env.getFilesWrittenSinceLastFlush();
+    expect(written).not.toContain('/bar_directive.js');
+    expect(written).not.toContain('/bar_component.js');
+    expect(written).not.toContain('/bar_module.js');
+    expect(written).toContain('/foo_component.js');
+    expect(written).toContain('/foo_pipe.js');
+    expect(written).toContain('/foo_module.js');
+  });
+
+  it('should rebuild components where their NgModule has changed', () => {
+    setupFooBarProgram(env);
+
+    // Pretend a change was made to FooPipe.
+    env.invalidateCachedFile('foo_module.ts');
+    env.driveMain();
+    const written = env.getFilesWrittenSinceLastFlush();
+    expect(written).not.toContain('/bar_directive.js');
+    expect(written).not.toContain('/bar_component.js');
+    expect(written).not.toContain('/bar_module.js');
+    expect(written).toContain('/foo_component.js');
+    expect(written).toContain('/foo_pipe.js');
+    expect(written).toContain('/foo_module.js');
+  });
+
+  it('should rebuild everything if a typings file changes', () => {
+    setupFooBarProgram(env);
+
+    // Pretend a change was made to a typings file.
+    env.invalidateCachedFile('foo_selector.d.ts');
+    env.driveMain();
+    const written = env.getFilesWrittenSinceLastFlush();
+    expect(written).toContain('/bar_directive.js');
+    expect(written).toContain('/bar_component.js');
+    expect(written).toContain('/bar_module.js');
+    expect(written).toContain('/foo_component.js');
+    expect(written).toContain('/foo_pipe.js');
+    expect(written).toContain('/foo_module.js');
+  });
+
   it('should compile incrementally with template type-checking turned on', () => {
     env.tsconfig({ivyTemplateTypeCheck: true});
     env.write('main.ts', 'export class Foo {}');
@@ -54,4 +168,58 @@ describe('ngtsc incremental compilation', () => {
     // If program reuse were configured incorrectly (as was responsible for
     // https://github.com/angular/angular/issues/30079), this would have crashed.
   });
-});
+});
+
+function setupFooBarProgram(env: NgtscTestEnvironment) {
+  env.write('foo_component.ts', `
+    import {Component} from '@angular/core';
+    import {fooSelector} from './foo_selector';
+
+    @Component({selector: fooSelector, template: 'foo'})
+    export class FooCmp {}
+  `);
+  env.write('foo_pipe.ts', `
+    import {Pipe} from '@angular/core';
+
+    @Pipe({name: 'foo'})
+    export class FooPipe {}
+  `);
+  env.write('foo_module.ts', `
+    import {NgModule} from '@angular/core';
+    import {FooCmp} from './foo_component';
+    import {FooPipe} from './foo_pipe';
+    import {BarModule} from './bar_module';
+    @NgModule({
+      declarations: [FooCmp, FooPipe],
+      imports: [BarModule],
+    })
+    export class FooModule {}
+  `);
+  env.write('bar_component.ts', `
+    import {Component} from '@angular/core';
+
+    @Component({selector: 'bar', template: 'bar'})
+    export class BarCmp {}
+  `);
+  env.write('bar_directive.ts', `
+    import {Directive} from '@angular/core';
+
+    @Directive({selector: '[bar]'})
+    export class BarDir {}
+  `);
+  env.write('bar_module.ts', `
+    import {NgModule} from '@angular/core';
+    import {BarCmp} from './bar_component';
+    import {BarDir} from './bar_directive';
+    @NgModule({
+      declarations: [BarCmp, BarDir],
+      exports: [BarCmp],
+    })
+    export class BarModule {}
+  `);
+  env.write('foo_selector.d.ts', `
+    export const fooSelector = 'foo';
+  `);
+  env.driveMain();
+  env.flushWrittenFileTracking();
+}