From 16287036a0ae8459ff9d7d6d9dfc7166f45028ae Mon Sep 17 00:00:00 2001 From: Filipe Silva Date: Thu, 26 Oct 2017 19:10:07 +0100 Subject: [PATCH] fix(@ngtools/webpack): fix and improve error reporting Fix #7925 --- .../webpack/src/angular_compiler_plugin.ts | 359 ++++++++++-------- .../webpack/src/gather_diagnostics.ts | 6 - packages/@ngtools/webpack/src/loader.ts | 12 +- packages/@ngtools/webpack/src/type_checker.ts | 55 +-- tests/e2e/tests/build/build-errors.ts | 76 ++++ tests/e2e/tests/build/rebuild-error.ts | 81 +++- tests/e2e/utils/utils.ts | 4 +- 7 files changed, 392 insertions(+), 201 deletions(-) create mode 100644 tests/e2e/tests/build/build-errors.ts diff --git a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts index 6700bafc6bfa..460c46145c3e 100644 --- a/packages/@ngtools/webpack/src/angular_compiler_plugin.ts +++ b/packages/@ngtools/webpack/src/angular_compiler_plugin.ts @@ -102,6 +102,7 @@ export class AngularCompilerPlugin implements Tapable { private _transformMap: Map = new Map(); private _platform: PLATFORM; private _JitMode = false; + private _emitSkipped = true; // Webpack plugin. private _firstRun = true; @@ -245,6 +246,10 @@ export class AngularCompilerPlugin implements Tapable { this._compilerOptions.sourceRoot = undefined; } + // We want to allow emitting with errors so that imports can be added + // to the webpack dependency tree and rebuilds triggered by file edits. + this._compilerOptions.noEmitOnError = false; + // Compose Angular Compiler Options. this._angularCompilerOptions = Object.assign( this._compilerOptions, @@ -329,56 +334,76 @@ export class AngularCompilerPlugin implements Tapable { .filter(k => this._compilerHost.fileExists(k)); } + private _getChangedCompilationFiles() { + return this._compilerHost.getChangedFilePaths() + .filter(k => /\.(?:ts|html|css|scss|sass|less|styl)$/.test(k)); + } + private _createOrUpdateProgram() { - const changedTsFiles = this._getChangedTsFiles(); - changedTsFiles.forEach((file) => { - if (!this._tsFilenames.includes(file)) { - // TODO: figure out if action is needed for files that were removed from the compilation. - this._tsFilenames.push(file); - } - }); + return Promise.resolve() + .then(() => { + const changedTsFiles = this._getChangedTsFiles(); - // Update the forked type checker. - if (this._forkTypeChecker && !this._firstRun) { - this._updateForkedTypeChecker(changedTsFiles); - } + changedTsFiles.forEach((file) => { + if (!this._tsFilenames.includes(file)) { + // TODO: figure out if action is needed for files that were removed from the + // compilation. + this._tsFilenames.push(file); + } + }); - // 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 - }; + // Update the forked type checker. + if (this._forkTypeChecker && !this._firstRun) { + this._updateForkedTypeChecker(changedTsFiles); + } - if (this._JitMode) { - // Create the TypeScript program. - time('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); - this._program = ts.createProgram( - this._tsFilenames, - compilerOptions, - this._angularCompilerHost, - this._program as ts.Program - ); - timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); + if (this._JitMode) { + // Create the TypeScript program. + time('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); + this._program = ts.createProgram( + this._tsFilenames, + this._angularCompilerOptions, + this._angularCompilerHost, + this._program as ts.Program + ); + timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ts.createProgram'); - return Promise.resolve(); - } else { - time('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); - // Create the Angular program. - this._program = createProgram({ - rootNames: this._tsFilenames, - options: compilerOptions, - host: this._angularCompilerHost, - oldProgram: this._program as Program + return Promise.resolve(); + } else { + time('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); + // Create the Angular program. + try { + this._program = createProgram({ + rootNames: this._tsFilenames, + options: this._angularCompilerOptions, + host: this._angularCompilerHost, + oldProgram: this._program as Program + }); + timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); + + time('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); + return this._program.loadNgStructureAsync() + .then(() => { + timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); + }); + } catch (e) { + // TODO: remove this when the issue is addressed. + // Temporary workaround for https://github.com/angular/angular/issues/19951 + this._program = undefined; + throw e; + } + } + }) + .then(() => { + // If there's still no entryModule try to resolve from mainPath. + if (!this._entryModule && this._options.mainPath) { + time('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); + const mainPath = path.resolve(this._basePath, this._options.mainPath); + this._entryModule = resolveEntryModuleFromMain( + mainPath, this._compilerHost, this._getTsProgram()); + timeEnd('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); + } }); - timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.createProgram'); - - time('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); - return this._program.loadNgStructureAsync() - .then(() => { - timeEnd('AngularCompilerPlugin._createOrUpdateProgram.ng.loadNgStructureAsync'); - }); - } } private _getLazyRoutesFromNgtools() { @@ -646,6 +671,7 @@ export class AngularCompilerPlugin implements Tapable { private _make(compilation: any, cb: (err?: any, request?: any) => void) { time('AngularCompilerPlugin._make'); this._compilation = compilation; + this._emitSkipped = true; if (this._compilation._ngToolsWebpackPluginInstance) { return cb(new Error('An @ngtools/webpack plugin already exist for this compilation.')); } @@ -670,18 +696,6 @@ export class AngularCompilerPlugin implements Tapable { options: this._angularCompilerOptions, tsHost: this._compilerHost }) as CompilerHost & WebpackCompilerHost; - - return this._createOrUpdateProgram() - .then(() => { - // If there's still no entryModule try to resolve from mainPath. - if (!this._entryModule && this._options.mainPath) { - time('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); - const mainPath = path.resolve(this._basePath, this._options.mainPath); - this._entryModule = resolveEntryModuleFromMain( - mainPath, this._compilerHost, this._getTsProgram()); - timeEnd('AngularCompilerPlugin._make.resolveEntryModuleFromMain'); - } - }); } }) .then(() => this._update()) @@ -696,28 +710,30 @@ export class AngularCompilerPlugin implements Tapable { } private _update() { + time('AngularCompilerPlugin._update'); // We only want to update on TS and template changes, but all kinds of files are on this // list, like package.json and .ngsummary.json files. - time('AngularCompilerPlugin._update'); - let changedFiles = this._compilerHost.getChangedFilePaths() - .filter(k => /(ts|html|css|scss|sass|less|styl)/.test(k)); + let changedFiles = this._getChangedCompilationFiles(); + + // If nothing we care about changed and it isn't the first run, don't do anything. + if (changedFiles.length === 0 && !this._firstRun) { + return Promise.resolve(); + } return Promise.resolve() - .then(() => { - // Make a new program and load the Angular structure if there are changes. - if (changedFiles.length > 0) { - return this._createOrUpdateProgram(); - } - }) + // Make a new program and load the Angular structure. + .then(() => this._createOrUpdateProgram()) .then(() => { // Try to find lazy routes. // We need to run the `listLazyRoutes` the first time because it also navigates libraries // and other things that we might miss using the (faster) findLazyRoutesInAst. // Lazy routes modules will be read with compilerHost and added to the changed files. - const changedTsFiles = this._compilerHost.getChangedFilePaths() - .filter(k => k.endsWith('.ts')); + const changedTsFiles = this._getChangedTsFiles(); if (this._ngCompilerSupportsNewApi) { this._processLazyRoutes(this._listLazyRoutesFromProgram()); + // TODO: remove this when the issue is addressed. + // Fix for a bug in compiler where the program needs to be updated after + // _listLazyRoutesFromProgram is called. return this._createOrUpdateProgram(); } else if (this._firstRun) { this._processLazyRoutes(this._getLazyRoutesFromNgtools()); @@ -726,94 +742,94 @@ export class AngularCompilerPlugin implements Tapable { } }) .then(() => { - // Build transforms, emit and report errors if there are changes or it's the first run. - if (changedFiles.length > 0 || this._firstRun) { - - // We now have the final list of changed TS files. - // Go through each changed file and add transforms as needed. - const sourceFiles = this._getChangedTsFiles().map((fileName) => { - time('AngularCompilerPlugin._update.getSourceFile'); - const sourceFile = this._getTsProgram().getSourceFile(fileName); - if (!sourceFile) { - throw new Error(`${fileName} is not part of the TypeScript compilation. ` - + `Please include it in your tsconfig via the 'files' or 'include' property.`); - } - timeEnd('AngularCompilerPlugin._update.getSourceFile'); - return sourceFile; - }); + // Build transforms, emit and report errorsn. + + // We now have the final list of changed TS files. + // Go through each changed file and add transforms as needed. + const sourceFiles = this._getChangedTsFiles().map((fileName) => { + time('AngularCompilerPlugin._update.getSourceFile'); + const sourceFile = this._getTsProgram().getSourceFile(fileName); + if (!sourceFile) { + throw new Error(`${fileName} is not part of the TypeScript compilation. ` + + `Please include it in your tsconfig via the 'files' or 'include' property.`); + } + timeEnd('AngularCompilerPlugin._update.getSourceFile'); + return sourceFile; + }); - time('AngularCompilerPlugin._update.transformOps'); - sourceFiles.forEach((sf) => { - const fileName = this._compilerHost.resolve(sf.fileName); - let transformOps = []; + time('AngularCompilerPlugin._update.transformOps'); + sourceFiles.forEach((sf) => { + const fileName = this._compilerHost.resolve(sf.fileName); + let transformOps = []; - if (this._JitMode) { - transformOps.push(...replaceResources(sf)); + if (this._JitMode) { + transformOps.push(...replaceResources(sf)); + } + + if (this._platform === PLATFORM.Browser) { + if (!this._JitMode) { + transformOps.push(...replaceBootstrap(sf, this.entryModule)); } - if (this._platform === PLATFORM.Browser) { + // If we have a locale, auto import the locale data file. + if (this._angularCompilerOptions.i18nInLocale) { + transformOps.push(...registerLocaleData( + sf, + this.entryModule, + this._angularCompilerOptions.i18nInLocale + )); + } + } else if (this._platform === PLATFORM.Server) { + if (fileName === this._compilerHost.resolve(this._options.mainPath)) { + transformOps.push(...exportLazyModuleMap(sf, this._lazyRoutes)); if (!this._JitMode) { - transformOps.push(...replaceBootstrap(sf, this.entryModule)); - } - - // If we have a locale, auto import the locale data file. - if (this._angularCompilerOptions.i18nInLocale) { - transformOps.push(...registerLocaleData( - sf, - this.entryModule, - this._angularCompilerOptions.i18nInLocale - )); - } - } else if (this._platform === PLATFORM.Server) { - if (fileName === this._compilerHost.resolve(this._options.mainPath)) { - transformOps.push(...exportLazyModuleMap(sf, this._lazyRoutes)); - if (!this._JitMode) { - transformOps.push(...exportNgFactory(sf, this.entryModule)); - } + transformOps.push(...exportNgFactory(sf, this.entryModule)); } } + } - // We need to keep a map of transforms for each file, to reapply on each update. - this._transformMap.set(fileName, transformOps); - }); + // We need to keep a map of transforms for each file, to reapply on each update. + this._transformMap.set(fileName, transformOps); + }); - const transformOps: TransformOperation[] = []; - for (let fileTransformOps of this._transformMap.values()) { - transformOps.push(...fileTransformOps); - } - timeEnd('AngularCompilerPlugin._update.transformOps'); + const transformOps: TransformOperation[] = []; + for (let fileTransformOps of this._transformMap.values()) { + transformOps.push(...fileTransformOps); + } + timeEnd('AngularCompilerPlugin._update.transformOps'); + + time('AngularCompilerPlugin._update.makeTransform'); + const transformers: CustomTransformers = { + beforeTs: transformOps.length > 0 ? [makeTransform(transformOps)] : [] + }; + timeEnd('AngularCompilerPlugin._update.makeTransform'); + + // Emit files. + time('AngularCompilerPlugin._update._emit'); + const { emitResult, diagnostics } = this._emit(sourceFiles, transformers); + timeEnd('AngularCompilerPlugin._update._emit'); + + // Report diagnostics. + const errors = diagnostics + .filter((diag) => diag.category === ts.DiagnosticCategory.Error); + const warnings = diagnostics + .filter((diag) => diag.category === ts.DiagnosticCategory.Warning); + + if (errors.length > 0) { + const message = formatDiagnostics(errors); + this._compilation.errors.push(message); + } - time('AngularCompilerPlugin._update.makeTransform'); - const transformers: CustomTransformers = { - beforeTs: transformOps.length > 0 ? [makeTransform(transformOps)] : [] - }; - timeEnd('AngularCompilerPlugin._update.makeTransform'); - - // Emit files. - time('AngularCompilerPlugin._update._emit'); - const { emitResult, diagnostics } = this._emit(sourceFiles, transformers); - timeEnd('AngularCompilerPlugin._update._emit'); - - // Report diagnostics. - const errors = diagnostics - .filter((diag) => diag.category === ts.DiagnosticCategory.Error); - const warnings = diagnostics - .filter((diag) => diag.category === ts.DiagnosticCategory.Warning); - - if (errors.length > 0) { - const message = formatDiagnostics(errors); - this._compilation.errors.push(message); - } + if (warnings.length > 0) { + const message = formatDiagnostics(warnings); + this._compilation.warnings.push(message); + } - if (warnings.length > 0) { - const message = formatDiagnostics(warnings); - this._compilation.warnings.push(message); - } + this._emitSkipped = !emitResult || emitResult.emitSkipped; - // Reset changed files on successful compilation. - if (emitResult && !emitResult.emitSkipped && this._compilation.errors.length === 0) { - this._compilerHost.resetChangedFileTracker(); - } + // Reset changed files on successful compilation. + if (this._emitSkipped && this._compilation.errors.length === 0) { + this._compilerHost.resetChangedFileTracker(); } timeEnd('AngularCompilerPlugin._update'); }); @@ -838,12 +854,43 @@ export class AngularCompilerPlugin implements Tapable { } } - getFile(fileName: string) { + getCompiledFile(fileName: string) { const outputFile = fileName.replace(/.ts$/, '.js'); - return { - outputText: this._compilerHost.readFile(outputFile), - sourceMap: this._compilerHost.readFile(outputFile + '.map') - }; + let outputText: string; + let sourceMap: string; + let errorDependencies: string[] = []; + + if (this._emitSkipped) { + if (this._compilerHost.fileExists(outputFile, false)) { + // If the compilation didn't emit files this time, try to return the cached files from the + // last compilation and let the compilation errors show what's wrong. + outputText = this._compilerHost.readFile(outputFile); + sourceMap = this._compilerHost.readFile(outputFile + '.map'); + } else { + // There's nothing we can serve. Return an empty string to prevent lenghty webpack errors, + // add the rebuild warning if it's not there yet. + // We also need to all changed files as dependencies of this file, so that all of them + // will be watched and trigger a rebuild next time. + outputText = ''; + errorDependencies = this._getChangedCompilationFiles(); + } + } else { + // Check if the TS file exists. + if (fileName.endsWith('.ts') && !this._compilerHost.fileExists(fileName, false)) { + throw new Error(`${fileName} is not part of the compilation. ` + + `Please make sure it is in your tsconfig via the 'files' or 'include' property.`); + } + + // Check if the output file exists. + if (!this._compilerHost.fileExists(outputFile, false)) { + throw new Error(`${fileName} is not part of the compilation output. ` + + `Please check the other error messages for details.`); + } + + outputText = this._compilerHost.readFile(outputFile); + sourceMap = this._compilerHost.readFile(outputFile + '.map'); + } + return { outputText, sourceMap, errorDependencies }; } getDependencies(fileName: string): string[] { @@ -909,9 +956,7 @@ export class AngularCompilerPlugin implements Tapable { 'AngularCompilerPlugin._emit.ts')); } - // 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) { + if (!hasErrors(allDiagnostics)) { sourceFiles.forEach((sf) => { const timeLabel = `AngularCompilerPlugin._emit.ts+${sf.fileName}+.emit`; time(timeLabel); @@ -925,6 +970,11 @@ export class AngularCompilerPlugin implements Tapable { } else { const angularProgram = program as Program; + // Check Angular structural diagnostics. + time('AngularCompilerPlugin._emit.ng.getNgStructuralDiagnostics'); + allDiagnostics.push(...angularProgram.getNgStructuralDiagnostics()); + timeEnd('AngularCompilerPlugin._emit.ng.getNgStructuralDiagnostics'); + if (this._firstRun) { // Check TypeScript parameter diagnostics. time('AngularCompilerPlugin._emit.ng.getTsOptionDiagnostics'); @@ -942,9 +992,7 @@ export class AngularCompilerPlugin implements Tapable { 'AngularCompilerPlugin._emit.ng')); } - // 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) { + if (!hasErrors(allDiagnostics)) { time('AngularCompilerPlugin._emit.ng.emit'); const extractI18n = !!this._angularCompilerOptions.i18nOutFile; const emitFlags = extractI18n ? EmitFlags.I18nBundle : EmitFlags.Default; @@ -954,6 +1002,9 @@ export class AngularCompilerPlugin implements Tapable { this.writeI18nOutFile(); } timeEnd('AngularCompilerPlugin._emit.ng.emit'); + } else { + // Throw away the old program if there was an error. + this._program = undefined; } } } catch (e) { diff --git a/packages/@ngtools/webpack/src/gather_diagnostics.ts b/packages/@ngtools/webpack/src/gather_diagnostics.ts index c321c3a45acc..a5dfa131064f 100644 --- a/packages/@ngtools/webpack/src/gather_diagnostics.ts +++ b/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 && diff --git a/packages/@ngtools/webpack/src/loader.ts b/packages/@ngtools/webpack/src/loader.ts index 0eef2018d0fc..e75236ee77b1 100644 --- a/packages/@ngtools/webpack/src/loader.ts +++ b/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) { diff --git a/packages/@ngtools/webpack/src/type_checker.ts b/packages/@ngtools/webpack/src/type_checker.ts index 18c1629a3ee0..f761fb447574 100644 --- a/packages/@ngtools/webpack/src/type_checker.ts +++ b/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'); }); diff --git a/tests/e2e/tests/build/build-errors.ts b/tests/e2e/tests/build/build-errors.ts new file mode 100644 index 000000000000..341debe750b0 --- /dev/null +++ b/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)); +} diff --git a/tests/e2e/tests/build/rebuild-error.ts b/tests/e2e/tests/build/rebuild-error.ts index 3324a8fb5e1a..048a0117b355 100644 --- a/tests/e2e/tests/build/rebuild-error.ts +++ b/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; diff --git a/tests/e2e/utils/utils.ts b/tests/e2e/utils/utils.ts index 8021dba99b52..3f73ec59ba94 100644 --- a/tests/e2e/utils/utils.ts +++ b/tests/e2e/utils/utils.ts @@ -1,12 +1,12 @@ -export function expectToFail(fn: () => Promise, errorMessage?: string): Promise { +export function expectToFail(fn: () => Promise, errorMessage?: string): Promise { return fn() .then(() => { const functionSource = fn.name || (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 {