Skip to content

Commit 447761e

Browse files
clydinclydin
committed
fix(@angular-devkit/build-angular): cache stylesheet load errors with application builder
When using the `application` and/or `browser-esbuild` builders, stylesheets that generate errors will now have the errors cached between rebuilds during watch mode (include `ng serve`). This not only avoids reprocessing files that contain errors and that have not changed but also provides file watching information from the cache to ensure the error-causing files are properly invalidated.
1 parent fd2d72a commit 447761e

File tree

5 files changed

+147
-16
lines changed

5 files changed

+147
-16
lines changed

‎packages/angular_devkit/build_angular/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import { concatMap, count, timeout } from 'rxjs';
10+
import { buildApplication } from '../../index';
11+
import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup';
12+
13+
/**
14+
* Maximum time in milliseconds for single build/rebuild
15+
* This accounts for CI variability.
16+
*/
17+
export const BUILD_TIMEOUT = 30_000;
18+
19+
describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
20+
describe('Behavior: "Rebuilds when global stylesheets change"', () => {
21+
beforeEach(async () => {
22+
// Application code is not needed for styles tests
23+
await harness.writeFile('src/main.ts', 'console.log("TEST");');
24+
});
25+
26+
it('rebuilds Sass stylesheet after error on rebuild from import', async () => {
27+
harness.useTarget('build', {
28+
...BASE_OPTIONS,
29+
watch: true,
30+
styles: ['src/styles.scss'],
31+
});
32+
33+
await harness.writeFile('src/styles.scss', "@import './a';");
34+
await harness.writeFile('src/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
35+
36+
const builderAbort = new AbortController();
37+
const buildCount = await harness
38+
.execute({ signal: builderAbort.signal, outputLogsOnFailure: false })
39+
.pipe(
40+
timeout(30000),
41+
concatMap(async ({ result }, index) => {
42+
switch (index) {
43+
case 0:
44+
expect(result?.success).toBe(true);
45+
harness.expectFile('dist/browser/styles.css').content.toContain('color: aqua');
46+
harness.expectFile('dist/browser/styles.css').content.not.toContain('color: blue');
47+
48+
await harness.writeFile(
49+
'src/a.scss',
50+
'invalid-invalid-invalid\\nh1 { color: $primary; }',
51+
);
52+
break;
53+
case 1:
54+
expect(result?.success).toBe(false);
55+
56+
await harness.writeFile('src/a.scss', '$primary: blue;\\nh1 { color: $primary; }');
57+
break;
58+
case 2:
59+
expect(result?.success).toBe(true);
60+
harness.expectFile('dist/browser/styles.css').content.not.toContain('color: aqua');
61+
harness.expectFile('dist/browser/styles.css').content.toContain('color: blue');
62+
63+
// Test complete - abort watch mode
64+
builderAbort.abort();
65+
break;
66+
}
67+
}),
68+
count(),
69+
)
70+
.toPromise();
71+
72+
expect(buildCount).toBe(3);
73+
});
74+
75+
it('rebuilds Sass stylesheet after error on initial build from import', async () => {
76+
harness.useTarget('build', {
77+
...BASE_OPTIONS,
78+
watch: true,
79+
styles: ['src/styles.scss'],
80+
});
81+
82+
await harness.writeFile('src/styles.scss', "@import './a';");
83+
await harness.writeFile('src/a.scss', 'invalid-invalid-invalid\\nh1 { color: $primary; }');
84+
85+
const builderAbort = new AbortController();
86+
const buildCount = await harness
87+
.execute({ signal: builderAbort.signal, outputLogsOnFailure: false })
88+
.pipe(
89+
timeout(30000),
90+
concatMap(async ({ result }, index) => {
91+
switch (index) {
92+
case 0:
93+
expect(result?.success).toBe(false);
94+
95+
await harness.writeFile('src/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
96+
break;
97+
case 1:
98+
expect(result?.success).toBe(true);
99+
harness.expectFile('dist/browser/styles.css').content.toContain('color: aqua');
100+
harness.expectFile('dist/browser/styles.css').content.not.toContain('color: blue');
101+
102+
await harness.writeFile('src/a.scss', '$primary: blue;\\nh1 { color: $primary; }');
103+
break;
104+
case 2:
105+
expect(result?.success).toBe(true);
106+
harness.expectFile('dist/browser/styles.css').content.not.toContain('color: aqua');
107+
harness.expectFile('dist/browser/styles.css').content.toContain('color: blue');
108+
109+
// Test complete - abort watch mode
110+
builderAbort.abort();
111+
break;
112+
}
113+
}),
114+
count(),
115+
)
116+
.toPromise();
117+
118+
expect(buildCount).toBe(3);
119+
});
120+
});
121+
});

