fix(@ngtools/webpack): always emit on first build

We want to allow emitting with errors on the first run so that imports can be added to the webpack dependency tree and rebuilds triggered by file edits.

This will not cause webpack to finish the compilation successfully on error, it will just allow it to follow imports.

Fix #7890

Author: filipesilva

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

@@ -100,7 +100,6 @@ export class AngularCompilerPlugin implements Tapable {
   private _donePromise: Promise<void> | null;
   private _compiler: any = null;
   private _compilation: any = null;
-  private _failedCompilation = false;
 
   // TypeChecker process.
   private _forkTypeChecker = true;
@@ -114,7 +113,6 @@ export class AngularCompilerPlugin implements Tapable {
 
   get options() { return this._options; }
   get done() { return this._donePromise; }
-  get failedCompilation() { return this._failedCompilation; }
   get entryModule() {
     const splitted = this._entryModule.split('#');
     const path = splitted[0];
@@ -311,13 +309,19 @@ export class AngularCompilerPlugin implements Tapable {
       this._updateForkedTypeChecker(changedTsFiles);
     }
 
-    if (this._JitMode) {
+    // We want to allow emitting with errors on the first run so that imports can be added
+    // to the webpack dependency tree and rebuilds triggered by file edits.
+    const compilerOptions = {
+      ...this._angularCompilerOptions,
+      noEmitOnError: !this._firstRun
+    };
 
+    if (this._JitMode) {
       // Create the TypeScript program.
       time('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram');
       this._program = ts.createProgram(
         this._tsFilenames,
-        this._angularCompilerOptions,
+        compilerOptions,
         this._angularCompilerHost,
         this._program as ts.Program
       );
@@ -329,7 +333,7 @@ export class AngularCompilerPlugin implements Tapable {
       // Create the Angular program.
       this._program = createProgram({
         rootNames: this._tsFilenames,
-        options: this._angularCompilerOptions,
+        options: compilerOptions,
         host: this._angularCompilerHost,
         oldProgram: this._program as Program
       });
@@ -527,7 +531,6 @@ export class AngularCompilerPlugin implements Tapable {
     compiler.plugin('done', () => {
       this._donePromise = null;
       this._compilation = null;
-      this._failedCompilation = false;
     });
 
     // TODO: consider if it's better to remove this plugin and instead make it wait on the
@@ -604,7 +607,6 @@ export class AngularCompilerPlugin implements Tapable {
         timeEnd('AngularCompilerPlugin._make');
         cb();
       }, (err: any) => {
-        this._failedCompilation = true;
         compilation.errors.push(err.stack);
         timeEnd('AngularCompilerPlugin._make');
         cb();
@@ -726,8 +728,6 @@ export class AngularCompilerPlugin implements Tapable {
           // Reset changed files on successful compilation.
           if (emitResult && !emitResult.emitSkipped && this._compilation.errors.length === 0) {
             this._compilerHost.resetChangedFileTracker();
-          } else {
-            this._failedCompilation = true;
           }
         }
         timeEnd('AngularCompilerPlugin._update');
@@ -789,7 +789,9 @@ export class AngularCompilerPlugin implements Tapable {
             'AngularCompilerPlugin._emit.ts'));
         }
 
-        if (!hasErrors(allDiagnostics)) {
+        // Always emit on the first run, so that imports are processed by webpack and the user
+        // can trigger a rebuild by editing any file.
+        if (!hasErrors(allDiagnostics) || this._firstRun) {
           sourceFiles.forEach((sf) => {
             const timeLabel = `AngularCompilerPlugin._emit.ts+${sf.fileName}+.emit`;
             time(timeLabel);
@@ -820,7 +822,9 @@ export class AngularCompilerPlugin implements Tapable {
             'AngularCompilerPlugin._emit.ng'));
         }
 
-        if (!hasErrors(allDiagnostics)) {
+        // Always emit on the first run, so that imports are processed by webpack and the user
+        // can trigger a rebuild by editing any file.
+        if (!hasErrors(allDiagnostics) || this._firstRun) {
           time('AngularCompilerPlugin._emit.ng.emit');
           const extractI18n = !!this._angularCompilerOptions.i18nOutFile;
           const emitFlags = extractI18n ? EmitFlags.I18nBundle : EmitFlags.Default;

packages/@ngtools/webpack/src/loader.ts

@@ -544,19 +544,14 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
         .then(() => {
           timeEnd(timeLabel + '.ngcLoader.AngularCompilerPlugin');
           const result = plugin.getFile(sourceFileName);
-          if (plugin.failedCompilation) {
-            // Return an empty string if there is no result to prevent extra loader errors.
-            // Plugin errors were already pushed to the compilation errors.
-            timeEnd(timeLabel);
-            cb(null, result.outputText || '', result.sourceMap);
-          } else {
-            timeEnd(timeLabel);
+          timeEnd(timeLabel);
+          if (result.outputText === undefined) {
+            throw new Error('TypeScript compilation failed.');
+          }
           cb(null, result.outputText, result.sourceMap);
-        }
-      })
-      .catch(err => {
-        timeEnd(timeLabel + '.ngcLoader.AngularCompilerPlugin');
-        cb(err);
+        })
+        .catch(err => {
+          cb(err);
         });
     } else if (plugin instanceof AotPlugin) {
       time(timeLabel + '.ngcLoader.AotPlugin');
@@ -645,15 +640,12 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
           timeEnd(timeLabel + '.ngcLoader.AotPlugin.transpile');
 
           timeEnd(timeLabel + '.ngcLoader.AotPlugin');
-          if (plugin.failedCompilation && plugin.compilerOptions.noEmitOnError) {
-            // Return an empty string to prevent extra loader errors (missing imports etc).
-            // Plugin errors were already pushed to the compilation errors.
-            timeEnd(timeLabel);
-            cb(null, '');
-          } else {
-            timeEnd(timeLabel);
-            cb(null, result.outputText, result.sourceMap);
+          timeEnd(timeLabel);
+
+          if (result.outputText === undefined) {
+            throw new Error('TypeScript compilation failed.');
           }
+          cb(null, result.outputText, result.sourceMap);
         })
         .catch(err => cb(err));
       }

packages/@ngtools/webpack/src/plugin.ts

@@ -62,7 +62,6 @@ export class AotPlugin implements Tapable {
   private _donePromise: Promise<void> | null;
   private _compiler: any = null;
   private _compilation: any = null;
-  private _failedCompilation = false;
 
   private _typeCheck = true;
   private _skipCodeGeneration = false;
@@ -90,7 +89,6 @@ export class AotPlugin implements Tapable {
   get compilerHost() { return this._compilerHost; }
   get compilerOptions() { return this._compilerOptions; }
   get done() { return this._donePromise; }
-  get failedCompilation() { return this._failedCompilation; }
   get entryModule() {
     const splitted = this._entryModule.split('#');
     const path = splitted[0];
@@ -426,7 +424,6 @@ export class AotPlugin implements Tapable {
     compiler.plugin('done', () => {
       this._donePromise = null;
       this._compilation = null;
-      this._failedCompilation = false;
     });
 
     compiler.plugin('after-resolvers', (compiler: any) => {
@@ -638,14 +635,11 @@ export class AotPlugin implements Tapable {
       .then(() => {
         if (this._compilation.errors == 0) {
           this._compilerHost.resetChangedFileTracker();
-        } else {
-          this._failedCompilation = true;
         }
 
         timeEnd('AotPlugin._make');
         cb();
       }, (err: any) => {
-        this._failedCompilation = true;
         compilation.errors.push(err.stack);
         timeEnd('AotPlugin._make');
         cb();

tests/e2e/tests/build/rebuild-error.ts

@@ -0,0 +1,43 @@
+import {
+  killAllProcesses,
+  waitForAnyProcessOutputToMatch,
+  execAndWaitForOutputToMatch,
+} from '../../utils/process';
+import {replaceInFile, appendToFile} from '../../utils/fs';
+import {getGlobalVariable} from '../../utils/env';
+
+
+const failedRe = /webpack: Failed to compile/;
+const successRe = /webpack: Compiled successfully/;
+const errorRe = /ERROR in/;
+
+export default function() {
+  if (process.platform.startsWith('win')) {
+    return Promise.resolve();
+  }
+  // Skip this in ejected tests.
+  if (getGlobalVariable('argv').eject) {
+    return Promise.resolve();
+  }
+
+  return Promise.resolve()
+    // Add an error to a non-main file.
+    .then(() => appendToFile('src/app/app.component.ts', ']]]]]'))
+    // Should have an error.
+    .then(() => execAndWaitForOutputToMatch('ng', ['serve'], failedRe))
+    // Fix the error, should trigger a rebuild.
+    .then(() => Promise.all([
+      waitForAnyProcessOutputToMatch(successRe, 20000),
+      replaceInFile('src/app/app.component.ts', ']]]]]', '')
+    ]))
+    .then((results) => {
+      const stderr = results[0].stderr;
+      if (errorRe.test(stderr)) {
+        throw new Error('Expected no error but an error was shown.');
+      }
+    })
+    .then(() => killAllProcesses(), (err: any) => {
+      killAllProcesses();
+      throw err;
+    });
+}