Skip to content

Commit 2d53b78

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 d609a21 commit 2d53b78

2 files changed

Lines changed: 183 additions & 31 deletions

File tree

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

Lines changed: 69 additions & 30 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+
this.watchFiles.add(
455+
isAbsolute(entry) ? normalize(entry) : join(this.workspaceRoot, entry),
456+
);
457+
}
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,11 +472,18 @@ 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+
if (this.incremental && this.#loadCache && this.#loadCacheKeys) {
477+
for (const key of this.#loadCacheKeys) {
478+
const cachedResult = await this.#loadCache.get(key);
479+
if (cachedResult?.watchFiles) {
480+
for (const file of cachedResult.watchFiles) {
481+
if (!isInternalAngularFile(file)) {
482+
this.watchFiles.add(
483+
isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file),
484+
);
485+
}
486+
}
447487
}
448488
}
449489
}
@@ -499,27 +539,25 @@ export class BundlerContext {
499539
candidateFiles = normalizedFiles;
500540
}
501541

502-
let invalid = false;
503-
for (const file of candidateFiles) {
504-
if (this.#loadCache?.invalidate(file)) {
505-
invalid = true;
542+
if (this.#loadCache) {
543+
for (const file of candidateFiles) {
544+
this.#loadCache.invalidate(file);
506545
}
507546
}
508547

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-
}
548+
let invalid = false;
549+
if (this.watchFiles.size < candidateFiles.size) {
550+
for (const file of this.watchFiles) {
551+
if (candidateFiles.has(file)) {
552+
invalid = true;
553+
break;
516554
}
517-
} else {
518-
for (const file of candidateFiles) {
519-
if (this.watchFiles.has(file)) {
520-
invalid = true;
521-
break;
522-
}
555+
}
556+
} else {
557+
for (const file of candidateFiles) {
558+
if (this.watchFiles.has(file)) {
559+
invalid = true;
560+
break;
523561
}
524562
}
525563
}
@@ -544,6 +582,7 @@ export class BundlerContext {
544582
this.#esbuildResult = undefined;
545583
this.#activeBundlePromise = undefined;
546584
this.#loadCache = undefined;
585+
this.#loadCacheKeys = undefined;
547586
await this.#esbuildContext?.dispose();
548587
} finally {
549588
this.#esbuildContext = undefined;

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

Lines changed: 114 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,76 @@ 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+
if (!resultA2.errors) {
163+
expect(resultA2.outputFiles[0].text).toContain('green');
164+
}
165+
});
166+
92167
it('should work when watchFiles is smaller than changed files', () => {
93168
const context = new BundlerContext('/workspace', /* incremental */ true, () => ({
94169
entryPoints: ['main.js'],
@@ -110,4 +185,42 @@ describe('BundlerContext', () => {
110185
expect(context.invalidate(new Set(['/workspace/src/file50.css']))).toBeTrue();
111186
});
112187
});
188+
189+
describe('bundle', () => {
190+
it('should recover when fixing a broken @import in plain CSS in watch mode', async () => {
191+
const workspaceDir = await mkdtemp(join(tempRoot, 'css-import-recovery-'));
192+
const compCssPath = join(workspaceDir, 'comp.css');
193+
const subCssPath = join(workspaceDir, 'sub.css');
194+
await writeFile(compCssPath, "@import './sub.css';");
195+
await writeFile(subCssPath, "@import './non-existent.css';");
196+
197+
const context = new BundlerContext(
198+
workspaceDir,
199+
/* incremental */ true,
200+
() => ({
201+
absWorkingDir: workspaceDir,
202+
bundle: true,
203+
entryPoints: [compCssPath],
204+
outdir: workspaceDir,
205+
logLevel: 'silent',
206+
}),
207+
/* useContext */ false,
208+
);
209+
210+
const failedResult = await context.bundle();
211+
expect(failedResult.errors).toBeDefined();
212+
expect(context.watchFiles.has(subCssPath)).toBeTrue();
213+
expect(context.watchFiles.has(compCssPath)).toBeTrue();
214+
215+
// Fix sub.css and verify invalidation + recovery
216+
await writeFile(subCssPath, '.sub { color: purple; }');
217+
expect(context.invalidate([subCssPath])).toBeTrue();
218+
219+
const recoveredResult = await context.bundle();
220+
expect(recoveredResult.errors).toBeUndefined();
221+
if (!recoveredResult.errors) {
222+
expect(recoveredResult.outputFiles[0].text).toContain('purple');
223+
}
224+
});
225+
});
113226
});

0 commit comments

Comments
 (0)