Skip to content

Commit

Permalink
fix(@ngtools/webpack): recover from component stylesheet errors
Browse files Browse the repository at this point in the history
Webpack doesn't handle well expections and promise rejections. With this change we use the compilation errors.

Closes #19892
  • Loading branch information
alan-agius4 committed Feb 2, 2021
1 parent 2cf374a commit e1efc35
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 5 deletions.
@@ -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<logging.LogEntry>({
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<logging.LogEntry>({
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);
});
});
});
24 changes: 19 additions & 5 deletions packages/ngtools/webpack/src/resource_loader.ts
Expand Up @@ -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);
}
});
});

Expand Down Expand Up @@ -153,10 +161,16 @@ export class WebpackResourceLoader {
return { outputName: filePath, source: finalOutput ?? '', success: !errors?.length };
}

private async _evaluate(filename: string, source: string): Promise<string> {
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;
Expand Down

0 comments on commit e1efc35

Please sign in to comment.