Skip to content

Commit 8dae1af

Browse files
committed
refactor(@angular/build): optimize unmodified file and locale handling in i18n inliner
When AST analysis determines that a file has zero $localize call sites and zero locale insert sites, the worker returns a lightweight unmodified batch result immediately, skipping per-locale loops, translation dictionary queries, and sourcemap parsing. For files where transformations occur but a specific locale produces no text modifications, code and sourcemap strings are omitted from the worker result, avoiding redundant serialization over worker IPC. The main thread falls back directly to the existing BuildOutputFile contents and cloned sourcemaps.
1 parent 135662d commit 8dae1af

3 files changed

Lines changed: 170 additions & 34 deletions

File tree

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

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,25 @@ interface InlineFileBatchRequest {
7777
*/
7878
interface InlineLocaleResult {
7979
locale: string;
80-
code: string;
80+
code?: string;
8181
map?: string;
8282
messages: { type: 'error' | 'warning'; message: string }[];
8383
}
8484

8585
/**
8686
* The response returned from a batch file request.
8787
*/
88-
interface InlineFileBatchResult {
89-
file: string;
90-
results: InlineLocaleResult[];
91-
}
88+
type InlineFileBatchResult =
89+
| {
90+
file: string;
91+
unmodified: true;
92+
messages: { type: 'error' | 'warning'; message: string }[];
93+
}
94+
| {
95+
file: string;
96+
unmodified?: false;
97+
results: InlineLocaleResult[];
98+
};
9299

93100
// Extract the application files and common options used for inline requests from the Worker context
94101
const { files, missingTranslation } = (workerData || {}) as {
@@ -204,6 +211,18 @@ export async function inlineFileBatch(
204211

205212
const { code, metadata } = await loadFileData(request.filename, !request.ephemeral);
206213

214+
// Fast path: file has no $localize call sites or locale insert sites
215+
if (metadata.callSites.length === 0 && metadata.localeInsertSites.length === 0) {
216+
return {
217+
file: request.filename,
218+
unmodified: true,
219+
messages: (metadata.diagnostics ?? []).map((message) => ({
220+
type: 'error' as const,
221+
message,
222+
})),
223+
};
224+
}
225+
207226
// Parse the sourcemap once for the entire batch.
208227
// It will naturally be garbage-collected after this batch action returns.
209228
const rawMap = await files.get(request.filename + '.map')?.text();
@@ -254,7 +273,7 @@ export async function inlineCode(request: InlineCodeRequest) {
254273
);
255274

256275
return {
257-
output: result.code,
276+
output: result.code ?? request.code,
258277
messages: result.diagnostics.messages,
259278
};
260279
}
@@ -499,9 +518,17 @@ async function inlineLocalize(
499518
magicString.overwrite(callSite.start, callSite.end, replacement);
500519
}
501520

521+
if (!magicString.hasChanged()) {
522+
return {
523+
code: undefined,
524+
map: undefined,
525+
diagnostics,
526+
};
527+
}
528+
502529
const outputCode = magicString.toString();
503530
let outputMap;
504-
if (map && magicString.hasChanged()) {
531+
if (map) {
505532
// A decoded map is generated here rather than an encoded one because remapping decodes its
506533
// inputs. Encoding the mappings only for remapping to immediately decode them again doubles
507534
// the peak memory of the largest structure involved in inlining a file.

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

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export interface LocaleInlineResult {
9696
*/
9797
interface TransformedFileResult {
9898
file: string;
99-
code: string;
99+
code?: string;
100100
map?: string;
101101
messages: { type: 'error' | 'warning'; message: string }[];
102102
}
@@ -362,17 +362,23 @@ export class I18nInliner {
362362

363363
if (fileResults) {
364364
for (const filename of filenames) {
365+
const originalFile = this.#localizeFiles.get(filename);
366+
assert(originalFile !== undefined, 'Localize file must exist: ' + filename);
367+
365368
const fileResult = fileResults.get(filename);
366369
if (!fileResult) {
367370
continue;
368371
}
369372

370-
const type = this.#localizeFiles.get(filename)?.type;
371-
assert(type !== undefined, 'localized file should always have a type: ' + filename);
373+
const type = originalFile.type;
374+
const code = fileResult.code ?? originalFile.contents;
375+
outputFiles.push(createOutputFile(filename, code, type));
372376

373-
outputFiles.push(createOutputFile(filename, fileResult.code, type));
374-
if (fileResult.map) {
377+
const originalMap = this.#localizeFiles.get(filename + '.map');
378+
if (fileResult.map !== undefined) {
375379
outputFiles.push(createOutputFile(filename + '.map', fileResult.map, type));
380+
} else if (originalMap !== undefined) {
381+
outputFiles.push(originalMap.clone());
376382
}
377383

378384
for (const message of fileResult.messages) {
@@ -427,30 +433,54 @@ export class I18nInliner {
427433
activeLocales,
428434
},
429435
{ name: 'inlineFileBatch' },
430-
)) as {
431-
file: string;
432-
results: Array<TransformedFileResult & { locale: string }>;
433-
};
434-
435-
const cachePromises: Promise<unknown>[] = [];
436-
for (const res of batchResult.results) {
437-
const matchingEntry = batchEntries.find((e) => e.locale === res.locale);
438-
const cacheKey = matchingEntry?.cacheKey;
439-
440-
if (this.#cache && cacheKey) {
441-
cachePromises.push(
442-
this.#cache.put(cacheKey, {
443-
file: filename,
444-
code: res.code,
445-
map: res.map,
446-
messages: res.messages,
447-
}),
448-
);
436+
)) as
437+
| {
438+
file: string;
439+
unmodified: true;
440+
messages: { type: 'error' | 'warning'; message: string }[];
441+
}
442+
| {
443+
file: string;
444+
unmodified?: false;
445+
results: Array<TransformedFileResult & { locale: string }>;
446+
};
447+
448+
if (batchResult.unmodified) {
449+
const unmodifiedResult: TransformedFileResult = {
450+
file: filename,
451+
messages: batchResult.messages,
452+
};
453+
454+
const cachePromises: Promise<unknown>[] = [];
455+
for (const { locale, cacheKey } of batchEntries) {
456+
fileResultsByLocale.get(locale)?.set(filename, unmodifiedResult);
457+
458+
if (this.#cache && cacheKey) {
459+
cachePromises.push(this.#cache.put(cacheKey, unmodifiedResult));
460+
}
449461
}
450-
451-
fileResultsByLocale.get(res.locale)?.set(filename, res);
462+
await Promise.allSettled(cachePromises);
463+
} else {
464+
const cachePromises: Promise<unknown>[] = [];
465+
for (const res of batchResult.results) {
466+
const matchingEntry = batchEntries.find((e) => e.locale === res.locale);
467+
const cacheKey = matchingEntry?.cacheKey;
468+
469+
if (this.#cache && cacheKey) {
470+
cachePromises.push(
471+
this.#cache.put(cacheKey, {
472+
file: filename,
473+
code: res.code,
474+
map: res.map,
475+
messages: res.messages,
476+
}),
477+
);
478+
}
479+
480+
fileResultsByLocale.get(res.locale)?.set(filename, res);
481+
}
482+
await Promise.allSettled(cachePromises);
452483
}
453-
await Promise.allSettled(cachePromises);
454484
})();
455485

456486
workerTasks.push(task);

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,4 +644,83 @@ describe('I18nInliner', () => {
644644
const iifeMatches = frPolyfills.match(/function\s*\(global/g);
645645
expect(iifeMatches?.length).toBe(1);
646646
});
647+
648+
it('preserves code and sourcemaps when a file contains the $localize keyword only in comments', async () => {
649+
const source = '// $localize keyword in comment\nexport const message = "hello world";\n';
650+
const { code, map } = await transform(source, {
651+
sourcefile: 'comment-only.ts',
652+
loader: 'ts',
653+
sourcemap: 'external',
654+
});
655+
656+
const localeInliner = createInliner([
657+
browserFile('main.js', code),
658+
browserFile('main.js.map', map),
659+
]);
660+
661+
const results = await localeInliner.inlineAll([
662+
{ locale: 'fr', translation: { greeting: translationFor('Bonjour') } },
663+
{ locale: 'de', translation: { greeting: translationFor('Hallo') } },
664+
]);
665+
666+
expect(results.size).toBe(2);
667+
668+
for (const locale of ['fr', 'de'] as const) {
669+
const localeResult = results.get(locale);
670+
expect(localeResult).toBeDefined();
671+
expect(localeResult?.errors).toEqual([]);
672+
expect(localeResult?.warnings).toEqual([]);
673+
674+
const mainJs = findFile(localeResult?.outputFiles ?? [], 'main.js');
675+
expect(mainJs.text).toBe(code);
676+
677+
const mainMap = findFile(localeResult?.outputFiles ?? [], 'main.js.map');
678+
expect(mainMap.text).toBe(map);
679+
}
680+
});
681+
682+
it('correctly handles persistent caching for unmodified files', async () => {
683+
const cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'i18n-unmodified-cache-test-'));
684+
const source = '// $localize comment only\nexport const value = 123;\n';
685+
686+
try {
687+
const inliner1 = new I18nInliner({
688+
missingTranslation: 'error',
689+
outputFiles: [browserFile('main.js', source)],
690+
persistentCachePath: cacheDir,
691+
});
692+
693+
const results1 = await inliner1.inlineAll([
694+
{
695+
locale: 'fr',
696+
translation: { greeting: translationFor('Bonjour') },
697+
translationIntegrity: 'fr-1',
698+
},
699+
]);
700+
701+
expect(results1.get('fr')?.errors).toEqual([]);
702+
expect(findFile(results1.get('fr')?.outputFiles ?? [], 'main.js').text).toBe(source);
703+
await inliner1.close();
704+
705+
const inliner2 = new I18nInliner({
706+
missingTranslation: 'error',
707+
outputFiles: [browserFile('main.js', source)],
708+
persistentCachePath: cacheDir,
709+
});
710+
711+
const results2 = await inliner2.inlineAll([
712+
{
713+
locale: 'fr',
714+
translation: { greeting: translationFor('Bonjour') },
715+
translationIntegrity: 'fr-1',
716+
},
717+
]);
718+
719+
expect(results2.get('fr')?.errors).toEqual([]);
720+
expect(findFile(results2.get('fr')?.outputFiles ?? [], 'main.js').text).toBe(source);
721+
await inliner2.close();
722+
} finally {
723+
await fs.rm(cacheDir, { recursive: true, force: true });
724+
}
725+
});
647726
});

0 commit comments

Comments
 (0)