angular · Brocco · Oct 5, 2017 · Oct 4, 2017
diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts
@@ -101,7 +101,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;
@@ -115,7 +114,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];
@@ -327,13 +325,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
       );
@@ -345,7 +349,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
       });
@@ -543,7 +547,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
@@ -618,7 +621,6 @@ export class AngularCompilerPlugin implements Tapable {
         timeEnd('AngularCompilerPlugin._make');
         cb();
       }, (err: any) => {
-        this._failedCompilation = true;
         compilation.errors.push(err.stack);
         timeEnd('AngularCompilerPlugin._make');
         cb();
@@ -740,8 +742,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');
@@ -803,7 +803,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);
@@ -834,7 +836,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;

diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts
@@ -557,15 +557,11 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
             result.sourceMap = JSON.stringify(sourceMap);
           }
 
-          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);
-            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 => {
           timeEnd(timeLabel + '.ngcLoader.AngularCompilerPlugin');
@@ -658,15 +654,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));
       }

diff --git a/packages/@ngtools/webpack/src/plugin.ts b/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];
@@ -428,7 +426,6 @@ export class AotPlugin implements Tapable {
     compiler.plugin('done', () => {
       this._donePromise = null;
       this._compilation = null;
-      this._failedCompilation = false;
     });
 
     compiler.plugin('after-resolvers', (compiler: any) => {
@@ -640,14 +637,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();

diff --git a/tests/e2e/tests/build/rebuild-error.ts b/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;
+    });
+}