perf(@ngtools/webpack): reduce rebuild performance by typechecking more

package.json

@@ -137,7 +137,7 @@
     "@types/rimraf": "0.0.25-alpha",
     "@types/semver": "^5.3.30",
     "@types/source-map": "^0.5.0",
-    "@types/webpack": "^2.2.1",
+    "@types/webpack": "^2.2.4",
     "chai": "^3.5.0",
     "conventional-changelog": "^1.1.0",
     "dtsgenerator": "^0.7.1",

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

@@ -2,8 +2,10 @@ import * as path from 'path';
 import * as ts from 'typescript';
 import {AotPlugin} from './plugin';
 import {TypeScriptFileRefactor} from './refactor';
+import {LoaderContext, ModuleReason} from './webpack';
 
 const loaderUtils = require('loader-utils');
+const NormalModule = require('webpack/lib/NormalModule');
 
 
 function _getContentOfKeyLiteral(source: ts.SourceFile, node: ts.Node): string {
@@ -160,7 +162,7 @@ function _replaceResources(refactor: TypeScriptFileRefactor): void {
 
 
 function _checkDiagnostics(refactor: TypeScriptFileRefactor) {
-  const diagnostics = refactor.getDiagnostics();
+  const diagnostics: ts.Diagnostic[] = refactor.getDiagnostics();
 
   if (diagnostics.length > 0) {
     const message = diagnostics
@@ -175,10 +177,23 @@ function _checkDiagnostics(refactor: TypeScriptFileRefactor) {
 }
 
 
+/**
+ * Recursively calls diagnose on the plugins for all the reverse dependencies.
+ * @private
+ */
+function _diagnoseDeps(reasons: ModuleReason[], plugin: AotPlugin) {
+  reasons
+    .filter(reason => reason && reason.module && reason.module instanceof NormalModule)
+    .forEach(reason => {
+      plugin.diagnose(reason.module.resource);
+      _diagnoseDeps(reason.module.reasons, plugin);
+    });
+}
+
+
 // Super simple TS transpiler loader for testing / isolated usage. does not type check!
-export function ngcLoader(source: string) {
-  this.cacheable();
-  const cb: any = this.async();
+export function ngcLoader(this: LoaderContext & { _compilation: any }) {
+  const cb = this.async();
   const sourceFileName: string = this.resourcePath;
 
   const plugin = this._compilation._ngToolsWebpackPluginInstance as AotPlugin;
@@ -201,7 +216,13 @@ export function ngcLoader(source: string) {
       })
       .then(() => {
         if (plugin.typeCheck) {
-          _checkDiagnostics(refactor);
+          // Check all diagnostics from this and reverse dependencies also.
+          if (!plugin.firstRun) {
+            _diagnoseDeps(this._module.reasons, plugin);
+          }
+          // We do this here because it will throw on error, resulting in rebuilding this file
+          // the next time around if it changes.
+          plugin.diagnose(sourceFileName);
         }
       })
       .then(() => {

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

@@ -57,6 +57,7 @@ export class AotPlugin implements Tapable {
   private _i18nFormat: string;
   private _locale: string;
 
+  private _diagnoseFiles: { [path: string]: boolean } = {};
   private _firstRun = true;
 
   constructor(options: AotPluginOptions) {
@@ -227,7 +228,11 @@ export class AotPlugin implements Tapable {
   apply(compiler: any) {
     this._compiler = compiler;
 
-    compiler.plugin('invalid', (fileName: string, timestamp: number) => {
+    compiler.plugin('invalid', (fileName: string) => {
+      // Turn this off as soon as a file becomes invalid and we're about to start a rebuild.
+      this._firstRun = false;
+      this._diagnoseFiles = {};
+
       this._compilerHost.invalidate(fileName);
     });
 
@@ -291,6 +296,37 @@ export class AotPlugin implements Tapable {
     });
   }
 
+  diagnose(fileName: string) {
+    if (this._diagnoseFiles[fileName]) {
+      return;
+    }
+    this._diagnoseFiles[fileName] = true;
+
+    const sourceFile = this._program.getSourceFile(fileName);
+      if (!sourceFile) {
+        return;
+      }
+
+    const diagnostics: ts.Diagnostic[] = []
+      .concat(
+        this._program.getCompilerOptions().declaration
+          ? this._program.getDeclarationDiagnostics(sourceFile) : [],
+        this._program.getSyntacticDiagnostics(sourceFile),
+        this._program.getSemanticDiagnostics(sourceFile)
+      );
+
+    if (diagnostics.length > 0) {
+      const message = diagnostics
+        .map(diagnostic => {
+          const {line, character} = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
+          const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n');
+          return `${diagnostic.file.fileName} (${line + 1},${character + 1}): ${message})`;
+        })
+        .join('\n');
+      this._compilation.errors.push(message);
+    }
+  }
+
   private _make(compilation: any, cb: (err?: any, request?: any) => void) {
     this._compilation = compilation;
     if (this._compilation._ngToolsWebpackPluginInstance) {
@@ -382,8 +418,6 @@ export class AotPlugin implements Tapable {
       .then(() => {
         this._compilerHost.resetChangedFileTracker();
 
-        // Only turn this off for the first successful run.
-        this._firstRun = false;
         cb();
       }, (err: any) => {
         compilation.errors.push(err);

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

@@ -23,3 +23,29 @@ export interface ResolverPlugin extends Tapable {
   join(relativePath: string, innerRequest: Request): Request;
 }
 
+export interface LoaderCallback {
+  (err: Error | null, source?: string, sourceMap?: string): void;
+}
+
+export interface ModuleReason {
+  dependency: any;
+  module: NormalModule;
+}
+
+export interface NormalModule {
+  buildTimestamp: number;
+  built: boolean;
+  reasons: ModuleReason[];
+  resource: string;
+}
+
+export interface LoaderContext {
+  _module: NormalModule;
+
+  async(): LoaderCallback;
+  cacheable(): void;
+
+  readonly resourcePath: string;
+  readonly query: any;
+}
+

tests/e2e/tests/build/rebuild-deps-type-check.ts

@@ -0,0 +1,63 @@
+import {
+  killAllProcesses,
+  waitForAnyProcessOutputToMatch,
+  silentExecAndWaitForOutputToMatch,
+} from '../../utils/process';
+import {writeFile, prependToFile, appendToFile} from '../../utils/fs';
+import {wait} from '../../utils/utils';
+
+
+export default function() {
+  if (process.platform.startsWith('win')) {
+    return Promise.resolve();
+  }
+
+  return silentExecAndWaitForOutputToMatch('ng', ['serve'], /webpack: bundle is now VALID/)
+    // Create and import files.
+    .then(() => writeFile('src/funky2.ts', `
+      export function funky2(value: string): string {
+        return value + 'hello';
+      }
+    `))
+    .then(() => writeFile('src/funky.ts', `
+      export * from './funky2';
+    `))
+    .then(() => prependToFile('src/main.ts', `
+      import { funky } from './funky';
+    `))
+    .then(() => appendToFile('src/main.ts', `
+      console.log(funky('town'));
+    `))
+    // Should trigger a rebuild, no error expected.
+    .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
+    // Create and import files.
+    .then(() => wait(2000))
+    .then(() => writeFile('src/funky2.ts', `
+      export function funky(value: number): number {
+        return value + 1;
+      }
+    `))
+    // Should trigger a rebuild, this time an error is expected.
+    .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
+    .then(({ stdout }) => {
+      if (!/ERROR in .*\/src\/main\.ts \(/.test(stdout)) {
+        throw new Error('Expected an error but none happened.');
+      }
+    })
+    // Fix the error!
+    .then(() => writeFile('src/funky2.ts', `
+      export function funky(value: string): string {
+        return value + 'hello';
+      }
+    `))
+    .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
+    .then(({ stdout }) => {
+      if (/ERROR in .*\/src\/main\.ts \(/.test(stdout)) {
+        throw new Error('Expected no error but an error was shown.');
+      }
+    })
+    .then(() => killAllProcesses(), (err: any) => {
+      killAllProcesses();
+      throw err;
+    });
+}

tests/e2e/tests/build/rebuild.ts

@@ -3,7 +3,7 @@ import {
   exec,
   waitForAnyProcessOutputToMatch,
   silentExecAndWaitForOutputToMatch,
-  ng, execAndWaitForOutputToMatch,
+  ng,
 } from '../../utils/process';
 import {writeFile} from '../../utils/fs';
 import {wait} from '../../utils/utils';
@@ -17,18 +17,18 @@ export default function() {
   let oldNumberOfChunks = 0;
   const chunkRegExp = /chunk\s+\{/g;
 
-  return execAndWaitForOutputToMatch('ng', ['serve'], /webpack: bundle is now VALID/)
+  return silentExecAndWaitForOutputToMatch('ng', ['serve'], /webpack: bundle is now VALID/)
     // Should trigger a rebuild.
     .then(() => exec('touch', 'src/main.ts'))
     .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000))
     .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
     // Count the bundles.
-    .then((stdout: string) => {
+    .then(({ stdout }) => {
       oldNumberOfChunks = stdout.split(chunkRegExp).length;
     })
     // Add a lazy module.
     .then(() => ng('generate', 'module', 'lazy', '--routing'))
-    // Just wait for the rebuild, otherwise we might be validating this build.
+    // Just wait for the rebuild, otherwise we might be validating the last build.
     .then(() => wait(1000))
     .then(() => writeFile('src/app/app.module.ts', `
       import { BrowserModule } from '@angular/platform-browser';
@@ -60,7 +60,7 @@ export default function() {
     .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now INVALID/, 1000))
     .then(() => waitForAnyProcessOutputToMatch(/webpack: bundle is now VALID/, 5000))
     // Count the bundles.
-    .then((stdout: string) => {
+    .then(({ stdout }) => {
       let newNumberOfChunks = stdout.split(chunkRegExp).length;
       if (oldNumberOfChunks >= newNumberOfChunks) {
         throw new Error('Expected webpack to create a new chunk, but did not.');

tests/e2e/utils/fs.ts

@@ -103,6 +103,12 @@ export function appendToFile(filePath: string, text: string) {
 }
 
 
+export function prependToFile(filePath: string, text: string) {
+  return readFile(filePath)
+    .then((content: string) => writeFile(filePath, text.concat(content)));
+}
+
+
 export function expectFileToExist(fileName: string) {
   return new Promise((resolve, reject) => {
     fs.exists(fileName, (exist) => {

tests/e2e/utils/process.ts

@@ -11,7 +11,13 @@ interface ExecOptions {
 
 let _processes: child_process.ChildProcess[] = [];
 
-function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<string> {
+type ProcessOutput = {
+  stdout: string;
+  stdout: string;
+};
+
+
+function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<ProcessOutput> {
   let stdout = '';
   let stderr = '';
   const cwd = process.cwd();
@@ -51,6 +57,9 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<strin
   });
   childProcess.stderr.on('data', (data: Buffer) => {
     stderr += data.toString('utf-8');
+    if (options.silent) {
+      return;
+    }
     data.toString('utf-8')
       .split(/[\n\r]+/)
       .filter(line => line !== '')
@@ -76,23 +85,28 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<strin
     if (options.waitForMatch) {
       childProcess.stdout.on('data', (data: Buffer) => {
         if (data.toString().match(options.waitForMatch)) {
-          resolve(stdout);
+          resolve({ stdout, stderr });
         }
       });
     }
   });
 }
 
-export function waitForAnyProcessOutputToMatch(match: RegExp, timeout = 30000): Promise<string> {
+export function waitForAnyProcessOutputToMatch(match: RegExp,
+                                               timeout = 30000): Promise<ProcessOutput> {
   // Race between _all_ processes, and the timeout. First one to resolve/reject wins.
   return Promise.race(_processes.map(childProcess => new Promise(resolve => {
     let stdout = '';
+    let stderr = '';
     childProcess.stdout.on('data', (data: Buffer) => {
       stdout += data.toString();
       if (data.toString().match(match)) {
-        resolve(stdout);
+        resolve({ stdout, stderr });
       }
     });
+    childProcess.stderr.on('data', (data: Buffer) => {
+      stderr += data.toString();
+    });
   })).concat([
     new Promise((resolve, reject) => {
       // Wait for 30 seconds and timeout.