From e3a9d389f3b24b7e33bdeabc774e555b90b0bb96 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:35:53 -0400 Subject: [PATCH] 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. --- .../src/tools/esbuild/i18n-inliner-worker.ts | 41 +++++++-- .../build/src/tools/esbuild/i18n-inliner.ts | 87 +++++++++++++------ .../src/tools/esbuild/i18n-inliner_spec.ts | 79 +++++++++++++++++ 3 files changed, 173 insertions(+), 34 deletions(-) diff --git a/packages/angular/build/src/tools/esbuild/i18n-inliner-worker.ts b/packages/angular/build/src/tools/esbuild/i18n-inliner-worker.ts index d6a56ad10d74..e7b11cc53225 100644 --- a/packages/angular/build/src/tools/esbuild/i18n-inliner-worker.ts +++ b/packages/angular/build/src/tools/esbuild/i18n-inliner-worker.ts @@ -77,7 +77,7 @@ interface InlineFileBatchRequest { */ interface InlineLocaleResult { locale: string; - code: string; + code?: string; map?: string; messages: { type: 'error' | 'warning'; message: string }[]; } @@ -85,10 +85,17 @@ interface InlineLocaleResult { /** * The response returned from a batch file request. */ -interface InlineFileBatchResult { - file: string; - results: InlineLocaleResult[]; -} +type InlineFileBatchResult = + | { + file: string; + unmodified: true; + messages: { type: 'error' | 'warning'; message: string }[]; + } + | { + file: string; + unmodified?: false; + results: InlineLocaleResult[]; + }; // Extract the application files and common options used for inline requests from the Worker context const { files, missingTranslation } = (workerData || {}) as { @@ -204,6 +211,18 @@ export async function inlineFileBatch( const { code, metadata } = await loadFileData(request.filename, !request.ephemeral); + // Fast path: file has no $localize call sites or locale insert sites + if (metadata.callSites.length === 0 && metadata.localeInsertSites.length === 0) { + return { + file: request.filename, + unmodified: true, + messages: (metadata.diagnostics ?? []).map((message) => ({ + type: 'error' as const, + message, + })), + }; + } + // Parse the sourcemap once for the entire batch. // It will naturally be garbage-collected after this batch action returns. const rawMap = await files.get(request.filename + '.map')?.text(); @@ -254,7 +273,7 @@ export async function inlineCode(request: InlineCodeRequest) { ); return { - output: result.code, + output: result.code ?? request.code, messages: result.diagnostics.messages, }; } @@ -499,9 +518,17 @@ async function inlineLocalize( magicString.overwrite(callSite.start, callSite.end, replacement); } + if (!magicString.hasChanged()) { + return { + code: undefined, + map: undefined, + diagnostics, + }; + } + const outputCode = magicString.toString(); let outputMap; - if (map && magicString.hasChanged()) { + if (map) { // A decoded map is generated here rather than an encoded one because remapping decodes its // inputs. Encoding the mappings only for remapping to immediately decode them again doubles // the peak memory of the largest structure involved in inlining a file. diff --git a/packages/angular/build/src/tools/esbuild/i18n-inliner.ts b/packages/angular/build/src/tools/esbuild/i18n-inliner.ts index 33b570a46017..5cca10c534c9 100644 --- a/packages/angular/build/src/tools/esbuild/i18n-inliner.ts +++ b/packages/angular/build/src/tools/esbuild/i18n-inliner.ts @@ -96,7 +96,7 @@ export interface LocaleInlineResult { */ interface TransformedFileResult { file: string; - code: string; + code?: string; map?: string; messages: { type: 'error' | 'warning'; message: string }[]; } @@ -362,17 +362,26 @@ export class I18nInliner { if (fileResults) { for (const filename of filenames) { + const originalFile = this.#localizeFiles.get(filename); + assert(originalFile !== undefined, 'Localize file must exist: ' + filename); + const fileResult = fileResults.get(filename); if (!fileResult) { continue; } - const type = this.#localizeFiles.get(filename)?.type; - assert(type !== undefined, 'localized file should always have a type: ' + filename); + const type = originalFile.type; + if (fileResult.code != undefined) { + outputFiles.push(createOutputFile(filename, fileResult.code, type)); + } else { + outputFiles.push(originalFile.clone()); + } - outputFiles.push(createOutputFile(filename, fileResult.code, type)); - if (fileResult.map) { + const originalMap = this.#localizeFiles.get(filename + '.map'); + if (fileResult.map !== undefined) { outputFiles.push(createOutputFile(filename + '.map', fileResult.map, type)); + } else if (originalMap !== undefined) { + outputFiles.push(originalMap.clone()); } for (const message of fileResult.messages) { @@ -427,30 +436,54 @@ export class I18nInliner { activeLocales, }, { name: 'inlineFileBatch' }, - )) as { - file: string; - results: Array; - }; - - const cachePromises: Promise[] = []; - for (const res of batchResult.results) { - const matchingEntry = batchEntries.find((e) => e.locale === res.locale); - const cacheKey = matchingEntry?.cacheKey; - - if (this.#cache && cacheKey) { - cachePromises.push( - this.#cache.put(cacheKey, { - file: filename, - code: res.code, - map: res.map, - messages: res.messages, - }), - ); + )) as + | { + file: string; + unmodified: true; + messages: { type: 'error' | 'warning'; message: string }[]; + } + | { + file: string; + unmodified?: false; + results: Array; + }; + + if (batchResult.unmodified) { + const unmodifiedResult: TransformedFileResult = { + file: filename, + messages: batchResult.messages, + }; + + const cachePromises: Promise[] = []; + for (const { locale, cacheKey } of batchEntries) { + fileResultsByLocale.get(locale)?.set(filename, unmodifiedResult); + + if (this.#cache && cacheKey) { + cachePromises.push(this.#cache.put(cacheKey, unmodifiedResult)); + } } - - fileResultsByLocale.get(res.locale)?.set(filename, res); + await Promise.allSettled(cachePromises); + } else { + const cachePromises: Promise[] = []; + for (const res of batchResult.results) { + const matchingEntry = batchEntries.find((e) => e.locale === res.locale); + const cacheKey = matchingEntry?.cacheKey; + + if (this.#cache && cacheKey) { + cachePromises.push( + this.#cache.put(cacheKey, { + file: filename, + code: res.code, + map: res.map, + messages: res.messages, + }), + ); + } + + fileResultsByLocale.get(res.locale)?.set(filename, res); + } + await Promise.allSettled(cachePromises); } - await Promise.allSettled(cachePromises); })(); workerTasks.push(task); 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 acceab7513b5..d2f5559f5bb2 100644 --- a/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts +++ b/packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts @@ -644,4 +644,83 @@ describe('I18nInliner', () => { const iifeMatches = frPolyfills.match(/function\s*\(global/g); expect(iifeMatches?.length).toBe(1); }); + + it('preserves code and sourcemaps when a file contains the $localize keyword only in comments', async () => { + const source = '// $localize keyword in comment\nexport const message = "hello world";\n'; + const { code, map } = await transform(source, { + sourcefile: 'comment-only.ts', + loader: 'ts', + sourcemap: 'external', + }); + + const localeInliner = createInliner([ + browserFile('main.js', code), + browserFile('main.js.map', map), + ]); + + const results = await localeInliner.inlineAll([ + { locale: 'fr', translation: { greeting: translationFor('Bonjour') } }, + { locale: 'de', translation: { greeting: translationFor('Hallo') } }, + ]); + + expect(results.size).toBe(2); + + for (const locale of ['fr', 'de'] as const) { + const localeResult = results.get(locale); + expect(localeResult).toBeDefined(); + expect(localeResult?.errors).toEqual([]); + expect(localeResult?.warnings).toEqual([]); + + const mainJs = findFile(localeResult?.outputFiles ?? [], 'main.js'); + expect(mainJs.text).toBe(code); + + const mainMap = findFile(localeResult?.outputFiles ?? [], 'main.js.map'); + expect(mainMap.text).toBe(map); + } + }); + + it('correctly handles persistent caching for unmodified files', async () => { + const cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'i18n-unmodified-cache-test-')); + const source = '// $localize comment only\nexport const value = 123;\n'; + + try { + const inliner1 = new I18nInliner({ + missingTranslation: 'error', + outputFiles: [browserFile('main.js', source)], + persistentCachePath: cacheDir, + }); + + const results1 = await inliner1.inlineAll([ + { + locale: 'fr', + translation: { greeting: translationFor('Bonjour') }, + translationIntegrity: 'fr-1', + }, + ]); + + expect(results1.get('fr')?.errors).toEqual([]); + expect(findFile(results1.get('fr')?.outputFiles ?? [], 'main.js').text).toBe(source); + await inliner1.close(); + + const inliner2 = new I18nInliner({ + missingTranslation: 'error', + outputFiles: [browserFile('main.js', source)], + persistentCachePath: cacheDir, + }); + + const results2 = await inliner2.inlineAll([ + { + locale: 'fr', + translation: { greeting: translationFor('Bonjour') }, + translationIntegrity: 'fr-1', + }, + ]); + + expect(results2.get('fr')?.errors).toEqual([]); + expect(findFile(results2.get('fr')?.outputFiles ?? [], 'main.js').text).toBe(source); + await inliner2.close(); + } finally { + await fs.rm(cacheDir, { recursive: true, force: true }); + } + }); });