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
This commit is contained in:
Alex Rickabaugh 2019-04-24 10:26:47 -07:00 committed by Andrew Kushnir
parent 00ea3983e3
commit 3938563565
4 changed files with 40 additions and 18 deletions

View File

@ -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<string, FactoryInfo>|null = null;
@ -68,7 +69,7 @@ export class NgtscProgram implements api.Program {
constructor(
rootNames: ReadonlyArray<string>, 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;
}

View File

@ -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<string, ts.SourceFile>();
@ -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(

View File

@ -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);
}

View File

@ -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.
});
});