diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index 5ca4aee79256..1e812dd5ec7c 100644 --- 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 | 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 index 1d5074e74c38..016ba0e0dc80 100644 --- 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 index e1dfaef48211..222abacef9a2 100644 --- 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 | 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 new file mode 100644 index 000000000000..c35d1ba5fdd3 --- /dev/null +++ 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; + }); +}