diff --git a/packages/angular/build/src/tools/esbuild/bundler-context.ts b/packages/angular/build/src/tools/esbuild/bundler-context.ts index dd2764c8c608..7fe28e5a51d4 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-context.ts @@ -59,6 +59,8 @@ export class BundlerContext { #esbuildContext?: BuildContext<{ metafile: true; write: false }>; #esbuildOptions?: BuildOptions & { metafile: true; write: false }; #esbuildResult?: BundleContextResult; + #activeBundlePromise?: Promise; + #disposed = false; #optionsFactory: BundlerOptionsFactory; #shouldCacheResult: boolean; #loadCache?: MemoryLoadResultCache; @@ -177,7 +179,18 @@ export class BundlerContext { return this.#esbuildResult; } - const result = await this.#performBundle(); + if (!force && this.#activeBundlePromise) { + return this.#activeBundlePromise; + } + + const bundlePromise = this.#performBundle().finally(() => { + if (this.#activeBundlePromise === bundlePromise) { + this.#activeBundlePromise = undefined; + } + }); + this.#activeBundlePromise = bundlePromise; + + const result = await bundlePromise; if (this.#shouldCacheResult) { this.#esbuildResult = result; } @@ -207,7 +220,12 @@ export class BundlerContext { } else { // Create a build context and perform the build. // Context creation does not perform a build. - this.#esbuildContext = await context(this.#esbuildOptions); + const esbuildContext = await context(this.#esbuildOptions); + if (this.#disposed) { + await esbuildContext.dispose(); + throw new Error('BundlerContext was disposed during build.'); + } + this.#esbuildContext = esbuildContext; result = await this.#esbuildContext.rebuild(); } } catch (failure) { @@ -446,9 +464,11 @@ export class BundlerContext { * @returns A promise that resolves when disposal is complete. */ async dispose(): Promise { + this.#disposed = true; try { this.#esbuildOptions = undefined; this.#esbuildResult = undefined; + this.#activeBundlePromise = undefined; this.#loadCache = undefined; await this.#esbuildContext?.dispose(); } finally { diff --git a/packages/angular/build/src/tools/esbuild/cache.ts b/packages/angular/build/src/tools/esbuild/cache.ts index 5bd0dc84d73f..bd6e4cee72fc 100644 --- a/packages/angular/build/src/tools/esbuild/cache.ts +++ b/packages/angular/build/src/tools/esbuild/cache.ts @@ -44,11 +44,18 @@ export interface CacheStore { * to use a cache. */ export class Cache = CacheStore> { + readonly #requests = new Map>(); + readonly #writeCounts = new Map(); + constructor( protected readonly store: S, readonly namespace?: string, ) {} + #incrementWrite(key: string) { + this.#writeCounts.set(key, (this.#writeCounts.get(key) || 0) + 1); + } + /** * Prefixes a key with the cache namespace if present. * @param key A key string to prefix. @@ -72,14 +79,52 @@ export class Cache = CacheStore> { */ async getOrCreate(key: string, creator: () => V | Promise): Promise { const namespacedKey = this.withNamespace(key); - let value = await this.store.get(namespacedKey); - if (value === undefined) { - value = await creator(); - await this.store.set(namespacedKey, value); + let activeRequest = this.#requests.get(namespacedKey); + if (activeRequest !== undefined) { + return activeRequest; } - return value; + const startWriteCount = this.#writeCounts.get(namespacedKey) || 0; + + const value = await this.store.get(namespacedKey); + + // If a write occurred during the await gap, we must re-evaluate + if ((this.#writeCounts.get(namespacedKey) || 0) !== startWriteCount) { + return this.getOrCreate(key, creator); + } + + if (value !== undefined) { + return value; + } + + // Recheck active request after the await gap in case another one was initiated + activeRequest = this.#requests.get(namespacedKey); + if (activeRequest !== undefined) { + return activeRequest; + } + + activeRequest = Promise.resolve(creator()).then( + async (newValue) => { + if (this.#requests.get(namespacedKey) === activeRequest) { + this.#incrementWrite(namespacedKey); + await this.store.set(namespacedKey, newValue); + this.#requests.delete(namespacedKey); + } + + return newValue; + }, + (error) => { + if (this.#requests.get(namespacedKey) === activeRequest) { + this.#requests.delete(namespacedKey); + } + throw error; + }, + ); + + this.#requests.set(namespacedKey, activeRequest); + + return activeRequest; } /** @@ -100,7 +145,18 @@ export class Cache = CacheStore> { * @param value A value to put in the cache. */ async put(key: string, value: V): Promise { - await this.store.set(this.withNamespace(key), value); + const namespacedKey = this.withNamespace(key); + this.#requests.delete(namespacedKey); + this.#incrementWrite(namespacedKey); + await this.store.set(namespacedKey, value); + } + + /** + * Clears the base class internal state (requests and write counts). + */ + protected clearInternal(): void { + this.#requests.clear(); + this.#writeCounts.clear(); } } @@ -116,6 +172,7 @@ export class MemoryCache extends Cache> { * Removes all entries from the cache instance. */ clear() { + this.clearInternal(); this.store.clear(); } diff --git a/packages/angular/build/src/tools/esbuild/cache_spec.ts b/packages/angular/build/src/tools/esbuild/cache_spec.ts new file mode 100644 index 000000000000..bcd70cda9153 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/cache_spec.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright Google LLC 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.dev/license + */ + +import { MemoryCache } from './cache'; + +describe('MemoryCache', () => { + let cache: MemoryCache; + + beforeEach(() => { + cache = new MemoryCache(); + }); + + it('should return cached value on subsequent getOrCreate calls', async () => { + let callCount = 0; + const creator = () => { + callCount++; + + return 'value'; + }; + + const val1 = await cache.getOrCreate('key', creator); + const val2 = await cache.getOrCreate('key', creator); + + expect(val1).toBe('value'); + expect(val2).toBe('value'); + expect(callCount).toBe(1); + }); + + it('should call creator only once for concurrent getOrCreate calls with the same key', async () => { + let callCount = 0; + let resolveCreator!: (value: string) => void; + const promise = new Promise((resolve) => { + resolveCreator = resolve; + }); + + const creator = () => { + callCount++; + + return promise; + }; + + const p1 = cache.getOrCreate('key', creator); + const p2 = cache.getOrCreate('key', creator); + + resolveCreator('concurrent-value'); + + const [val1, val2] = await Promise.all([p1, p2]); + + expect(val1).toBe('concurrent-value'); + expect(val2).toBe('concurrent-value'); + expect(callCount).toBe(1); + }); + + it('should call creator multiple times for concurrent getOrCreate calls with different keys', async () => { + let callCount = 0; + const creator = (val: string) => { + callCount++; + + return Promise.resolve(val); + }; + + const p1 = cache.getOrCreate('key1', () => creator('value1')); + const p2 = cache.getOrCreate('key2', () => creator('value2')); + + const [val1, val2] = await Promise.all([p1, p2]); + + expect(val1).toBe('value1'); + expect(val2).toBe('value2'); + expect(callCount).toBe(2); + }); + + it('should clean up active request if creator throws/rejects', async () => { + let callCount = 0; + let rejectCreator!: (err: Error) => void; + const promise = new Promise((_, reject) => { + rejectCreator = reject; + }); + + const creator = () => { + callCount++; + + return promise; + }; + + const p1 = cache.getOrCreate('key', creator); + const p2 = cache.getOrCreate('key', creator); + + rejectCreator(new Error('creator error')); + + await expectAsync(p1).toBeRejectedWithError('creator error'); + await expectAsync(p2).toBeRejectedWithError('creator error'); + + // Subsequent call should trigger the creator again + const p3 = cache.getOrCreate('key', () => { + callCount++; + + return Promise.resolve('new-value'); + }); + const val3 = await p3; + expect(val3).toBe('new-value'); + expect(callCount).toBe(2); + }); + + it('should override/clear active requests when put is called', async () => { + let resolveCreator!: (value: string) => void; + const promise = new Promise((resolve) => { + resolveCreator = resolve; + }); + + const creator = () => promise; + + const p1 = cache.getOrCreate('key', creator); + + // Call put before the creator promise resolves + await cache.put('key', 'override-value'); + + resolveCreator('original-value'); + + const val1 = await p1; + expect(val1).toBe('override-value'); + + // Subsequent getOrCreate should return the put/overridden value, not the resolved original-value + const val2 = await cache.getOrCreate('key', () => 'should-not-run'); + expect(val2).toBe('override-value'); + }); +});