From 1d48a375d10540ebc6793cf4e7ac0da94629286c Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:59:04 -0400 Subject: [PATCH] refactor(@angular/build): replace maxThreads with maxConcurrency option in i18n inliner The I18nInliner constructor previously accepted an optional maxThreads parameter as a second argument, while sliding window sizing and batch sharding calculations accessed this.#workerPool.maxThreads directly. This couples the inliner concurrency planning to the worker pool construction argument and makes it awkward to configure when using or moving toward shared worker pools. This change adds maxConcurrency to I18nInlinerOptions and removes the separate maxThreads constructor parameter. The internal #maxConcurrency getter prioritizes options.maxConcurrency before falling back to the worker pool thread count. All call sites and tests now pass maxConcurrency through the options object. --- .../build/src/builders/application/i18n.ts | 14 +- .../build/src/tools/esbuild/i18n-inliner.ts | 31 ++- .../src/tools/esbuild/i18n-inliner_spec.ts | 194 ++++++++++++++---- 3 files changed, 182 insertions(+), 57 deletions(-) diff --git a/packages/angular/build/src/builders/application/i18n.ts b/packages/angular/build/src/builders/application/i18n.ts index d2db9b6babd9..f775acdc491d 100644 --- a/packages/angular/build/src/builders/application/i18n.ts +++ b/packages/angular/build/src/builders/application/i18n.ts @@ -45,14 +45,12 @@ export async function inlineI18n( const { i18nOptions, baseHref, cacheOptions } = options; // Create the multi-threaded inliner with common options. - const inliner = new I18nInliner( - { - missingTranslation: i18nOptions.missingTranslationBehavior ?? 'warning', - persistentCachePath: cacheOptions.enabled ? cacheOptions.path : undefined, - localizeVersion: i18nOptions.localizeVersion, - }, - maxWorkers, - ); + const inliner = new I18nInliner({ + missingTranslation: i18nOptions.missingTranslationBehavior ?? 'warning', + maxConcurrency: maxWorkers, + persistentCachePath: cacheOptions.enabled ? cacheOptions.path : undefined, + localizeVersion: i18nOptions.localizeVersion, + }); const inlineResult: { errors: string[]; diff --git a/packages/angular/build/src/tools/esbuild/i18n-inliner.ts b/packages/angular/build/src/tools/esbuild/i18n-inliner.ts index 212095c38761..d76b00c0a31a 100644 --- a/packages/angular/build/src/tools/esbuild/i18n-inliner.ts +++ b/packages/angular/build/src/tools/esbuild/i18n-inliner.ts @@ -121,6 +121,12 @@ async function serializeTranslation( */ export interface I18nInlinerOptions { missingTranslation: 'error' | 'warning' | 'ignore'; + + /** + * The maximum number of concurrent translation inlining operations. + * When omitted, concurrency defaults to the available worker pool threads. + */ + maxConcurrency?: number; persistentCachePath?: string; localizeVersion?: string; } @@ -187,12 +193,23 @@ export class I18nInliner { #translationCache: Cache | undefined; #generation = 0; - constructor( - private readonly options: I18nInlinerOptions, - maxThreads?: number, - ) { + get #maxConcurrency(): number { + return this.options.maxConcurrency ?? (this.#workerPool.maxThreads || 1); + } + + constructor(private readonly options: I18nInlinerOptions) { + if ( + options.maxConcurrency !== undefined && + (!Number.isInteger(options.maxConcurrency) || options.maxConcurrency < 1) + ) { + throw new RangeError('options.maxConcurrency must be an integer greater than or equal to 1.'); + } + + // Piscina uses object spread against default options internally. Only define + // maxThreads when specified to avoid overwriting Piscina's default thread count + // with undefined. this.#workerPool = new WorkerPool({ - maxThreads, + ...(options.maxConcurrency !== undefined && { maxThreads: options.maxConcurrency }), }); } @@ -286,7 +303,7 @@ export class I18nInliner { // Process locales in sliding windows to cap peak worker memory. // Ensure the window has at least enough locales to saturate all available workers on high-core machines. - const windowSize = Math.max(DEFAULT_LOCALE_WINDOW_SIZE, this.#workerPool.maxThreads || 1); + const windowSize = Math.max(DEFAULT_LOCALE_WINDOW_SIZE, this.#maxConcurrency); for (let i = 0; i < localeList.length; i += windowSize) { const windowLocales = localeList.slice(i, i + windowSize); const activeLocales = windowLocales.map((item) => item.locale); @@ -461,7 +478,7 @@ export class I18nInliner { isLastWindow = true, generation?: number, ): Promise { - const workerCount = this.#workerPool.maxThreads || 1; + const workerCount = this.#maxConcurrency; // Extract file data and identify the heaviest file size in a single pass let maxFileSize = 0; diff --git a/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts b/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts index e64efe2fa05b..b02b171cf399 100644 --- a/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts +++ b/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts @@ -12,6 +12,7 @@ import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; import { initializeHash } from '../../utils/hash'; +import { WorkerPool } from '../../utils/worker-pool'; import { type BuildOutputFile, BuildOutputFileType, createOutputFile } from './bundler-files'; import { createPersistentCacheStore } from './cache'; import { I18nInliner, type I18nInlinerOptions } from './i18n-inliner'; @@ -63,8 +64,8 @@ describe('I18nInliner', () => { // A single thread is used throughout so that every file of every locale is inlined by the same // Worker. Any translation state that a Worker retains between requests is then observable. - function createInliner(options?: Partial, maxThreads = 1): I18nInliner { - inliner = new I18nInliner({ missingTranslation: 'warning', ...options }, maxThreads); + function createInliner(options?: Partial, maxConcurrency = 1): I18nInliner { + inliner = new I18nInliner({ missingTranslation: 'warning', maxConcurrency, ...options }); return inliner; } @@ -341,12 +342,10 @@ describe('I18nInliner', () => { browserFile('chunk2.js', GREETING_SOURCE), browserFile('chunk3.js', GREETING_SOURCE), ]; - inliner = new I18nInliner( - { - missingTranslation: 'warning', - }, - 4, - ); + inliner = new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: 4, + }); const { outputFiles, errors, warnings } = await inliner.inlineForLocale(files, 'fr', { greeting: translationFor('Bonjour'), @@ -423,13 +422,11 @@ describe('I18nInliner', () => { }); it('inlines the translations of a locale when localizeVersion is configured in options', async () => { - inliner = new I18nInliner( - { - missingTranslation: 'warning', - localizeVersion: '20.2.0', - }, - 1, - ); + inliner = new I18nInliner({ + missingTranslation: 'warning', + localizeVersion: '20.2.0', + maxConcurrency: 1, + }); const { outputFiles, errors, warnings } = await inliner.inlineForLocale( [browserFile('main.js', GREETING_SOURCE)], @@ -543,13 +540,11 @@ describe('I18nInliner', () => { browserFile('main.js', GREETING_SOURCE), browserFile('other.js', 'export const answer = 42;\n'), ]; - const initialInliner = new I18nInliner( - { - missingTranslation: 'warning', - persistentCachePath: cacheDir, - }, - 2, - ); + const initialInliner = new I18nInliner({ + missingTranslation: 'warning', + persistentCachePath: cacheDir, + maxConcurrency: 2, + }); // Pre-populate cache for 'fr' await initialInliner.inlineForLocale( @@ -561,13 +556,11 @@ describe('I18nInliner', () => { await initialInliner.close(); // Create new inliner with same cache path, inlining cached 'fr' alongside uncached 'de' and 'es' - inliner = new I18nInliner( - { - missingTranslation: 'warning', - persistentCachePath: cacheDir, - }, - 2, - ); + inliner = new I18nInliner({ + missingTranslation: 'warning', + persistentCachePath: cacheDir, + maxConcurrency: 2, + }); const results = await inliner.inlineAll(files, [ { @@ -618,12 +611,10 @@ describe('I18nInliner', () => { })); const files = [browserFile('main.js', GREETING_SOURCE)]; - inliner = new I18nInliner( - { - missingTranslation: 'warning', - }, - 2, - ); + inliner = new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: 2, + }); const results = await inliner.inlineAll(files, locales); @@ -882,12 +873,10 @@ describe('I18nInliner', () => { }); it('correctly transforms files across multiple inlineAll runs on the same inliner instance', async () => { - const localeInliner = new I18nInliner( - { - missingTranslation: 'warning', - }, - 2, - ); + const localeInliner = new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: 2, + }); try { const files1 = [browserFile('main.js', GREETING_SOURCE)]; @@ -942,7 +931,7 @@ describe('I18nInliner', () => { const smallFile = browserFile('chunk.js', smallSource); const files = [largeFile, smallFile]; - inliner = new I18nInliner({ missingTranslation: 'error' }, 2); + inliner = new I18nInliner({ missingTranslation: 'error', maxConcurrency: 2 }); const results = await inliner.inlineAll(files, [ { @@ -982,4 +971,125 @@ describe('I18nInliner', () => { expect(findFile(esFiles, 'main.js').text).toContain('"Hola"'); expect(findFile(esFiles, 'chunk.js').text).toContain('"Adios"'); }); + + it('respects maxConcurrency when processing multiple locales and files', async () => { + const files = [ + browserFile('main.js', GREETING_SOURCE), + browserFile('chunk.js', 'console.log($localize`:@@farewell:Goodbye`);'), + ]; + + inliner = new I18nInliner({ + missingTranslation: 'error', + maxConcurrency: 1, + }); + + const results = await inliner.inlineAll(files, [ + { + locale: 'fr', + translation: { + greeting: translationFor('Bonjour'), + farewell: translationFor('Au revoir'), + }, + }, + { + locale: 'de', + translation: { + greeting: translationFor('Guten Tag'), + farewell: translationFor('Auf Wiedersehen'), + }, + }, + ]); + + expect(results.size).toBe(2); + expect(findFile(results.get('fr')?.outputFiles ?? [], 'main.js').text).toContain('"Bonjour"'); + expect(findFile(results.get('de')?.outputFiles ?? [], 'chunk.js').text).toContain( + '"Auf Wiedersehen"', + ); + }); + + describe('maxConcurrency validation', () => { + it('throws when maxConcurrency is less than 1', () => { + expect( + () => + new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: 0, + }), + ).toThrowError( + RangeError, + 'options.maxConcurrency must be an integer greater than or equal to 1.', + ); + + expect( + () => + new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: -1, + }), + ).toThrowError( + RangeError, + 'options.maxConcurrency must be an integer greater than or equal to 1.', + ); + }); + + it('throws when maxConcurrency is not an integer', () => { + expect( + () => + new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: 1.5, + }), + ).toThrowError( + RangeError, + 'options.maxConcurrency must be an integer greater than or equal to 1.', + ); + + expect( + () => + new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: NaN, + }), + ).toThrowError( + RangeError, + 'options.maxConcurrency must be an integer greater than or equal to 1.', + ); + + expect( + () => + new I18nInliner({ + missingTranslation: 'warning', + maxConcurrency: Infinity, + }), + ).toThrowError( + RangeError, + 'options.maxConcurrency must be an integer greater than or equal to 1.', + ); + }); + + it('defaults to worker pool concurrency when maxConcurrency is omitted', async () => { + const maxThreadsSpy = spyOnProperty( + WorkerPool.prototype, + 'maxThreads', + 'get', + ).and.callThrough(); + + inliner = new I18nInliner({ + missingTranslation: 'warning', + }); + + const files = [browserFile('main.js', GREETING_SOURCE)]; + const results = await inliner.inlineAll(files, [ + { + locale: 'fr', + translation: { greeting: translationFor('Bonjour') }, + }, + ]); + + expect(results.size).toBe(1); + expect(findFile(results.get('fr')?.outputFiles ?? [], 'main.js').text).toContain('"Bonjour"'); + expect(maxThreadsSpy).toHaveBeenCalled(); + expect(maxThreadsSpy.calls.mostRecent().returnValue).toBeGreaterThanOrEqual(1); + }); + }); });