From d99655c1eacf051e3b828626d84df803bca247ab Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Tue, 2 Feb 2021 09:10:27 +0100 Subject: [PATCH] fix(@ngtools/webpack): recover from component stylesheet errors Webpack doesn't handle well expections and promise rejections. With this change we use the compilation errors. Closes #19892 (cherry picked from commit e1efc35e411a921baf80e064a0a62e2c37797742) --- .../tests/behavior/rebuild-errors_spec.ts | 64 +++++++++++++++++++ .../ngtools/webpack/src/resource_loader.ts | 24 +++++-- 2 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 packages/angular_devkit/build_angular/src/browser/tests/behavior/rebuild-errors_spec.ts diff --git a/packages/angular_devkit/build_angular/src/browser/tests/behavior/rebuild-errors_spec.ts b/packages/angular_devkit/build_angular/src/browser/tests/behavior/rebuild-errors_spec.ts new file mode 100644 index 000000000000..9e1311de2028 --- /dev/null +++ b/packages/angular_devkit/build_angular/src/browser/tests/behavior/rebuild-errors_spec.ts @@ -0,0 +1,64 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import { logging } from '@angular-devkit/core'; +import { concatMap, count, take, timeout } from 'rxjs/operators'; +import { buildWebpackBrowser } from '../../index'; +import { BASE_OPTIONS, BROWSER_BUILDER_INFO, describeBuilder } from '../setup'; + +describeBuilder(buildWebpackBrowser, BROWSER_BUILDER_INFO, (harness) => { + describe('Behavior: "Rebuild Error"', () => { + it('recovers from component stylesheet error', async () => { + harness.useTarget('build', { + ...BASE_OPTIONS, + watch: true, + }); + + const buildCount = await harness + .execute({ outputLogsOnFailure: false }) + .pipe( + timeout(30000), + concatMap(async ({ result, logs }, index) => { + switch (index) { + case 0: + expect(result?.success).toBeTrue(); + await harness.writeFile('src/app/app.component.css', 'invalid-css-content'); + + break; + case 1: + expect(result?.success).toBeFalse(); + expect(logs).toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching('invalid-css-content'), + }), + ); + + await harness.writeFile('src/app/app.component.css', 'p { color: green }'); + + break; + case 2: + expect(result?.success).toBeTrue(); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching('invalid-css-content'), + }), + ); + + harness.expectFile('dist/main.js').content.toContain('p { color: green }'); + + break; + } + }), + take(3), + count(), + ) + .toPromise(); + + expect(buildCount).toBe(3); + }); + }); +}); diff --git a/packages/ngtools/webpack/src/resource_loader.ts b/packages/ngtools/webpack/src/resource_loader.ts index 148a9064c7c2..6bb2a2374b1e 100644 --- a/packages/ngtools/webpack/src/resource_loader.ts +++ b/packages/ngtools/webpack/src/resource_loader.ts @@ -93,14 +93,22 @@ export class WebpackResourceLoader { new LibraryTemplatePlugin('resource', 'var').apply(childCompiler); childCompiler.hooks.thisCompilation.tap('ngtools-webpack', (compilation: any) => { - compilation.hooks.additionalAssets.tapPromise('ngtools-webpack', async () => { + compilation.hooks.additionalAssets.tap('ngtools-webpack', () => { const asset = compilation.assets[filePath]; if (!asset) { return; } - const output = await this._evaluate(filePath, asset.source()); - compilation.assets[filePath] = new RawSource(output); + try { + const output = this._evaluate(filePath, asset.source()); + + if (typeof output === 'string') { + compilation.assets[filePath] = new RawSource(output); + } + } catch (error) { + // Use compilation errors, as otherwise webpack will choke + compilation.errors.push(error); + } }); }); @@ -153,10 +161,16 @@ export class WebpackResourceLoader { return { outputName: filePath, source: finalOutput ?? '', success: !errors?.length }; } - private async _evaluate(filename: string, source: string): Promise { + private _evaluate(filename: string, source: string): string | null { // Evaluate code const context: { resource?: string | { default?: string } } = {}; - vm.runInNewContext(source, context, { filename }); + + try { + vm.runInNewContext(source, context, { filename }); + } catch { + // Error are propagated through the child compilation. + return null; + } if (typeof context.resource === 'string') { return context.resource;