diff --git a/packages/angular/build/src/tools/esbuild/bundler-context.ts b/packages/angular/build/src/tools/esbuild/bundler-context.ts index c2a53df67b8c..838cee086d64 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-context.ts @@ -81,6 +81,7 @@ export class BundlerContext { #optionsFactory: BundlerOptionsFactory; #shouldCacheResult: boolean; #loadCache?: LoadResultCache; + #loadCacheKeys?: Set; #invalidationEpoch = 0; readonly watchFiles = new Set(); @@ -227,11 +228,27 @@ export class BundlerContext { if (this.incremental && !this.#loadCache) { this.#loadCache = new MemoryLoadResultCache(); } - this.#esbuildOptions = this.#optionsFactory(this.#loadCache); - } - - if (this.incremental) { - this.watchFiles.clear(); + const loadCache = this.#loadCache; + const scopedLoadCache: LoadResultCache | undefined = + this.incremental && loadCache + ? { + get: (path) => { + (this.#loadCacheKeys ??= new Set()).add(path); + + return loadCache.get(path); + }, + put: (path, result) => { + (this.#loadCacheKeys ??= new Set()).add(path); + + return loadCache.put(path, result); + }, + invalidate: (path) => loadCache.invalidate(path), + get watchFiles() { + return loadCache.watchFiles; + }, + } + : loadCache; + this.#esbuildOptions = this.#optionsFactory(scopedLoadCache); } let result: BuildResult<{ metafile: true; write: false }>; @@ -263,7 +280,7 @@ export class BundlerContext { // Build failures will throw an exception which contains errors/warnings if (isEsBuildFailure(failure)) { this.#addErrorsToWatch(failure); - this.#addLoadCacheFilesToWatch(); + await this.#addLoadCacheFilesToWatch(); return failure; } else { @@ -275,6 +292,8 @@ export class BundlerContext { // While this should technically not be linked to incremental mode, incremental is only // currently enabled with watch mode where watch files are needed. if (this.incremental) { + this.watchFiles.clear(); + // Add input files except virtual angular files which do not exist on disk for (const input of Object.keys(result.metafile.inputs)) { const isInternal = isInternalAngularFile(input) || isInternalBundlerFile(input); @@ -309,7 +328,7 @@ export class BundlerContext { // Return if the build encountered any errors if (result.errors.length) { this.#addErrorsToWatch(result); - this.#addLoadCacheFilesToWatch(); + await this.#addLoadCacheFilesToWatch(); return { errors: result.errors, @@ -425,12 +444,26 @@ export class BundlerContext { } #addErrorsToWatch(result: BuildFailure | BuildResult): void { + const entryPoints = this.#esbuildOptions?.entryPoints; + if (entryPoints) { + const entries = Array.isArray(entryPoints) + ? entryPoints.map((entry) => (typeof entry === 'string' ? entry : entry.in)) + : Object.values(entryPoints); + for (const entry of entries) { + if (!isInternalAngularFile(entry) && !isInternalBundlerFile(entry)) { + this.watchFiles.add( + isAbsolute(entry) ? normalize(entry) : join(this.workspaceRoot, entry), + ); + } + } + } + for (const error of result.errors) { let file = error.location?.file; if (file && !isInternalAngularFile(file)) { this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file)); } - for (const note of error.notes) { + for (const note of error.notes ?? []) { file = note.location?.file; if (file && !isInternalAngularFile(file)) { this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file)); @@ -439,11 +472,21 @@ export class BundlerContext { } } - #addLoadCacheFilesToWatch(): void { - if (this.incremental && this.#loadCache) { - for (const file of this.#loadCache.watchFiles) { - if (!isInternalAngularFile(file)) { - this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file)); + async #addLoadCacheFilesToWatch(): Promise { + const loadCache = this.#loadCache; + if (this.incremental && loadCache && this.#loadCacheKeys) { + const results = await Promise.all( + Array.from(this.#loadCacheKeys, (key) => loadCache.get(key)), + ); + for (const cachedResult of results) { + if (cachedResult?.watchFiles) { + for (const file of cachedResult.watchFiles) { + if (!isInternalAngularFile(file)) { + this.watchFiles.add( + isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file), + ); + } + } } } } @@ -499,30 +542,13 @@ export class BundlerContext { candidateFiles = normalizedFiles; } - let invalid = false; - for (const file of candidateFiles) { - if (this.#loadCache?.invalidate(file)) { - invalid = true; + if (this.#loadCache) { + for (const file of candidateFiles) { + this.#loadCache.invalidate(file); } } - if (!invalid) { - if (this.watchFiles.size < candidateFiles.size) { - for (const file of this.watchFiles) { - if (candidateFiles.has(file)) { - invalid = true; - break; - } - } - } else { - for (const file of candidateFiles) { - if (this.watchFiles.has(file)) { - invalid = true; - break; - } - } - } - } + const invalid = !this.watchFiles.isDisjointFrom(candidateFiles); if (invalid) { this.#invalidationEpoch++; @@ -544,6 +570,7 @@ export class BundlerContext { this.#esbuildResult = undefined; this.#activeBundlePromise = undefined; this.#loadCache = undefined; + this.#loadCacheKeys = undefined; await this.#esbuildContext?.dispose(); } finally { this.#esbuildContext = undefined; diff --git a/packages/angular/build/src/tools/esbuild/bundler-context_spec.ts b/packages/angular/build/src/tools/esbuild/bundler-context_spec.ts index 213354389122..8c5f3a6b79b0 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context_spec.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-context_spec.ts @@ -6,10 +6,21 @@ * found in the LICENSE file at https://angular.dev/license */ +import assert from 'node:assert'; +import { mkdtemp, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; import { BundlerContext } from './bundler-context'; -import { MemoryLoadResultCache } from './load-result-cache'; +import { MemoryLoadResultCache, createCachedLoad } from './load-result-cache'; describe('BundlerContext', () => { + let tempRoot: string; + + beforeAll(async () => { + const baseTmpDir = process.env['TEST_TMPDIR']; + assert(baseTmpDir, 'TEST_TMPDIR is not set'); + tempRoot = await mkdtemp(join(baseTmpDir, 'angular-cli-bundler-context-test-')); + }); + describe('invalidate', () => { it('should return false when incremental is disabled', () => { const context = new BundlerContext('/workspace', /* incremental */ false, () => ({ @@ -83,12 +94,76 @@ describe('BundlerContext', () => { /* initialFilter */ undefined, loadCache, ); + context.watchFiles.add('/workspace/src/app.css'); expect(loadCache.get('file:/workspace/src/app.css')).toBeDefined(); expect(context.invalidate(['/workspace/src/app.css'])).toBeTrue(); expect(loadCache.get('file:/workspace/src/app.css')).toBeUndefined(); }); + it('should not invalidate unrelated contexts sharing a load cache', async () => { + const workspaceDir = await mkdtemp(join(tempRoot, 'shared-cache-')); + const compAPath = join(workspaceDir, 'comp-a.css'); + const compBPath = join(workspaceDir, 'comp-b.css'); + await writeFile(compAPath, '.a { color: red; }'); + await writeFile(compBPath, '.b { color: blue; }'); + + const sharedCache = new MemoryLoadResultCache(); + const createCssContext = (entry: string) => + new BundlerContext( + workspaceDir, + /* incremental */ true, + (loadCache) => ({ + absWorkingDir: workspaceDir, + bundle: true, + entryPoints: [entry], + outdir: workspaceDir, + plugins: [ + { + name: 'test-cached-css', + setup(build) { + build.onLoad( + { filter: /\.css$/ }, + createCachedLoad(loadCache, async (args) => ({ + contents: await import('node:fs/promises').then((f) => + f.readFile(args.path, 'utf8'), + ), + loader: 'css', + watchFiles: [args.path], + })), + ); + }, + }, + ], + }), + /* useContext */ false, + /* initialFilter */ undefined, + sharedCache, + ); + + const contextA = createCssContext(compAPath); + const contextB = createCssContext(compBPath); + + const resultA1 = await contextA.bundle(); + const resultB1 = await contextB.bundle(); + expect(resultA1.errors).toBeUndefined(); + expect(resultB1.errors).toBeUndefined(); + + await writeFile(compAPath, '.a { color: green; }'); + + // Even if contextB.invalidate runs first against the shared cache, contextB must not be invalidated + expect(contextB.invalidate([compAPath])).toBeFalse(); + expect(contextA.invalidate([compAPath])).toBeTrue(); + + const resultB2 = await contextB.bundle(); + const resultA2 = await contextA.bundle(); + expect(resultB2).toBe(resultB1); + expect(resultA2).not.toBe(resultA1); + if (!resultA2.errors) { + expect(resultA2.outputFiles[0].text).toContain('green'); + } + }); + it('should work when watchFiles is smaller than changed files', () => { const context = new BundlerContext('/workspace', /* incremental */ true, () => ({ entryPoints: ['main.js'], @@ -110,4 +185,42 @@ describe('BundlerContext', () => { expect(context.invalidate(new Set(['/workspace/src/file50.css']))).toBeTrue(); }); }); + + describe('bundle', () => { + it('should recover when fixing a broken @import in plain CSS in watch mode', async () => { + const workspaceDir = await mkdtemp(join(tempRoot, 'css-import-recovery-')); + const compCssPath = join(workspaceDir, 'comp.css'); + const subCssPath = join(workspaceDir, 'sub.css'); + await writeFile(compCssPath, "@import './sub.css';"); + await writeFile(subCssPath, "@import './non-existent.css';"); + + const context = new BundlerContext( + workspaceDir, + /* incremental */ true, + () => ({ + absWorkingDir: workspaceDir, + bundle: true, + entryPoints: [compCssPath], + outdir: workspaceDir, + logLevel: 'silent', + }), + /* useContext */ false, + ); + + const failedResult = await context.bundle(); + expect(failedResult.errors).toBeDefined(); + expect(context.watchFiles.has(subCssPath)).toBeTrue(); + expect(context.watchFiles.has(compCssPath)).toBeTrue(); + + // Fix sub.css and verify invalidation + recovery + await writeFile(subCssPath, '.sub { color: purple; }'); + expect(context.invalidate([subCssPath])).toBeTrue(); + + const recoveredResult = await context.bundle(); + expect(recoveredResult.errors).toBeUndefined(); + if (!recoveredResult.errors) { + expect(recoveredResult.outputFiles[0].text).toContain('purple'); + } + }); + }); });