Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions packages/angular/build/src/tools/esbuild/i18n-inliner-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type { Node } from '@oxc-project/types';
import { MagicString } from 'magic-string';
import assert from 'node:assert';
import { deserialize } from 'node:v8';
import { workerData } from 'node:worker_threads';
import { parseSync } from 'oxc-parser';
import { traversePostOrder } from '../oxc/traversal';
import { loadLocaleData } from './i18n-locale-plugin';
Expand All @@ -21,7 +20,7 @@ import { createSharedTranslationProxy } from './i18n-translation-reader';
/**
* The options passed to the inliner for each code request
*/
interface InlineCodeRequest {
export interface InlineCodeRequest {
/**
* The code that should be processed.
*/
Expand All @@ -43,12 +42,25 @@ interface InlineCodeRequest {
* the Worker by reference instead of being copied into it for every request.
*/
translation?: Blob | SharedArrayBuffer;

/**
* How to handle missing translations.
*/
missingTranslation?: 'error' | 'warning' | 'ignore';
}

/**
* The response returned from a code request.
*/
export interface InlineCodeResult {
output: string;
messages: { type: 'error' | 'warning'; message: string }[];
}

/**
* The options passed to the inliner for a batch file request
*/
interface InlineFileBatchRequest {
export interface InlineFileBatchRequest {
/**
* The filename that should be processed.
*/
Expand All @@ -69,6 +81,11 @@ interface InlineFileBatchRequest {
*/
locales: ReadonlyMap<string, Blob | SharedArrayBuffer | undefined>;

/**
* How to handle missing translations.
*/
missingTranslation?: 'error' | 'warning' | 'ignore';

/**
* Whether the file data should be treated as ephemeral and not cached long-term in the Worker.
* Typically true when all remaining locales for the file are processed in a single batch.
Expand All @@ -91,7 +108,7 @@ interface InlineFileBatchRequest {
/**
* The result for a single locale within a batch file request.
*/
interface InlineLocaleResult {
export interface InlineLocaleResult {
locale: string;
code?: string;
map?: string;
Expand All @@ -101,7 +118,7 @@ interface InlineLocaleResult {
/**
* The response returned from a batch file request.
*/
type InlineFileBatchResult =
export type InlineFileBatchResult =
| {
file: string;
unmodified: true;
Expand All @@ -113,11 +130,6 @@ type InlineFileBatchResult =
results: InlineLocaleResult[];
};

// Extract common options used for inline requests from the Worker context
const { missingTranslation } = (workerData || {}) as {
missingTranslation: 'error' | 'warning' | 'ignore';
};

/**
* Cached file data including code and extracted localization metadata.
*/
Expand Down Expand Up @@ -268,6 +280,7 @@ export async function inlineFileBatch(
locale,
await loadTranslation(locale, translation),
request.filename,
request.missingTranslation,
);

return {
Expand All @@ -292,7 +305,7 @@ export async function inlineFileBatch(
* @param request An InlineRequest object representing the options for inlining
* @returns An object containing the inlined code.
*/
export async function inlineCode(request: InlineCodeRequest) {
export async function inlineCode(request: InlineCodeRequest): Promise<InlineCodeResult> {
const metadata = extractLocalizeMetadata(request.filename, request.code);
const result = await inlineLocalize(
request.code,
Expand All @@ -301,6 +314,7 @@ export async function inlineCode(request: InlineCodeRequest) {
request.locale,
await loadTranslation(request.locale, request.translation),
request.filename,
request.missingTranslation,
);

return {
Expand Down Expand Up @@ -440,6 +454,7 @@ function escapeTemplatePart(part: string): string {
* @param locale The target locale identifier.
* @param translation The translation messages dictionary, or undefined for untranslated locale.
* @param filename The name of the file being transformed.
* @param missingTranslation How to handle missing translations.
* @returns The transformed code, optional remapped source map, and diagnostics.
*/
async function inlineLocalize(
Expand All @@ -449,6 +464,7 @@ async function inlineLocalize(
locale: string,
translation: Record<string, ɵParsedTranslation> | undefined,
filename: string,
missingTranslation: 'error' | 'warning' | 'ignore' = 'warning',
) {
const magicString = new MagicString(code);
const { Diagnostics, translate } = await loadLocalizeTools();
Expand Down
112 changes: 61 additions & 51 deletions packages/angular/build/src/tools/esbuild/i18n-inliner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,36 @@

import type { ɵParsedTranslation } from '@angular/localize';
import assert from 'node:assert';
import { createRequire } from 'node:module';
import { extname, join } from 'node:path';
import { serialize } from 'node:v8';
import { calculateHash, createContentHash, initializeHash } from '../../utils/hash';
import { WorkerPool } from '../../utils/worker-pool';
import { type BuildOutputFile, BuildOutputFileType, createOutputFile } from './bundler-files';
import { type Cache, type PersistentCacheStore, createPersistentCacheStore } from './cache';
import type {
InlineCodeRequest,
InlineCodeResult,
InlineFileBatchRequest,
InlineFileBatchResult,
} from './i18n-inliner-worker';
import { encodeTranslationToBuffer } from './i18n-translation-encoder';

// TODO: Convert to import.meta usage during ESM transition
const localRequire = createRequire(__filename);
const INLINER_WORKER_PATH = localRequire.resolve('./i18n-inliner-worker');

interface WorkerTaskMap {
inlineFileBatch: {
request: InlineFileBatchRequest;
result: InlineFileBatchResult;
};
inlineCode: {
request: InlineCodeRequest;
result: InlineCodeResult;
};
}

/**
* A keyword used to indicate if a JavaScript file may require inlining of translations.
* This keyword is used to avoid processing files that would not otherwise need i18n processing.
Expand Down Expand Up @@ -169,15 +191,8 @@ export class I18nInliner {
private readonly options: I18nInlinerOptions,
maxThreads?: number,
) {
const { missingTranslation } = options;

this.#workerPool = new WorkerPool({
filename: require.resolve('./i18n-inliner-worker'),
maxThreads,
// Extract options to ensure only the named options are serialized and sent to the worker
workerData: {
missingTranslation,
},
});
}

Expand Down Expand Up @@ -492,28 +507,16 @@ export class I18nInliner {
for (let i = 0; i < entries.length; i += localesPerBatch) {
const batchEntries = entries.slice(i, i + localesPerBatch);
const task = (async () => {
const batchResult = (await this.#workerPool.run(
{
filename,
code: codeBlob,
map: mapBlob,
locales: new Map(batchEntries.map((e) => [e.locale, e.translation])),
ephemeral,
activeLocales,
generation,
},
{ name: 'inlineFileBatch' },
)) as
| {
file: string;
unmodified: true;
messages: { type: 'error' | 'warning'; message: string }[];
}
| {
file: string;
unmodified?: false;
results: Array<TransformedFileResult & { locale: string }>;
};
const batchResult = await this.#runWorkerTask('inlineFileBatch', {
filename,
code: codeBlob,
map: mapBlob,
locales: new Map(batchEntries.map((e) => [e.locale, e.translation])),
missingTranslation: this.options.missingTranslation,
ephemeral,
activeLocales,
generation,
});

if (batchResult.unmodified) {
const unmodifiedResult: TransformedFileResult = {
Expand All @@ -535,19 +538,18 @@ export class I18nInliner {
for (const res of batchResult.results) {
const matchingEntry = batchEntries.find((e) => e.locale === res.locale);
const cacheKey = matchingEntry?.cacheKey;
const fileResult: TransformedFileResult = {
file: filename,
code: res.code,
map: res.map,
messages: res.messages,
};

if (this.#transformedFileCache && cacheKey) {
cachePromises.push(
this.#transformedFileCache.put(cacheKey, {
file: filename,
code: res.code,
map: res.map,
messages: res.messages,
}),
);
cachePromises.push(this.#transformedFileCache.put(cacheKey, fileResult));
}

fileResultsByLocale.get(res.locale)?.set(filename, res);
fileResultsByLocale.get(res.locale)?.set(filename, fileResult);
}
await Promise.allSettled(cachePromises);
}
Expand All @@ -560,6 +562,16 @@ export class I18nInliner {
await Promise.all(workerTasks);
}

#runWorkerTask<T extends keyof WorkerTaskMap>(
name: T,
request: WorkerTaskMap[T]['request'],
): Promise<WorkerTaskMap[T]['result']> {
return this.#workerPool.run(request, {
filename: INLINER_WORKER_PATH,
name,
}) as Promise<WorkerTaskMap[T]['result']>;
}
Comment thread
clydin marked this conversation as resolved.

/**
* Performs inlining of translations for the provided locale and translations.
*
Expand Down Expand Up @@ -599,19 +611,17 @@ export class I18nInliner {
};
}

const { output, messages } = await this.#workerPool.run(
{
code: templateCode,
filename: templateId,
locale,
translation: await serializeTranslation(
translation,
translationIntegrity,
this.#translationCache,
),
},
{ name: 'inlineCode' },
);
const { output, messages } = await this.#runWorkerTask('inlineCode', {
code: templateCode,
filename: templateId,
locale,
missingTranslation: this.options.missingTranslation,
translation: await serializeTranslation(
translation,
translationIntegrity,
this.#translationCache,
),
});

const errors: string[] = [];
const warnings: string[] = [];
Expand Down
25 changes: 25 additions & 0 deletions packages/angular/build/src/tools/esbuild/i18n-inliner_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,31 @@ describe('I18nInliner', () => {
expect(findFile(outputFiles, 'main.js').text).toContain('"Hello"');
});

it('errors and retains the original message when missingTranslation is "error"', async () => {
const { outputFiles, errors, warnings } = await createInliner({
missingTranslation: 'error',
}).inlineForLocale([browserFile('main.js', GREETING_SOURCE)], 'fr', {
unrelated: translationFor('Sans rapport'),
});

expect(errors.length).toBe(1);
expect(errors[0]).toContain('greeting');
expect(warnings).toEqual([]);
expect(findFile(outputFiles, 'main.js').text).toContain('"Hello"');
});

it('ignores missing translations when missingTranslation is "ignore"', async () => {
const { outputFiles, errors, warnings } = await createInliner({
missingTranslation: 'ignore',
}).inlineForLocale([browserFile('main.js', GREETING_SOURCE)], 'fr', {
unrelated: translationFor('Sans rapport'),
});

expect(errors).toEqual([]);
expect(warnings).toEqual([]);
expect(findFile(outputFiles, 'main.js').text).toContain('"Hello"');
});

it('replaces the locale placeholder with the locale being inlined', async () => {
// The placeholder is only inlined for files that use `$localize`, which is where the build
// inserts it, so the message is present alongside it here.
Expand Down