diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index d113fa3b25..31edc42660 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -50,8 +50,6 @@ export class InjectableDecoratorHandler implements analyze(node: ClassDeclaration, decorator: Decorator): AnalysisOutput { 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, diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts index 2b6ca5c236..fd9d354ca9 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts +++ b/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(); const metadata = new Map(); + const oldFiles = new Set(oldProgram.getSourceFiles()); + const newFiles = new Set(newProgram.getSourceFiles()); - // Compute the set of files that's unchanged. - const oldFiles = new Set(); - 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(), new Map()); } - 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): 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(); directiveMeta = new Map(); diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 6bbc1fe54c..c3ab8ca2de 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/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; } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index e8f8f7a4ea..76a627818a 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -110,7 +110,6 @@ export interface AnalysisOutput { diagnostics?: ts.Diagnostic[]; factorySymbolName?: string; typeCheck?: boolean; - allowSkipAnalysisAndEmit?: boolean; } /** diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index f23c2a19f6..cd9e298c4b 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -172,11 +172,9 @@ export class IvyCompilation { private analyze(sf: ts.SourceFile, preanalyze: true): Promise|undefined; private analyze(sf: ts.SourceFile, preanalyze: boolean): Promise|undefined { const promises: Promise[] = []; - - // 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; } } diff --git a/packages/compiler-cli/test/ngtsc/incremental_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_spec.ts index f78fbe1b31..bb94d9a796 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_spec.ts +++ b/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. }); -}); \ No newline at end of file +}); + +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(); +} \ No newline at end of file