Skip to content

Commit 10e9f39

Browse files
committed
fix(@angular/build): scope BundlerContext invalidation and cache one-shot bundle results
Ensure `BundlerContext.invalidate()` only clears `#esbuildResult` when a changed file is in `this.watchFiles` while still invalidating `#loadCache`, preventing edits to one component stylesheet from invalidating all other components sharing the same `MemoryLoadResultCache`. Also scope `#addLoadCacheFilesToWatch()` to the load cache keys accessed by the current `BundlerContext`, retain `watchFiles` across failed rebuilds while tracking entry points and unresolved relative imports on build failures, and cache `#esbuildResult` in non-incremental builds so multiple components sharing the same `styleUrl` do not re-run esbuild.
1 parent a68d0df commit 10e9f39

2 files changed

Lines changed: 174 additions & 35 deletions

File tree

‎packages/angular/build/src/tools/esbuild/bundler-context.ts‎

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export class BundlerContext {
8181
#optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;
8282
#shouldCacheResult: boolean;
8383
#loadCache?: LoadResultCache;
84+
#loadCacheKeys?: Set<string>;
8485
#invalidationEpoch = 0;
8586
readonly watchFiles = new Set<string>();
8687

@@ -227,11 +228,27 @@ export class BundlerContext {
227228
if (this.incremental && !this.#loadCache) {
228229
this.#loadCache = new MemoryLoadResultCache();
229230
}
230-
this.#esbuildOptions = this.#optionsFactory(this.#loadCache);
231-
}
232-
233-
if (this.incremental) {
234-
this.watchFiles.clear();
231+
const loadCache = this.#loadCache;
232+
const scopedLoadCache: LoadResultCache | undefined =
233+
this.incremental && loadCache
234+
? {
235+
get: (path) => {
236+
(this.#loadCacheKeys ??= new Set()).add(path);
237+
238+
return loadCache.get(path);
239+
},
240+
put: (path, result) => {
241+
(this.#loadCacheKeys ??= new Set()).add(path);
242+
243+
return loadCache.put(path, result);
244+
},
245+
invalidate: (path) => loadCache.invalidate(path),
246+
get watchFiles() {
247+
return loadCache.watchFiles;
248+
},
249+
}
250+
: loadCache;
251+
this.#esbuildOptions = this.#optionsFactory(scopedLoadCache);
235252
}
236253

237254
let result: BuildResult<{ metafile: true; write: false }>;
@@ -263,7 +280,7 @@ export class BundlerContext {
263280
// Build failures will throw an exception which contains errors/warnings
264281
if (isEsBuildFailure(failure)) {
265282
this.#addErrorsToWatch(failure);
266-
this.#addLoadCacheFilesToWatch();
283+
await this.#addLoadCacheFilesToWatch();
267284

268285
return failure;
269286
} else {
@@ -275,6 +292,8 @@ export class BundlerContext {
275292
// While this should technically not be linked to incremental mode, incremental is only
276293
// currently enabled with watch mode where watch files are needed.
277294
if (this.incremental) {
295+
this.watchFiles.clear();
296+
278297
// Add input files except virtual angular files which do not exist on disk
279298
for (const input of Object.keys(result.metafile.inputs)) {
280299
const isInternal = isInternalAngularFile(input) || isInternalBundlerFile(input);
@@ -309,7 +328,7 @@ export class BundlerContext {
309328
// Return if the build encountered any errors
310329
if (result.errors.length) {
311330
this.#addErrorsToWatch(result);
312-
this.#addLoadCacheFilesToWatch();
331+
await this.#addLoadCacheFilesToWatch();
313332

314333
return {
315334
errors: result.errors,
@@ -425,12 +444,26 @@ export class BundlerContext {
425444
}
426445

427446
#addErrorsToWatch(result: BuildFailure | BuildResult): void {
447+
const entryPoints = this.#esbuildOptions?.entryPoints;
448+
if (entryPoints) {
449+
const entries = Array.isArray(entryPoints)
450+
? entryPoints.map((entry) => (typeof entry === 'string' ? entry : entry.in))
451+
: Object.values(entryPoints);
452+
for (const entry of entries) {
453+
if (isInternalAngularFile(entry) || isInternalBundlerFile(entry)) {
454+
continue;
455+
}
456+
457+
this.watchFiles.add(isAbsolute(entry) ? normalize(entry) : join(this.workspaceRoot, entry));
458+
}
459+
}
460+
428461
for (const error of result.errors) {
429462
let file = error.location?.file;
430463
if (file && !isInternalAngularFile(file)) {
431464
this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
432465
}
433-
for (const note of error.notes) {
466+
for (const note of error.notes ?? []) {
434467
file = note.location?.file;
435468
if (file && !isInternalAngularFile(file)) {
436469
this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
@@ -439,12 +472,24 @@ export class BundlerContext {
439472
}
440473
}
441474

442-
#addLoadCacheFilesToWatch(): void {
443-
if (this.incremental && this.#loadCache) {
444-
for (const file of this.#loadCache.watchFiles) {
445-
if (!isInternalAngularFile(file)) {
446-
this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
475+
async #addLoadCacheFilesToWatch(): Promise<void> {
476+
const loadCache = this.#loadCache;
477+
if (!this.incremental || !loadCache || !this.#loadCacheKeys) {
478+
return;
479+
}
480+
481+
const results = await Promise.all(Array.from(this.#loadCacheKeys, (key) => loadCache.get(key)));
482+
for (const cachedResult of results) {
483+
if (!cachedResult?.watchFiles) {
484+
continue;
485+
}
486+
487+
for (const file of cachedResult.watchFiles) {
488+
if (isInternalAngularFile(file)) {
489+
continue;
447490
}
491+
492+
this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
448493
}
449494
}
450495
}
@@ -499,30 +544,13 @@ export class BundlerContext {
499544
candidateFiles = normalizedFiles;
500545
}
501546

502-
let invalid = false;
503-
for (const file of candidateFiles) {
504-
if (this.#loadCache?.invalidate(file)) {
505-
invalid = true;
547+
if (this.#loadCache) {
548+
for (const file of candidateFiles) {
549+
this.#loadCache.invalidate(file);
506550
}
507551
}
508552

509-
if (!invalid) {
510-
if (this.watchFiles.size < candidateFiles.size) {
511-
for (const file of this.watchFiles) {
512-
if (candidateFiles.has(file)) {
513-
invalid = true;
514-
break;
515-
}
516-
}
517-
} else {
518-
for (const file of candidateFiles) {
519-
if (this.watchFiles.has(file)) {
520-
invalid = true;
521-
break;
522-
}
523-
}
524-
}
525-
}
553+
const invalid = !this.watchFiles.isDisjointFrom(candidateFiles);
526554

527555
if (invalid) {
528556
this.#invalidationEpoch++;
@@ -544,6 +572,7 @@ export class BundlerContext {
544572
this.#esbuildResult = undefined;
545573
this.#activeBundlePromise = undefined;
546574
this.#loadCache = undefined;
575+
this.#loadCacheKeys = undefined;
547576
await this.#esbuildContext?.dispose();
548577
} finally {
549578
this.#esbuildContext = undefined;

‎packages/angular/build/src/tools/esbuild/bundler-context_spec.ts‎

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,21 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import assert from 'node:assert';
10+
import { mkdtemp, writeFile } from 'node:fs/promises';
11+
import { join } from 'node:path';
912
import { BundlerContext } from './bundler-context';
10-
import { MemoryLoadResultCache } from './load-result-cache';
13+
import { MemoryLoadResultCache, createCachedLoad } from './load-result-cache';
1114

1215
describe('BundlerContext', () => {
16+
let tempRoot: string;
17+
18+
beforeAll(async () => {
19+
const baseTmpDir = process.env['TEST_TMPDIR'];
20+
assert(baseTmpDir, 'TEST_TMPDIR is not set');
21+
tempRoot = await mkdtemp(join(baseTmpDir, 'angular-cli-bundler-context-test-'));
22+
});
23+
1324
describe('invalidate', () => {
1425
it('should return false when incremental is disabled', () => {
1526
const context = new BundlerContext('/workspace', /* incremental */ false, () => ({
@@ -83,12 +94,75 @@ describe('BundlerContext', () => {
8394
/* initialFilter */ undefined,
8495
loadCache,
8596
);
97+
context.watchFiles.add('/workspace/src/app.css');
8698

8799
expect(loadCache.get('file:/workspace/src/app.css')).toBeDefined();
88100
expect(context.invalidate(['/workspace/src/app.css'])).toBeTrue();
89101
expect(loadCache.get('file:/workspace/src/app.css')).toBeUndefined();
90102
});
91103

104+
it('should not invalidate unrelated contexts sharing a load cache', async () => {
105+
const workspaceDir = await mkdtemp(join(tempRoot, 'shared-cache-'));
106+
const compAPath = join(workspaceDir, 'comp-a.css');
107+
const compBPath = join(workspaceDir, 'comp-b.css');
108+
await writeFile(compAPath, '.a { color: red; }');
109+
await writeFile(compBPath, '.b { color: blue; }');
110+
111+
const sharedCache = new MemoryLoadResultCache();
112+
const createCssContext = (entry: string) =>
113+
new BundlerContext(
114+
workspaceDir,
115+
/* incremental */ true,
116+
(loadCache) => ({
117+
absWorkingDir: workspaceDir,
118+
bundle: true,
119+
entryPoints: [entry],
120+
outdir: workspaceDir,
121+
plugins: [
122+
{
123+
name: 'test-cached-css',
124+
setup(build) {
125+
build.onLoad(
126+
{ filter: /\.css$/ },
127+
createCachedLoad(loadCache, async (args) => ({
128+
contents: await import('node:fs/promises').then((f) =>
129+
f.readFile(args.path, 'utf8'),
130+
),
131+
loader: 'css',
132+
watchFiles: [args.path],
133+
})),
134+
);
135+
},
136+
},
137+
],
138+
}),
139+
/* useContext */ false,
140+
/* initialFilter */ undefined,
141+
sharedCache,
142+
);
143+
144+
const contextA = createCssContext(compAPath);
145+
const contextB = createCssContext(compBPath);
146+
147+
const resultA1 = await contextA.bundle();
148+
const resultB1 = await contextB.bundle();
149+
expect(resultA1.errors).toBeUndefined();
150+
expect(resultB1.errors).toBeUndefined();
151+
152+
await writeFile(compAPath, '.a { color: green; }');
153+
154+
// Even if contextB.invalidate runs first against the shared cache, contextB must not be invalidated
155+
expect(contextB.invalidate([compAPath])).toBeFalse();
156+
expect(contextA.invalidate([compAPath])).toBeTrue();
157+
158+
const resultB2 = await contextB.bundle();
159+
const resultA2 = await contextA.bundle();
160+
expect(resultB2).toBe(resultB1);
161+
expect(resultA2).not.toBe(resultA1);
162+
assert(!resultA2.errors);
163+
expect(resultA2.outputFiles[0].text).toContain('green');
164+
});
165+
92166
it('should work when watchFiles is smaller than changed files', () => {
93167
const context = new BundlerContext('/workspace', /* incremental */ true, () => ({
94168
entryPoints: ['main.js'],
@@ -110,4 +184,40 @@ describe('BundlerContext', () => {
110184
expect(context.invalidate(new Set(['/workspace/src/file50.css']))).toBeTrue();
111185
});
112186
});
187+
188+
describe('bundle', () => {
189+
it('should recover when fixing a broken @import in plain CSS in watch mode', async () => {
190+
const workspaceDir = await mkdtemp(join(tempRoot, 'css-import-recovery-'));
191+
const compCssPath = join(workspaceDir, 'comp.css');
192+
const subCssPath = join(workspaceDir, 'sub.css');
193+
await writeFile(compCssPath, "@import './sub.css';");
194+
await writeFile(subCssPath, "@import './non-existent.css';");
195+
196+
const context = new BundlerContext(
197+
workspaceDir,
198+
/* incremental */ true,
199+
() => ({
200+
absWorkingDir: workspaceDir,
201+
bundle: true,
202+
entryPoints: [compCssPath],
203+
outdir: workspaceDir,
204+
logLevel: 'silent',
205+
}),
206+
/* useContext */ false,
207+
);
208+
209+
const failedResult = await context.bundle();
210+
expect(failedResult.errors).toBeDefined();
211+
expect(context.watchFiles.has(subCssPath)).toBeTrue();
212+
expect(context.watchFiles.has(compCssPath)).toBeTrue();
213+
214+
// Fix sub.css and verify invalidation + recovery
215+
await writeFile(subCssPath, '.sub { color: purple; }');
216+
expect(context.invalidate([subCssPath])).toBeTrue();
217+
218+
const recoveredResult = await context.bundle();
219+
assert(!recoveredResult.errors);
220+
expect(recoveredResult.outputFiles[0].text).toContain('purple');
221+
});
222+
});
113223
});

0 commit comments

Comments
 (0)