From 3938563565dde9d22d0a004b12ac849bc1e046ae Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 24 Apr 2019 10:26:47 -0700 Subject: [PATCH] fix(ivy): don't reuse a ts.Program more than once in ngtsc (#30090) ngtsc previously could attempt to reuse the main ts.Program twice. This occurred when template type-checking was enabled and then an incremental build was performed. This breaks a TypeScript invariant - ts.Programs can only be reused once. The creation of the template type-checking program reuses the main program, rendering it moot. Then, on the next incremental build the main program would be subject to reuse again, which would crash inside TypeScript. This commit fixes the issue by reusing the template type-checking program from the previous run on the next incremental build. Since under normal circumstances the files in the type-checking program aren't changed, this should be just as fast. Testing strategy: a test is added in the incremental_spec which validates that program reuse with type-checking turned on does not crash the compiler. Fixes #30079 PR Close #30090 --- packages/compiler-cli/src/ngtsc/program.ts | 13 +++++--- .../src/ngtsc/typecheck/src/context.ts | 33 ++++++++++++------- .../compiler-cli/src/transformers/program.ts | 2 +- .../test/ngtsc/incremental_spec.ts | 10 ++++++ 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index a145cc98ba..57fb55efe7 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -37,6 +37,7 @@ import {getRootDirs, isDtsPath} from './util/src/typescript'; export class NgtscProgram implements api.Program { private tsProgram: ts.Program; + private reuseTsProgram: ts.Program; private resourceManager: HostResourceLoader; private compilation: IvyCompilation|undefined = undefined; private factoryToSourceInfo: Map|null = null; @@ -68,7 +69,7 @@ export class NgtscProgram implements api.Program { constructor( rootNames: ReadonlyArray, private options: api.CompilerOptions, - host: api.CompilerHost, oldProgram?: api.Program) { + host: api.CompilerHost, oldProgram?: NgtscProgram) { if (shouldEnablePerfTracing(options)) { this.perfTracker = PerfTracker.zeroedToNow(); this.perfRecorder = this.perfTracker; @@ -146,7 +147,8 @@ export class NgtscProgram implements api.Program { } this.tsProgram = - ts.createProgram(rootFiles, options, this.host, oldProgram && oldProgram.getTsProgram()); + ts.createProgram(rootFiles, options, this.host, oldProgram && oldProgram.reuseTsProgram); + this.reuseTsProgram = this.tsProgram; this.entryPoint = entryPoint !== null ? this.tsProgram.getSourceFile(entryPoint) || null : null; this.moduleResolver = new ModuleResolver(this.tsProgram, options, this.host); @@ -155,9 +157,8 @@ export class NgtscProgram implements api.Program { if (oldProgram === undefined) { this.incrementalState = IncrementalState.fresh(); } else { - const oldNgtscProgram = oldProgram as NgtscProgram; this.incrementalState = IncrementalState.reconcile( - oldNgtscProgram.incrementalState, oldNgtscProgram.tsProgram, this.tsProgram); + oldProgram.incrementalState, oldProgram.reuseTsProgram, this.tsProgram); } } @@ -415,8 +416,10 @@ export class NgtscProgram implements api.Program { // Get the diagnostics. const typeCheckSpan = this.perfRecorder.start('typeCheckDiagnostics'); - const diagnostics = ctx.calculateTemplateDiagnostics(this.tsProgram, this.host, this.options); + const {diagnostics, program} = + ctx.calculateTemplateDiagnostics(this.tsProgram, this.host, this.options); this.perfRecorder.stop(typeCheckSpan); + this.reuseTsProgram = program; return diagnostics; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 6d545eb1a8..e6571e73ce 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -164,7 +164,10 @@ export class TypeCheckContext { calculateTemplateDiagnostics( originalProgram: ts.Program, originalHost: ts.CompilerHost, - originalOptions: ts.CompilerOptions): ts.Diagnostic[] { + originalOptions: ts.CompilerOptions): { + diagnostics: ts.Diagnostic[], + program: ts.Program, + } { const typeCheckSf = this.typeCheckFile.render(); // First, build the map of original source files. const sfMap = new Map(); @@ -191,17 +194,23 @@ export class TypeCheckContext { diagnostics.push(...typeCheckProgram.getSemanticDiagnostics(sf)); } - return diagnostics.filter((diag: ts.Diagnostic): boolean => { - if (diag.code === 6133 /* $var is declared but its value is never read. */) { - return false; - } else if (diag.code === 6199 /* All variables are unused. */) { - return false; - } else if ( - diag.code === 2695 /* Left side of comma operator is unused and has no side effects. */) { - return false; - } - return true; - }); + return { + diagnostics: diagnostics.filter( + (diag: ts.Diagnostic): + boolean => { + if (diag.code === 6133 /* $var is declared but its value is never read. */) { + return false; + } else if (diag.code === 6199 /* All variables are unused. */) { + return false; + } else if ( + diag.code === + 2695 /* Left side of comma operator is unused and has no side effects. */) { + return false; + } + return true; + }), + program: typeCheckProgram, + }; } private addInlineTypeCheckBlock( diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index f14c259466..0935a87b30 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -899,7 +899,7 @@ export function createProgram({rootNames, options, host, oldProgram}: { host: CompilerHost, oldProgram?: Program }): Program { if (options.enableIvy === true) { - return new NgtscProgram(rootNames, options, host, oldProgram); + return new NgtscProgram(rootNames, options, host, oldProgram as NgtscProgram); } else if (options.enableIvy === 'tsc') { return new TscPassThroughProgram(rootNames, options, host, oldProgram); } diff --git a/packages/compiler-cli/test/ngtsc/incremental_spec.ts b/packages/compiler-cli/test/ngtsc/incremental_spec.ts index 5092ea4d86..f78fbe1b31 100644 --- a/packages/compiler-cli/test/ngtsc/incremental_spec.ts +++ b/packages/compiler-cli/test/ngtsc/incremental_spec.ts @@ -44,4 +44,14 @@ describe('ngtsc incremental compilation', () => { expect(written).toContain('/test.js'); expect(written).not.toContain('/service.js'); }); + + it('should compile incrementally with template type-checking turned on', () => { + env.tsconfig({ivyTemplateTypeCheck: true}); + env.write('main.ts', 'export class Foo {}'); + env.driveMain(); + env.invalidateCachedFile('main.ts'); + env.driveMain(); + // 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