‎packages/angular_devkit/build_angular/src/tools/esbuild/bundler-context.ts

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,16 @@ export class BundlerContext {
220220
} else {
221221
throw failure;
222222
}
223+
} finally {
224+
if (this.incremental) {
225+
// When incremental always add any files from the load result cache
226+
if (this.#loadCache) {
227+
this.#loadCache.watchFiles
228+
.filter((file) => !isInternalAngularFile(file))
229+
// watch files are fully resolved paths
230+
.forEach((file) => this.watchFiles.add(file));
231+
}
232+
}
223233
}
224234

225235
// Update files that should be watched.
@@ -228,16 +238,9 @@ export class BundlerContext {
228238
if (this.incremental) {
229239
// Add input files except virtual angular files which do not exist on disk
230240
Object.keys(result.metafile.inputs)
231-
.filter((input) => !input.startsWith('angular:'))
241+
.filter((input) => !isInternalAngularFile(input))
232242
// input file paths are always relative to the workspace root
233243
.forEach((input) => this.watchFiles.add(join(this.workspaceRoot, input)));
234-
// Also add any files from the load result cache
235-
if (this.#loadCache) {
236-
this.#loadCache.watchFiles
237-
.filter((file) => !file.startsWith('angular:'))
238-
// watch files are fully resolved paths
239-
.forEach((file) => this.watchFiles.add(file));
240-
}
241244
}
242245

243246
// Return if the build encountered any errors
@@ -349,12 +352,12 @@ export class BundlerContext {
349352
#addErrorsToWatch(result: BuildFailure | BuildResult): void {
350353
for (const error of result.errors) {
351354
let file = error.location?.file;
352-
if (file) {
355+
if (file && !isInternalAngularFile(file)) {
353356
this.watchFiles.add(join(this.workspaceRoot, file));
354357
}
355358
for (const note of error.notes) {
356359
file = note.location?.file;
357-
if (file) {
360+
if (file && !isInternalAngularFile(file)) {
358361
this.watchFiles.add(join(this.workspaceRoot, file));
359362
}
360363
}
@@ -406,3 +409,7 @@ export class BundlerContext {
406409
}
407410
}
408411
}
412+
413+
function isInternalAngularFile(file: string) {
414+
return file.startsWith('angular:');
415+
}

‎packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export function createCachedLoad(
3030
if (result === undefined) {
3131
result = await callback(args);
3232

33-
// Do not cache null or undefined or results with errors
34-
if (result && result.errors === undefined) {
33+
// Do not cache null or undefined
34+
if (result) {
3535
await cache.put(loadCacheKey, result);
3636
}
3737
}

‎packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/less-language.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ async function compileString(
115115
};
116116
} catch (error) {
117117
if (isLessException(error)) {
118+
const location = convertExceptionLocation(error);
119+
118120
// Retry with a warning for less files requiring the deprecated inline JavaScript option
119121
if (error.message.includes('Inline JavaScript is not enabled.')) {
120122
const withJsResult = await compileString(
@@ -127,7 +129,7 @@ async function compileString(
127129
withJsResult.warnings = [
128130
{
129131
text: 'Deprecated inline execution of JavaScript has been enabled ("javascriptEnabled")',
130-
location: convertExceptionLocation(error),
132+
location,
131133
notes: [
132134
{
133135
location: null,
@@ -148,10 +150,11 @@ async function compileString(
148150
errors: [
149151
{
150152
text: error.message,
151-
location: convertExceptionLocation(error),
153+
location,
152154
},
153155
],
154156
loader: 'css',
157+
watchFiles: location.file ? [filename, location.file] : [filename],
155158
};
156159
}
157160

‎packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ async function compileString(
176176
};
177177
} catch (error) {
178178
if (isSassException(error)) {
179-
const file = error.span.url ? fileURLToPath(error.span.url) : undefined;
179+
const fileWithError = error.span.url ? fileURLToPath(error.span.url) : undefined;
180180

181181
return {
182182
loader: 'css',
@@ -186,7 +186,7 @@ async function compileString(
186186
},
187187
],
188188
warnings,
189-
watchFiles: file ? [file] : undefined,
189+
watchFiles: fileWithError ? [filePath, fileWithError] : [filePath],
190190
};
191191
}
192192

0 commit comments

Comments
 (0)