From ec243cab9135898b6c57c281bfcc5ddc5a2507be Mon Sep 17 00:00:00 2001 From: igord Date: Wed, 14 May 2025 15:31:52 +0200 Subject: [PATCH] fix(@angular-devkit/build-angular): respect i18nDuplicateTranslation option when duplicates exist Running the extract-i18n command will always log duplicate i18n message IDs as warnings, regardless of the value of the i18nDuplicateTranslation option. With this fix, command will log duplication errors and exit with code 1 Fixes #23635 --- goldens/public-api/angular/build/index.api.md | 1 + .../angular_devkit/build_angular/index.api.md | 1 + .../src/builders/extract-i18n/builder.ts | 13 +++++++-- .../src/builders/extract-i18n/options.ts | 10 +++++-- .../src/builders/extract-i18n/schema.json | 5 ++++ .../angular/build/src/utils/i18n-options.ts | 2 -- .../src/builders/extract-i18n/builder.ts | 13 +++++++-- .../src/builders/extract-i18n/options.ts | 10 +++++-- .../src/builders/extract-i18n/schema.json | 5 ++++ .../src/builders/extract-i18n/works_spec.ts | 28 +++++++++++++++++++ 10 files changed, 76 insertions(+), 12 deletions(-) diff --git a/goldens/public-api/angular/build/index.api.md b/goldens/public-api/angular/build/index.api.md index 31576f664e36..aa9ac693864e 100644 --- a/goldens/public-api/angular/build/index.api.md +++ b/goldens/public-api/angular/build/index.api.md @@ -158,6 +158,7 @@ export function executeNgPackagrBuilder(options: NgPackagrBuilderOptions, contex export type ExtractI18nBuilderOptions = { buildTarget?: string; format?: Format; + i18nDuplicateTranslation?: I18NDuplicateTranslation; outFile?: string; outputPath?: string; progress?: boolean; diff --git a/goldens/public-api/angular_devkit/build_angular/index.api.md b/goldens/public-api/angular_devkit/build_angular/index.api.md index 9208d1ad56a1..cb46b4458351 100644 --- a/goldens/public-api/angular_devkit/build_angular/index.api.md +++ b/goldens/public-api/angular_devkit/build_angular/index.api.md @@ -191,6 +191,7 @@ export type ExecutionTransformer = (input: T) => T | Promise; export type ExtractI18nBuilderOptions = { buildTarget?: string; format?: Format; + i18nDuplicateTranslation?: I18NDuplicateTranslation; outFile?: string; outputPath?: string; progress?: boolean; diff --git a/packages/angular/build/src/builders/extract-i18n/builder.ts b/packages/angular/build/src/builders/extract-i18n/builder.ts index 5e44c31fa516..15b0156f749b 100644 --- a/packages/angular/build/src/builders/extract-i18n/builder.ts +++ b/packages/angular/build/src/builders/extract-i18n/builder.ts @@ -90,16 +90,23 @@ export async function execute( return path.relative(from, to); }, }; + const duplicateTranslationBehavior = normalizedOptions.i18nOptions.duplicateTranslationBehavior; const diagnostics = checkDuplicateMessages( // eslint-disable-next-line @typescript-eslint/no-explicit-any checkFileSystem as any, extractionResult.messages, - normalizedOptions.i18nOptions.i18nDuplicateTranslation || 'warning', + duplicateTranslationBehavior, // eslint-disable-next-line @typescript-eslint/no-explicit-any extractionResult.basePath as any, ); - if (diagnostics.messages.length > 0) { - context.logger.warn(diagnostics.formatDiagnostics('')); + if (diagnostics.messages.length > 0 && duplicateTranslationBehavior !== 'ignore') { + if (duplicateTranslationBehavior === 'error') { + context.logger.error(`Extraction Failed: ${diagnostics.formatDiagnostics('')}`); + + return { success: false }; + } else { + context.logger.warn(diagnostics.formatDiagnostics('')); + } } // Serialize all extracted messages diff --git a/packages/angular/build/src/builders/extract-i18n/options.ts b/packages/angular/build/src/builders/extract-i18n/options.ts index 24be86ee7c8f..2904a466bd60 100644 --- a/packages/angular/build/src/builders/extract-i18n/options.ts +++ b/packages/angular/build/src/builders/extract-i18n/options.ts @@ -6,10 +6,11 @@ * found in the LICENSE file at https://angular.dev/license */ +import { type DiagnosticHandlingStrategy } from '@angular/localize/tools'; import { BuilderContext, targetFromTargetString } from '@angular-devkit/architect'; import { fail } from 'node:assert'; import path from 'node:path'; -import { createI18nOptions } from '../../utils/i18n-options'; +import { type I18nOptions, createI18nOptions } from '../../utils/i18n-options'; import { Schema as ExtractI18nOptions, Format } from './schema'; export type NormalizedExtractI18nOptions = Awaited>; @@ -36,7 +37,12 @@ export async function normalizeOptions( // Target specifier defaults to the current project's build target with no specified configuration const buildTargetSpecifier = options.buildTarget ?? ':'; const buildTarget = targetFromTargetString(buildTargetSpecifier, projectName, 'build'); - const i18nOptions = createI18nOptions(projectMetadata, /** inline */ false, context.logger); + const i18nOptions: I18nOptions & { + duplicateTranslationBehavior: DiagnosticHandlingStrategy; + } = { + ...createI18nOptions(projectMetadata, /** inline */ false, context.logger), + duplicateTranslationBehavior: options.i18nDuplicateTranslation || 'warning', + }; // Normalize xliff format extensions let format = options.format; diff --git a/packages/angular/build/src/builders/extract-i18n/schema.json b/packages/angular/build/src/builders/extract-i18n/schema.json index 9ab939b0e938..08a118ad7d5e 100644 --- a/packages/angular/build/src/builders/extract-i18n/schema.json +++ b/packages/angular/build/src/builders/extract-i18n/schema.json @@ -27,6 +27,11 @@ "outFile": { "type": "string", "description": "Name of the file to output." + }, + "i18nDuplicateTranslation": { + "type": "string", + "description": "How to handle duplicate translations.", + "enum": ["error", "warning", "ignore"] } }, "additionalProperties": false diff --git a/packages/angular/build/src/utils/i18n-options.ts b/packages/angular/build/src/utils/i18n-options.ts index 53e5aca4d540..822683bef03d 100644 --- a/packages/angular/build/src/utils/i18n-options.ts +++ b/packages/angular/build/src/utils/i18n-options.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.dev/license */ -import { DiagnosticHandlingStrategy } from '@angular/localize/tools'; import path from 'node:path'; import type { TranslationLoader } from './load-translations'; @@ -29,7 +28,6 @@ export interface I18nOptions { flatOutput?: boolean; readonly shouldInline: boolean; hasDefinedSourceLocale?: boolean; - i18nDuplicateTranslation?: DiagnosticHandlingStrategy; } function normalizeTranslationFileOption( diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts index bb6750189a1a..2d22fda36e96 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/builder.ts @@ -110,16 +110,23 @@ export async function execute( return path.relative(from, to); }, }; + const duplicateTranslationBehavior = normalizedOptions.i18nOptions.duplicateTranslationBehavior; const diagnostics = checkDuplicateMessages( // eslint-disable-next-line @typescript-eslint/no-explicit-any checkFileSystem as any, extractionResult.messages, - 'warning', + duplicateTranslationBehavior, // eslint-disable-next-line @typescript-eslint/no-explicit-any extractionResult.basePath as any, ); - if (diagnostics.messages.length > 0) { - context.logger.warn(diagnostics.formatDiagnostics('')); + if (diagnostics.messages.length > 0 && duplicateTranslationBehavior !== 'ignore') { + if (duplicateTranslationBehavior === 'error') { + context.logger.error(`Extraction Failed: ${diagnostics.formatDiagnostics('')}`); + + return { success: false }; + } else { + context.logger.warn(diagnostics.formatDiagnostics('')); + } } // Serialize all extracted messages diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/options.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/options.ts index 492909da14f0..0aaf255e7d47 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/options.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/options.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.dev/license */ -import { createI18nOptions } from '@angular/build/private'; +import { type I18nOptions, createI18nOptions } from '@angular/build/private'; +import { type DiagnosticHandlingStrategy } from '@angular/localize/tools'; import { BuilderContext, targetFromTargetString } from '@angular-devkit/architect'; import { fail } from 'node:assert'; import path from 'node:path'; @@ -36,7 +37,12 @@ export async function normalizeOptions( // Target specifier defaults to the current project's build target with no specified configuration const buildTargetSpecifier = options.buildTarget ?? ':'; const buildTarget = targetFromTargetString(buildTargetSpecifier, projectName, 'build'); - const i18nOptions = createI18nOptions(projectMetadata, /** inline */ false, context.logger); + const i18nOptions: I18nOptions & { + duplicateTranslationBehavior: DiagnosticHandlingStrategy; + } = { + ...createI18nOptions(projectMetadata, /** inline */ false, context.logger), + duplicateTranslationBehavior: options.i18nDuplicateTranslation || 'warning', + }; // Normalize xliff format extensions let format = options.format; diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/schema.json b/packages/angular_devkit/build_angular/src/builders/extract-i18n/schema.json index 9ab939b0e938..08a118ad7d5e 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/schema.json +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/schema.json @@ -27,6 +27,11 @@ "outFile": { "type": "string", "description": "Name of the file to output." + }, + "i18nDuplicateTranslation": { + "type": "string", + "description": "How to handle duplicate translations.", + "enum": ["error", "warning", "ignore"] } }, "additionalProperties": false diff --git a/packages/angular_devkit/build_angular/src/builders/extract-i18n/works_spec.ts b/packages/angular_devkit/build_angular/src/builders/extract-i18n/works_spec.ts index b0705dfee1b8..1f29fb96a581 100644 --- a/packages/angular_devkit/build_angular/src/builders/extract-i18n/works_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/extract-i18n/works_spec.ts @@ -147,6 +147,34 @@ describe('Extract i18n Target', () => { expect(fullLog).toContain('Duplicate messages with id'); }); + it('issues errors for duplicate message identifiers', async () => { + host.appendToFile( + 'src/app/app.component.ts', + 'const c = $localize`:@@message-2:message contents`; const d = $localize`:@@message-2:different message contents`;', + ); + + const logger = new logging.Logger(''); + const logs: string[] = []; + logger.subscribe((e) => logs.push(e.message)); + + const run = await architect.scheduleTarget( + extractI18nTargetSpec, + { + i18nDuplicateTranslation: 'error', + }, + { logger }, + ); + await expectAsync(run.result).toBeResolvedTo(jasmine.objectContaining({ success: false })); + + await run.stop(); + + expect(host.scopedSync().exists(extractionFile)).toBe(false); + + const fullLog = logs.join(); + expect(fullLog).toContain('Duplicate messages with id'); + expect(fullLog).toContain('Extraction Failed'); + }); + it('ignores inline styles', async () => { host.appendToFile('src/app/app.component.html', '

i18n test

'); host.replaceInFile('src/app/app.component.ts', 'styleUrls', 'styles');