Skip to content

Commit 9b26f9b

Browse files
filipesilvaalexeagle
filipesilva
authored and
alexeagle
committed
feat(@ngtools/webpack): only do type checking on forked type checker
Syntactic errors were only being reported in the type checker when it was running. This caused rebuilds to finish successfully if they had syntactic errors, which could lead to very weird behaviour on rebuilds.
1 parent cd0b01a commit 9b26f9b

File tree

3 files changed

+50
-33
lines changed

3 files changed

+50
-33
lines changed

‎packages/ngtools/webpack/src/angular_compiler_plugin.ts‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { Compiler, compilation } from 'webpack';
3939
import { time, timeEnd } from './benchmark';
4040
import { WebpackCompilerHost, workaroundResolve } from './compiler_host';
4141
import { resolveEntryModuleFromMain } from './entry_resolver';
42-
import { gatherDiagnostics, hasErrors } from './gather_diagnostics';
42+
import { DiagnosticMode, gatherDiagnostics, hasErrors } from './gather_diagnostics';
4343
import { TypeScriptPathsPlugin } from './paths-plugin';
4444
import { WebpackResourceLoader } from './resource_loader';
4545
import {
@@ -284,6 +284,7 @@ export class AngularCompilerPlugin {
284284
if (options.forkTypeChecker !== undefined) {
285285
this._forkTypeChecker = options.forkTypeChecker;
286286
}
287+
// this._forkTypeChecker = false;
287288

288289
// Add custom platform transformers.
289290
if (options.platformTransformers !== undefined) {
@@ -1038,6 +1039,8 @@ export class AngularCompilerPlugin {
10381039
time('AngularCompilerPlugin._emit');
10391040
const program = this._program;
10401041
const allDiagnostics: Array<ts.Diagnostic | Diagnostic> = [];
1042+
const diagMode = (this._firstRun || !this._forkTypeChecker) ?
1043+
DiagnosticMode.All : DiagnosticMode.Syntactic;
10411044

10421045
let emitResult: ts.EmitResult | undefined;
10431046
try {
@@ -1051,10 +1054,8 @@ export class AngularCompilerPlugin {
10511054
timeEnd('AngularCompilerPlugin._emit.ts.getOptionsDiagnostics');
10521055
}
10531056

1054-
if ((this._firstRun || !this._forkTypeChecker) && this._program) {
1055-
allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode,
1056-
'AngularCompilerPlugin._emit.ts'));
1057-
}
1057+
allDiagnostics.push(...gatherDiagnostics(tsProgram, this._JitMode,
1058+
'AngularCompilerPlugin._emit.ts', diagMode));
10581059

10591060
if (!hasErrors(allDiagnostics)) {
10601061
if (this._firstRun || sourceFiles.length > 20) {
@@ -1098,10 +1099,8 @@ export class AngularCompilerPlugin {
10981099
timeEnd('AngularCompilerPlugin._emit.ng.getNgOptionDiagnostics');
10991100
}
11001101

1101-
if ((this._firstRun || !this._forkTypeChecker) && this._program) {
1102-
allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode,
1103-
'AngularCompilerPlugin._emit.ng'));
1104-
}
1102+
allDiagnostics.push(...gatherDiagnostics(angularProgram, this._JitMode,
1103+
'AngularCompilerPlugin._emit.ng', diagMode));
11051104

11061105
if (!hasErrors(allDiagnostics)) {
11071106
time('AngularCompilerPlugin._emit.ng.emit');

‎packages/ngtools/webpack/src/gather_diagnostics.ts‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ import { Diagnostic, Diagnostics, Program } from '@angular/compiler-cli';
99
import * as ts from 'typescript';
1010
import { time, timeEnd } from './benchmark';
1111

12+
export enum DiagnosticMode {
13+
Syntactic = 1 << 0,
14+
Semantic = 1 << 1,
15+
16+
All = Syntactic | Semantic,
17+
Default = All,
18+
}
1219

1320
export class CancellationToken implements ts.CancellationToken {
1421
private _isCancelled = false;
@@ -36,6 +43,7 @@ export function gatherDiagnostics(
3643
program: ts.Program | Program,
3744
jitMode: boolean,
3845
benchmarkLabel: string,
46+
mode = DiagnosticMode.All,
3947
cancellationToken?: CancellationToken,
4048
): Diagnostics {
4149
const allDiagnostics: Array<ts.Diagnostic | Diagnostic> = [];
@@ -52,34 +60,44 @@ export function gatherDiagnostics(
5260
}
5361
}
5462

63+
const gatherSyntacticDiagnostics = (mode & DiagnosticMode.Syntactic) != 0;
64+
const gatherSemanticDiagnostics = (mode & DiagnosticMode.Semantic) != 0;
65+
5566
if (jitMode) {
5667
const tsProgram = program as ts.Program;
57-
// Check syntactic diagnostics.
58-
time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
59-
checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram));
60-
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
61-
62-
// Check semantic diagnostics.
63-
time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
64-
checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram));
65-
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
68+
if (gatherSyntacticDiagnostics) {
69+
// Check syntactic diagnostics.
70+
time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
71+
checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram));
72+
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
73+
}
74+
75+
if (gatherSemanticDiagnostics) {
76+
// Check semantic diagnostics.
77+
time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
78+
checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram));
79+
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
80+
}
6681
} else {
6782
const angularProgram = program as Program;
83+
if (gatherSyntacticDiagnostics) {
84+
// Check TypeScript syntactic diagnostics.
85+
time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
86+
checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram));
87+
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
88+
}
6889

69-
// Check TypeScript syntactic diagnostics.
70-
time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
71-
checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram));
72-
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
73-
74-
// Check TypeScript semantic and Angular structure diagnostics.
75-
time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
76-
checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram));
77-
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
90+
if (gatherSemanticDiagnostics) {
91+
// Check TypeScript semantic and Angular structure diagnostics.
92+
time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
93+
checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram));
94+
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
7895

79-
// Check Angular semantic diagnostics
80-
time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
81-
checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram));
82-
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
96+
// Check Angular semantic diagnostics
97+
time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
98+
checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram));
99+
timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
100+
}
83101
}
84102

85103
return allDiagnostics;

‎packages/ngtools/webpack/src/type_checker.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
import * as ts from 'typescript';
1919
import { time, timeEnd } from './benchmark';
2020
import { WebpackCompilerHost } from './compiler_host';
21-
import { CancellationToken, gatherDiagnostics } from './gather_diagnostics';
21+
import { CancellationToken, DiagnosticMode, gatherDiagnostics } from './gather_diagnostics';
2222
import { LogMessage, TypeCheckerMessage } from './type_checker_messages';
2323

2424

@@ -101,7 +101,7 @@ export class TypeChecker {
101101

102102
private _diagnose(cancellationToken: CancellationToken) {
103103
const allDiagnostics = gatherDiagnostics(
104-
this._program, this._JitMode, 'TypeChecker', cancellationToken);
104+
this._program, this._JitMode, 'TypeChecker', DiagnosticMode.Semantic, cancellationToken);
105105

106106
// Report diagnostics.
107107
if (!cancellationToken.isCancellationRequested()) {

0 commit comments

Comments
 (0)