Skip to content

Commit 1d48a37

Browse files
committed
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.
1 parent 6ee559c commit 1d48a37

3 files changed

Lines changed: 182 additions & 57 deletions

File tree

packages/angular/build/src/builders/application/i18n.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,12 @@ export async function inlineI18n(
4545
const { i18nOptions, baseHref, cacheOptions } = options;
4646

4747
// Create the multi-threaded inliner with common options.
48-
const inliner = new I18nInliner(
49-
{
50-
missingTranslation: i18nOptions.missingTranslationBehavior ?? 'warning',
51-
persistentCachePath: cacheOptions.enabled ? cacheOptions.path : undefined,
52-
localizeVersion: i18nOptions.localizeVersion,
53-
},
54-
maxWorkers,
55-
);
48+
const inliner = new I18nInliner({
49+
missingTranslation: i18nOptions.missingTranslationBehavior ?? 'warning',
50+
maxConcurrency: maxWorkers,
51+
persistentCachePath: cacheOptions.enabled ? cacheOptions.path : undefined,
52+
localizeVersion: i18nOptions.localizeVersion,
53+
});
5654

5755
const inlineResult: {
5856
errors: string[];

packages/angular/build/src/tools/esbuild/i18n-inliner.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,12 @@ async function serializeTranslation(
121121
*/
122122
export interface I18nInlinerOptions {
123123
missingTranslation: 'error' | 'warning' | 'ignore';
124+
125+
/**
126+
* The maximum number of concurrent translation inlining operations.
127+
* When omitted, concurrency defaults to the available worker pool threads.
128+
*/
129+
maxConcurrency?: number;
124130
persistentCachePath?: string;
125131
localizeVersion?: string;
126132
}
@@ -187,12 +193,23 @@ export class I18nInliner {
187193
#translationCache: Cache<Uint8Array> | undefined;
188194
#generation = 0;
189195

190-
constructor(
191-
private readonly options: I18nInlinerOptions,
192-
maxThreads?: number,
193-
) {
196+
get #maxConcurrency(): number {
197+
return this.options.maxConcurrency ?? (this.#workerPool.maxThreads || 1);
198+
}
199+
200+
constructor(private readonly options: I18nInlinerOptions) {
201+
if (
202+
options.maxConcurrency !== undefined &&
203+
(!Number.isInteger(options.maxConcurrency) || options.maxConcurrency < 1)
204+
) {
205+
throw new RangeError('options.maxConcurrency must be an integer greater than or equal to 1.');
206+
}
207+
208+
// Piscina uses object spread against default options internally. Only define
209+
// maxThreads when specified to avoid overwriting Piscina's default thread count
210+
// with undefined.
194211
this.#workerPool = new WorkerPool({
195-
maxThreads,
212+
...(options.maxConcurrency !== undefined && { maxThreads: options.maxConcurrency }),
196213
});
197214
}
198215

@@ -286,7 +303,7 @@ export class I18nInliner {
286303

287304
// Process locales in sliding windows to cap peak worker memory.
288305
// Ensure the window has at least enough locales to saturate all available workers on high-core machines.
289-
const windowSize = Math.max(DEFAULT_LOCALE_WINDOW_SIZE, this.#workerPool.maxThreads || 1);
306+
const windowSize = Math.max(DEFAULT_LOCALE_WINDOW_SIZE, this.#maxConcurrency);
290307
for (let i = 0; i < localeList.length; i += windowSize) {
291308
const windowLocales = localeList.slice(i, i + windowSize);
292309
const activeLocales = windowLocales.map((item) => item.locale);
@@ -461,7 +478,7 @@ export class I18nInliner {
461478
isLastWindow = true,
462479
generation?: number,
463480
): Promise<void> {
464-
const workerCount = this.#workerPool.maxThreads || 1;
481+
const workerCount = this.#maxConcurrency;
465482

466483
// Extract file data and identify the heaviest file size in a single pass
467484
let maxFileSize = 0;

packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts

Lines changed: 152 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import fs from 'node:fs/promises';
1212
import os from 'node:os';
1313
import path from 'node:path';
1414
import { initializeHash } from '../../utils/hash';
15+
import { WorkerPool } from '../../utils/worker-pool';
1516
import { type BuildOutputFile, BuildOutputFileType, createOutputFile } from './bundler-files';
1617
import { createPersistentCacheStore } from './cache';
1718
import { I18nInliner, type I18nInlinerOptions } from './i18n-inliner';
@@ -63,8 +64,8 @@ describe('I18nInliner', () => {
6364

6465
// A single thread is used throughout so that every file of every locale is inlined by the same
6566
// Worker. Any translation state that a Worker retains between requests is then observable.
66-
function createInliner(options?: Partial<I18nInlinerOptions>, maxThreads = 1): I18nInliner {
67-
inliner = new I18nInliner({ missingTranslation: 'warning', ...options }, maxThreads);
67+
function createInliner(options?: Partial<I18nInlinerOptions>, maxConcurrency = 1): I18nInliner {
68+
inliner = new I18nInliner({ missingTranslation: 'warning', maxConcurrency, ...options });
6869

6970
return inliner;
7071
}
@@ -341,12 +342,10 @@ describe('I18nInliner', () => {
341342
browserFile('chunk2.js', GREETING_SOURCE),
342343
browserFile('chunk3.js', GREETING_SOURCE),
343344
];
344-
inliner = new I18nInliner(
345-
{
346-
missingTranslation: 'warning',
347-
},
348-
4,
349-
);
345+
inliner = new I18nInliner({
346+
missingTranslation: 'warning',
347+
maxConcurrency: 4,
348+
});
350349

351350
const { outputFiles, errors, warnings } = await inliner.inlineForLocale(files, 'fr', {
352351
greeting: translationFor('Bonjour'),
@@ -423,13 +422,11 @@ describe('I18nInliner', () => {
423422
});
424423

425424
it('inlines the translations of a locale when localizeVersion is configured in options', async () => {
426-
inliner = new I18nInliner(
427-
{
428-
missingTranslation: 'warning',
429-
localizeVersion: '20.2.0',
430-
},
431-
1,
432-
);
425+
inliner = new I18nInliner({
426+
missingTranslation: 'warning',
427+
localizeVersion: '20.2.0',
428+
maxConcurrency: 1,
429+
});
433430

434431
const { outputFiles, errors, warnings } = await inliner.inlineForLocale(
435432
[browserFile('main.js', GREETING_SOURCE)],
@@ -543,13 +540,11 @@ describe('I18nInliner', () => {
543540
browserFile('main.js', GREETING_SOURCE),
544541
browserFile('other.js', 'export const answer = 42;\n'),
545542
];
546-
const initialInliner = new I18nInliner(
547-
{
548-
missingTranslation: 'warning',
549-
persistentCachePath: cacheDir,
550-
},
551-
2,
552-
);
543+
const initialInliner = new I18nInliner({
544+
missingTranslation: 'warning',
545+
persistentCachePath: cacheDir,
546+
maxConcurrency: 2,
547+
});
553548

554549
// Pre-populate cache for 'fr'
555550
await initialInliner.inlineForLocale(
@@ -561,13 +556,11 @@ describe('I18nInliner', () => {
561556
await initialInliner.close();
562557

563558
// Create new inliner with same cache path, inlining cached 'fr' alongside uncached 'de' and 'es'
564-
inliner = new I18nInliner(
565-
{
566-
missingTranslation: 'warning',
567-
persistentCachePath: cacheDir,
568-
},
569-
2,
570-
);
559+
inliner = new I18nInliner({
560+
missingTranslation: 'warning',
561+
persistentCachePath: cacheDir,
562+
maxConcurrency: 2,
563+
});
571564

572565
const results = await inliner.inlineAll(files, [
573566
{
@@ -618,12 +611,10 @@ describe('I18nInliner', () => {
618611
}));
619612

620613
const files = [browserFile('main.js', GREETING_SOURCE)];
621-
inliner = new I18nInliner(
622-
{
623-
missingTranslation: 'warning',
624-
},
625-
2,
626-
);
614+
inliner = new I18nInliner({
615+
missingTranslation: 'warning',
616+
maxConcurrency: 2,
617+
});
627618

628619
const results = await inliner.inlineAll(files, locales);
629620

@@ -882,12 +873,10 @@ describe('I18nInliner', () => {
882873
});
883874

884875
it('correctly transforms files across multiple inlineAll runs on the same inliner instance', async () => {
885-
const localeInliner = new I18nInliner(
886-
{
887-
missingTranslation: 'warning',
888-
},
889-
2,
890-
);
876+
const localeInliner = new I18nInliner({
877+
missingTranslation: 'warning',
878+
maxConcurrency: 2,
879+
});
891880

892881
try {
893882
const files1 = [browserFile('main.js', GREETING_SOURCE)];
@@ -942,7 +931,7 @@ describe('I18nInliner', () => {
942931
const smallFile = browserFile('chunk.js', smallSource);
943932
const files = [largeFile, smallFile];
944933

945-
inliner = new I18nInliner({ missingTranslation: 'error' }, 2);
934+
inliner = new I18nInliner({ missingTranslation: 'error', maxConcurrency: 2 });
946935

947936
const results = await inliner.inlineAll(files, [
948937
{
@@ -982,4 +971,125 @@ describe('I18nInliner', () => {
982971
expect(findFile(esFiles, 'main.js').text).toContain('"Hola"');
983972
expect(findFile(esFiles, 'chunk.js').text).toContain('"Adios"');
984973
});
974+
975+
it('respects maxConcurrency when processing multiple locales and files', async () => {
976+
const files = [
977+
browserFile('main.js', GREETING_SOURCE),
978+
browserFile('chunk.js', 'console.log($localize`:@@farewell:Goodbye`);'),
979+
];
980+
981+
inliner = new I18nInliner({
982+
missingTranslation: 'error',
983+
maxConcurrency: 1,
984+
});
985+
986+
const results = await inliner.inlineAll(files, [
987+
{
988+
locale: 'fr',
989+
translation: {
990+
greeting: translationFor('Bonjour'),
991+
farewell: translationFor('Au revoir'),
992+
},
993+
},
994+
{
995+
locale: 'de',
996+
translation: {
997+
greeting: translationFor('Guten Tag'),
998+
farewell: translationFor('Auf Wiedersehen'),
999+
},
1000+
},
1001+
]);
1002+
1003+
expect(results.size).toBe(2);
1004+
expect(findFile(results.get('fr')?.outputFiles ?? [], 'main.js').text).toContain('"Bonjour"');
1005+
expect(findFile(results.get('de')?.outputFiles ?? [], 'chunk.js').text).toContain(
1006+
'"Auf Wiedersehen"',
1007+
);
1008+
});
1009+
1010+
describe('maxConcurrency validation', () => {
1011+
it('throws when maxConcurrency is less than 1', () => {
1012+
expect(
1013+
() =>
1014+
new I18nInliner({
1015+
missingTranslation: 'warning',
1016+
maxConcurrency: 0,
1017+
}),
1018+
).toThrowError(
1019+
RangeError,
1020+
'options.maxConcurrency must be an integer greater than or equal to 1.',
1021+
);
1022+
1023+
expect(
1024+
() =>
1025+
new I18nInliner({
1026+
missingTranslation: 'warning',
1027+
maxConcurrency: -1,
1028+
}),
1029+
).toThrowError(
1030+
RangeError,
1031+
'options.maxConcurrency must be an integer greater than or equal to 1.',
1032+
);
1033+
});
1034+
1035+
it('throws when maxConcurrency is not an integer', () => {
1036+
expect(
1037+
() =>
1038+
new I18nInliner({
1039+
missingTranslation: 'warning',
1040+
maxConcurrency: 1.5,
1041+
}),
1042+
).toThrowError(
1043+
RangeError,
1044+
'options.maxConcurrency must be an integer greater than or equal to 1.',
1045+
);
1046+
1047+
expect(
1048+
() =>
1049+
new I18nInliner({
1050+
missingTranslation: 'warning',
1051+
maxConcurrency: NaN,
1052+
}),
1053+
).toThrowError(
1054+
RangeError,
1055+
'options.maxConcurrency must be an integer greater than or equal to 1.',
1056+
);
1057+
1058+
expect(
1059+
() =>
1060+
new I18nInliner({
1061+
missingTranslation: 'warning',
1062+
maxConcurrency: Infinity,
1063+
}),
1064+
).toThrowError(
1065+
RangeError,
1066+
'options.maxConcurrency must be an integer greater than or equal to 1.',
1067+
);
1068+
});
1069+
1070+
it('defaults to worker pool concurrency when maxConcurrency is omitted', async () => {
1071+
const maxThreadsSpy = spyOnProperty(
1072+
WorkerPool.prototype,
1073+
'maxThreads',
1074+
'get',
1075+
).and.callThrough();
1076+
1077+
inliner = new I18nInliner({
1078+
missingTranslation: 'warning',
1079+
});
1080+
1081+
const files = [browserFile('main.js', GREETING_SOURCE)];
1082+
const results = await inliner.inlineAll(files, [
1083+
{
1084+
locale: 'fr',
1085+
translation: { greeting: translationFor('Bonjour') },
1086+
},
1087+
]);
1088+
1089+
expect(results.size).toBe(1);
1090+
expect(findFile(results.get('fr')?.outputFiles ?? [], 'main.js').text).toContain('"Bonjour"');
1091+
expect(maxThreadsSpy).toHaveBeenCalled();
1092+
expect(maxThreadsSpy.calls.mostRecent().returnValue).toBeGreaterThanOrEqual(1);
1093+
});
1094+
});
9851095
});

0 commit comments

Comments
 (0)