diff --git a/package.json b/package.json index 5923c23943ce..4b49093867a8 100644 --- a/package.json +++ b/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", diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts index 7c69ce292342..1fc297b6de23 100644 --- a/packages/@ngtools/webpack/src/loader.ts +++ b/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(() => { diff --git a/packages/@ngtools/webpack/src/plugin.ts b/packages/@ngtools/webpack/src/plugin.ts index b310845f92d9..acf7e2b2f629 100644 --- a/packages/@ngtools/webpack/src/plugin.ts +++ b/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); diff --git a/packages/@ngtools/webpack/src/webpack.ts b/packages/@ngtools/webpack/src/webpack.ts index 0074bed42e12..66ee79f7428f 100644 --- a/packages/@ngtools/webpack/src/webpack.ts +++ b/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; +} + diff --git a/tests/e2e/tests/build/rebuild-deps-type-check.ts b/tests/e2e/tests/build/rebuild-deps-type-check.ts new file mode 100644 index 000000000000..95884090295b --- /dev/null +++ b/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; + }); +} diff --git a/tests/e2e/tests/build/rebuild.ts b/tests/e2e/tests/build/rebuild.ts index de914664edb4..8806534f1706 100644 --- a/tests/e2e/tests/build/rebuild.ts +++ b/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.'); diff --git a/tests/e2e/utils/fs.ts b/tests/e2e/utils/fs.ts index 9e0f05b02e87..b1f8606c0184 100644 --- a/tests/e2e/utils/fs.ts +++ b/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) => { diff --git a/tests/e2e/utils/process.ts b/tests/e2e/utils/process.ts index 02b79addfd42..f2110856419a 100644 --- a/tests/e2e/utils/process.ts +++ b/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 { +type ProcessOutput = { + stdout: string; + stdout: string; +}; + + +function _exec(options: ExecOptions, cmd: string, args: string[]): Promise { let stdout = ''; let stderr = ''; const cwd = process.cwd(); @@ -51,6 +57,9 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise { 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 { if (data.toString().match(options.waitForMatch)) { - resolve(stdout); + resolve({ stdout, stderr }); } }); } }); } -export function waitForAnyProcessOutputToMatch(match: RegExp, timeout = 30000): Promise { +export function waitForAnyProcessOutputToMatch(match: RegExp, + timeout = 30000): Promise { // 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.