fix(@ngtools/webpack): fix and improve error reporting

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

@@ -72,12 +72,6 @@ export function gatherDiagnostics(
       checkDiagnostics(angularProgram.getTsSemanticDiagnostics(undefined, cancellationToken));
     timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getTsSemanticDiagnostics`);
 
-    // Check Angular structural diagnostics.
-    time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgStructuralDiagnostics`);
-    checkOtherDiagnostics = checkOtherDiagnostics &&
-      checkDiagnostics(angularProgram.getNgStructuralDiagnostics(cancellationToken));
-    timeEnd(`${benchmarkLabel}.gatherDiagnostics.ng.getNgStructuralDiagnostics`);
-
     // Check Angular semantic diagnostics
     time(`${benchmarkLabel}.gatherDiagnostics.ng.getNgSemanticDiagnostics`);
     checkOtherDiagnostics = checkOtherDiagnostics &&

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

@@ -567,7 +567,7 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
       plugin.done
         .then(() => {
           timeEnd(timeLabel + '.ngcLoader.AngularCompilerPlugin');
-          const result = plugin.getFile(sourceFileName);
+          const result = plugin.getCompiledFile(sourceFileName);
 
           if (result.sourceMap) {
             // Process sourcemaps for Webpack.
@@ -579,12 +579,9 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
             result.sourceMap = JSON.stringify(sourceMap);
           }
 
-          timeEnd(timeLabel);
-          if (result.outputText === undefined) {
-            throw new Error('TypeScript compilation failed.');
-          }
-
           // Dependencies must use system path separator.
+          // TODO: move the denormalizer into it's own helper.
+          result.errorDependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
           const dependencies = plugin.getDependencies(sourceFileName);
           dependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
 
@@ -596,10 +593,11 @@ export function ngcLoader(this: LoaderContext & { _compilation: any }, source: s
             origDependencies.forEach(dep => this.addDependency(dep.replace(/\//g, path.sep)));
           }
 
+          timeEnd(timeLabel);
           cb(null, result.outputText, result.sourceMap);
         })
         .catch(err => {
-          timeEnd(timeLabel + '.ngcLoader.AngularCompilerPlugin');
+          timeEnd(timeLabel);
           cb(err);
         });
     } else if (plugin instanceof AotPlugin) {

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

@@ -51,31 +51,36 @@ let lastCancellationToken: CancellationToken;
 
 process.on('message', (message: TypeCheckerMessage) => {
   time('TypeChecker.message');
-  switch (message.kind) {
-    case MESSAGE_KIND.Init:
-      const initMessage = message as InitMessage;
-      typeChecker = new TypeChecker(
-        initMessage.compilerOptions,
-        initMessage.basePath,
-        initMessage.jitMode,
-        initMessage.tsFilenames,
-      );
-      break;
-    case MESSAGE_KIND.Update:
-      if (!typeChecker) {
-        throw new Error('TypeChecker: update message received before initialization');
-      }
-      if (lastCancellationToken) {
-        // This cancellation token doesn't seem to do much, messages don't seem to be processed
-        // before the diagnostics finish.
-        lastCancellationToken.requestCancellation();
-      }
-      const updateMessage = message as UpdateMessage;
-      lastCancellationToken = new CancellationToken();
-      typeChecker.update(updateMessage.changedTsFiles, lastCancellationToken);
-      break;
-    default:
-      throw new Error(`TypeChecker: Unexpected message received: ${message}.`);
+  try {
+    switch (message.kind) {
+      case MESSAGE_KIND.Init:
+        const initMessage = message as InitMessage;
+        typeChecker = new TypeChecker(
+          initMessage.compilerOptions,
+          initMessage.basePath,
+          initMessage.jitMode,
+          initMessage.tsFilenames,
+        );
+        break;
+      case MESSAGE_KIND.Update:
+        if (!typeChecker) {
+          throw new Error('TypeChecker: update message received before initialization');
+        }
+        if (lastCancellationToken) {
+          // This cancellation token doesn't seem to do much, messages don't seem to be processed
+          // before the diagnostics finish.
+          lastCancellationToken.requestCancellation();
+        }
+        const updateMessage = message as UpdateMessage;
+        lastCancellationToken = new CancellationToken();
+        typeChecker.update(updateMessage.changedTsFiles, lastCancellationToken);
+        break;
+      default:
+        throw new Error(`TypeChecker: Unexpected message received: ${message}.`);
+    }
+  } catch (error) {
+    // Ignore errors in the TypeChecker.
+    // Anything that would throw here will error out the compilation as well.
   }
   timeEnd('TypeChecker.message');
 });

tests/e2e/tests/build/build-errors.ts

@@ -0,0 +1,76 @@
+import { ng } from '../../utils/process';
+import { updateJsonFile } from '../../utils/project';
+import { writeFile, appendToFile, readFile } from '../../utils/fs';
+import { getGlobalVariable } from '../../utils/env';
+import { expectToFail } from '../../utils/utils';
+
+
+const extraErrors = [
+  `Final loader didn't return a Buffer or String`,
+  `doesn't contain a valid alias configuration`,
+  `main.ts is not part of the TypeScript compilation.`,
+];
+
+export default function () {
+  if (process.platform.startsWith('win')) {
+    return Promise.resolve();
+  }
+  // Skip this in ejected tests.
+  if (getGlobalVariable('argv').eject) {
+    return Promise.resolve();
+  }
+
+  // Skip in non-nightly tests. Switch this check around when ng5 is out.
+  if (!getGlobalVariable('argv').nightly) {
+    return Promise.resolve();
+  }
+
+  let origContent: string;
+
+  return Promise.resolve()
+    // Save the original contents of `./src/app/app.component.ts`.
+    .then(() => readFile('./src/app/app.component.ts'))
+    .then((contents) => origContent = contents)
+    // Check `part of the TypeScript compilation` errors.
+    // These should show an error only for the missing file.
+    .then(() => updateJsonFile('./src/tsconfig.app.json', configJson => {
+      configJson.files = ['main.ts'];
+    }))
+    .then(() => expectToFail(() => ng('build')))
+    .then(({ message }) => {
+      if (!message.includes('polyfills.ts is not part of the compilation')) {
+        throw new Error(`Expected missing TS file error, got this instead:\n${message}`);
+      }
+      if (extraErrors.some((e) => message.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${message}`);
+      }
+    })
+    .then(() => updateJsonFile('./src/tsconfig.app.json', configJson => {
+      configJson.files = undefined;
+    }))
+    // Check simple single syntax errors.
+    // These shouldn't skip emit and just show a TS error.
+    .then(() => appendToFile('./src/app/app.component.ts', ']]]'))
+    .then(() => expectToFail(() => ng('build')))
+    .then(({ message }) => {
+      if (!message.includes('Declaration or statement expected.')) {
+        throw new Error(`Expected syntax error, got this instead:\n${message}`);
+      }
+      if (extraErrors.some((e) => message.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${message}`);
+      }
+    })
+    .then(() => writeFile('./src/app/app.component.ts', origContent))
+    // Check errors when files were not emitted.
+    .then(() => writeFile('./src/app/app.component.ts', ''))
+    .then(() => expectToFail(() => ng('build', '--aot')))
+    .then(({ message }) => {
+      if (!message.includes(`Unexpected value 'AppComponent`)) {
+        throw new Error(`Expected static analysis error, got this instead:\n${message}`);
+      }
+      if (extraErrors.some((e) => message.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${message}`);
+      }
+    })
+    .then(() => writeFile('./src/app/app.component.ts', origContent));
+}

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

@@ -3,14 +3,20 @@ import {
   waitForAnyProcessOutputToMatch,
   execAndWaitForOutputToMatch,
 } from '../../utils/process';
-import {replaceInFile, appendToFile} from '../../utils/fs';
-import {getGlobalVariable} from '../../utils/env';
+import { replaceInFile, readFile, writeFile } from '../../utils/fs';
+import { getGlobalVariable } from '../../utils/env';
+import { wait } from '../../utils/utils';
 
 
 const failedRe = /webpack: Failed to compile/;
 const successRe = /webpack: Compiled successfully/;
+const extraErrors = [
+  `Final loader didn't return a Buffer or String`,
+  `doesn't contain a valid alias configuration`,
+  `main.ts is not part of the TypeScript compilation.`,
+];
 
-export default function() {
+export default function () {
   if (process.platform.startsWith('win')) {
     return Promise.resolve();
   }
@@ -19,16 +25,77 @@ export default function() {
     return Promise.resolve();
   }
 
+  // Skip in non-nightly tests. Switch this check around when ng5 is out.
+  if (!getGlobalVariable('argv').nightly) {
+    return Promise.resolve();
+  }
+
+  let origContent: string;
+
   return Promise.resolve()
-    // Add an error to a non-main file.
-    .then(() => appendToFile('src/app/app.component.ts', ']]]]]'))
+    // Save the original contents of `./src/app/app.component.ts`.
+    .then(() => readFile('./src/app/app.component.ts'))
+    .then((contents) => origContent = contents)
+    // Add a major error on a non-main file to the initial build.
+    .then(() => writeFile('src/app/app.component.ts', ''))
     // Should have an error.
-    .then(() => execAndWaitForOutputToMatch('ng', ['serve'], failedRe))
-    // Fix the error, should trigger a rebuild.
+    .then(() => execAndWaitForOutputToMatch('ng', ['serve', '--aot'], failedRe))
+    .then((results) => {
+      const stderr = results.stderr;
+      if (!stderr.includes(`Unexpected value 'AppComponent`)) {
+        throw new Error(`Expected static analysis error, got this instead:\n${stderr}`);
+      }
+      if (extraErrors.some((e) => stderr.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${stderr}`);
+      }
+    })
+    // Fix the error, should trigger a successful rebuild.
+    .then(() => Promise.all([
+      waitForAnyProcessOutputToMatch(successRe, 20000),
+      writeFile('src/app/app.component.ts', origContent)
+    ]))
+    .then(() => wait(2000))
+    // Add an syntax error to a non-main file.
+    // Build should still be successfull and error reported on forked type checker.
+    .then(() => Promise.all([
+      waitForAnyProcessOutputToMatch(successRe, 20000),
+      writeFile('src/app/app.component.ts', origContent + '\n]]]]]')
+    ]))
+    .then((results) => {
+      const stderr = results[0].stderr;
+      if (!stderr.includes('Declaration or statement expected.')) {
+        throw new Error(`Expected syntax error, got this instead:\n${stderr}`);
+      }
+      if (extraErrors.some((e) => stderr.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${stderr}`);
+      }
+    })
+    // Fix the error, should trigger a successful rebuild.
     .then(() => Promise.all([
       waitForAnyProcessOutputToMatch(successRe, 20000),
       replaceInFile('src/app/app.component.ts', ']]]]]', '')
     ]))
+    .then(() => wait(2000))
+    // Add a major error on a rebuild.
+    // Should fail the rebuild.
+    .then(() => Promise.all([
+      waitForAnyProcessOutputToMatch(failedRe, 20000),
+      writeFile('src/app/app.component.ts', '')
+    ]))
+    .then((results) => {
+      const stderr = results[0].stderr;
+      if (!stderr.includes(`Unexpected value 'AppComponent`)) {
+        throw new Error(`Expected static analysis error, got this instead:\n${stderr}`);
+      }
+      if (extraErrors.some((e) => stderr.includes(e))) {
+        throw new Error(`Did not expect extra errors but got:\n${stderr}`);
+      }
+    })
+    // Fix the error, should trigger a successful rebuild.
+    .then(() => Promise.all([
+      waitForAnyProcessOutputToMatch(successRe, 20000),
+      writeFile('src/app/app.component.ts', origContent)
+    ]))
     .then(() => killAllProcesses(), (err: any) => {
       killAllProcesses();
       throw err;

tests/e2e/utils/utils.ts

@@ -1,12 +1,12 @@
 
-export function expectToFail(fn: () => Promise<any>, errorMessage?: string): Promise<void> {
+export function expectToFail(fn: () => Promise<any>, errorMessage?: string): Promise<any> {
   return fn()
     .then(() => {
       const functionSource = fn.name || (<any>fn).source || fn.toString();
       const errorDetails = errorMessage ? `\n\tDetails:\n\t${errorMessage}` : '';
       throw new Error(
         `Function ${functionSource} was expected to fail, but succeeded.${errorDetails}`);
-    }, () => { });
+    }, (err) => { return err; });
 }
 
 export function wait(msecs: number): Promise<void> {