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.
angular · Nov 1, 2018 · 9b26f9b · 9b26f9b
1 parent cd0b01a
commit 9b26f9b
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 33 deletions.
diff --git a/packages/ngtools/webpack/src/angular_compiler_plugin.ts b/packages/ngtools/webpack/src/angular_compiler_plugin.ts
@@ -39,7 +39,7 @@ import { Compiler, compilation } from 'webpack';
 import { time, timeEnd } from './benchmark';
 import { WebpackCompilerHost, workaroundResolve } from './compiler_host';
 import { resolveEntryModuleFromMain } from './entry_resolver';
-import { gatherDiagnostics, hasErrors } from './gather_diagnostics';
+import { DiagnosticMode, gatherDiagnostics, hasErrors } from './gather_diagnostics';
 import { TypeScriptPathsPlugin } from './paths-plugin';
 import { WebpackResourceLoader } from './resource_loader';
 import {
@@ -284,6 +284,7 @@ export class AngularCompilerPlugin {
     if (options.forkTypeChecker !== undefined) {
       this._forkTypeChecker = options.forkTypeChecker;
     }
+    // this._forkTypeChecker = false;
 
     // Add custom platform transformers.
     if (options.platformTransformers !== undefined) {
@@ -1038,6 +1039,8 @@ export class AngularCompilerPlugin {
     time('AngularCompilerPlugin._emit');
     const program = this._program;
     const allDiagnostics: Array<ts.Diagnostic | Diagnostic> = [];
+    const diagMode = (this._firstRun || !this._forkTypeChecker) ?
+      DiagnosticMode.All : DiagnosticMode.Syntactic;
 
     let emitResult: ts.EmitResult | undefined;
     try {
@@ -1051,10 +1054,8 @@ export class AngularCompilerPlugin {
           timeEnd('AngularCompilerPlugin._emit.ts.getOptionsDiagnostics');
         }
 
-        if ((this._firstRun || !this._forkTypeChecker) && this._program) {
-          allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode,
-            'AngularCompilerPlugin._emit.ts'));
-        }
+        allDiagnostics.push(...gatherDiagnostics(tsProgram, this._JitMode,
+          'AngularCompilerPlugin._emit.ts', diagMode));
 
         if (!hasErrors(allDiagnostics)) {
           if (this._firstRun || sourceFiles.length > 20) {
@@ -1098,10 +1099,8 @@ export class AngularCompilerPlugin {
           timeEnd('AngularCompilerPlugin._emit.ng.getNgOptionDiagnostics');
         }
 
-        if ((this._firstRun || !this._forkTypeChecker) && this._program) {
-          allDiagnostics.push(...gatherDiagnostics(this._program, this._JitMode,
-            'AngularCompilerPlugin._emit.ng'));
-        }
+        allDiagnostics.push(...gatherDiagnostics(angularProgram, this._JitMode,
+          'AngularCompilerPlugin._emit.ng', diagMode));
 
         if (!hasErrors(allDiagnostics)) {
           time('AngularCompilerPlugin._emit.ng.emit');

diff --git a/packages/ngtools/webpack/src/gather_diagnostics.ts b/packages/ngtools/webpack/src/gather_diagnostics.ts
@@ -9,6 +9,13 @@ import { Diagnostic, Diagnostics, Program } from '@angular/compiler-cli';
 import * as ts from 'typescript';
 import { time, timeEnd } from './benchmark';
 
+export enum DiagnosticMode {
+  Syntactic = 1 << 0,
+  Semantic = 1 << 1,
+
+  All = Syntactic | Semantic,
+  Default = All,
+}
 
 export class CancellationToken implements ts.CancellationToken {
   private _isCancelled = false;
@@ -36,6 +43,7 @@ export function gatherDiagnostics(
   program: ts.Program | Program,
   jitMode: boolean,
   benchmarkLabel: string,
+  mode = DiagnosticMode.All,
   cancellationToken?: CancellationToken,
 ): Diagnostics {
   const allDiagnostics: Array<ts.Diagnostic | Diagnostic> = [];
@@ -52,34 +60,44 @@ export function gatherDiagnostics(
     }
   }
 
+  const gatherSyntacticDiagnostics = (mode & DiagnosticMode.Syntactic) != 0;
+  const gatherSemanticDiagnostics = (mode & DiagnosticMode.Semantic) != 0;
+
   if (jitMode) {
     const tsProgram = program as ts.Program;
-    // Check syntactic diagnostics.
-    time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
-    checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
-
-    // Check semantic diagnostics.
-    time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
-    checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
+    if (gatherSyntacticDiagnostics) {
+      // Check syntactic diagnostics.
+      time(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
+      checkDiagnostics(tsProgram.getSyntacticDiagnostics.bind(tsProgram));
+      timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSyntacticDiagnostics`);
+    }
+
+    if (gatherSemanticDiagnostics) {
+      // Check semantic diagnostics.
+      time(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
+      checkDiagnostics(tsProgram.getSemanticDiagnostics.bind(tsProgram));
+      timeEnd(`${benchmarkLabel}.gatherDiagnostics.ts.getSemanticDiagnostics`);
+    }
   } else {
     const angularProgram = program as Program;
+    if (gatherSyntacticDiagnostics) {
+      // Check TypeScript syntactic diagnostics.
+      time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
+      checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram));
+      timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
+    }
 
-    // Check TypeScript syntactic diagnostics.
-    time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
-    checkDiagnostics(angularProgram.getTsSyntacticDiagnostics.bind(angularProgram));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSyntacticDiagnostics`);
-
-    // Check TypeScript semantic and Angular structure diagnostics.
-    time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
-    checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
+    if (gatherSemanticDiagnostics) {
+      // Check TypeScript semantic and Angular structure diagnostics.
+      time(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
+      checkDiagnostics(angularProgram.getTsSemanticDiagnostics.bind(angularProgram));
+      timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
 
-    // Check Angular semantic diagnostics
-    time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
-    checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
+      // Check Angular semantic diagnostics
+      time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
+      checkDiagnostics(angularProgram.getNgSemanticDiagnostics.bind(angularProgram));
+      timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
+    }
   }
 
   return allDiagnostics;

diff --git a/packages/ngtools/webpack/src/type_checker.ts b/packages/ngtools/webpack/src/type_checker.ts
@@ -18,7 +18,7 @@ import {
 import * as ts from 'typescript';
 import { time, timeEnd } from './benchmark';
 import { WebpackCompilerHost } from './compiler_host';
-import { CancellationToken, gatherDiagnostics } from './gather_diagnostics';
+import { CancellationToken, DiagnosticMode, gatherDiagnostics } from './gather_diagnostics';
 import { LogMessage, TypeCheckerMessage } from './type_checker_messages';
 
 
@@ -101,7 +101,7 @@ export class TypeChecker {
 
   private _diagnose(cancellationToken: CancellationToken) {
     const allDiagnostics = gatherDiagnostics(
-      this._program, this._JitMode, 'TypeChecker', cancellationToken);
+      this._program, this._JitMode, 'TypeChecker', DiagnosticMode.Semantic, cancellationToken);
 
     // Report diagnostics.
     if (!cancellationToken.isCancellationRequested()) {