angular-cli: extract-i18n does not respect the i18nDuplicateTranslation option

🐞 Bug report

Command (mark with an `x`)

new
build
serve
test
e2e
generate
add
update
lint
extract-i18n
run
config
help
version
doc

Is this a regression?

Description

Running the extract-i18n command will always log duplicate i18n message IDs as warnings, regardless of the value of the i18nDuplicateTranslation option in the build target.

🔬 Minimal Reproduction

This repository can be used to quickly reproduce this bug: https://github.com/reduckted/repro-extract-i18n-duplicates

Or using this archive: repro-extract-i18n-duplicates.zip

Run npm i
Run ng extract-i18n

The template app.component.html contains two i18n messages with the same ID but different text. The angular.json file has the i18nDuplicateTranslation option set to "error" in the browser builder.

The output is:

WARNINGS:
 - Duplicate messages with id "test":
   - "first" : src\app\app.component.html:1
   - "second" : src\app\app.component.html:2

🔥 Exception or Error

I expect that with i18nDuplicateTranslation set to "error", the duplicate message IDs will be logged as errors and that the process exits with a non-zero exit code.

Instead, the duplicate message IDs are logged as a warning, and the process exits with an exit code of zero.

🌍 Your Environment


> ng version

     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 14.1.0
Node: 16.12.0
Package Manager: npm 8.15.0
OS: win32 x64

Angular: 14.1.0
... animations, cli, common, compiler, compiler-cli, core, forms
... localize, platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1401.0
@angular-devkit/build-angular   14.1.0
@angular-devkit/core            14.1.0
@angular-devkit/schematics      14.1.0
@schematics/angular             14.1.0
rxjs                            7.5.6
typescript                      4.7.4

Anything else relevant?

Looking at the code, it seems like this might be a simple fix. 🤞

The duplicate message IDs are checked here with a hard-coded value of “warning” and then always logged as warnings: https://github.com/angular/angular-cli/blob/1b89fd43400455366a3054f6ada10fb0dae5a209/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts#L273-L283

Earlier in the same function, the browser target options are fetched here, which should contain the value of the i18nDuplicateTranslation option: https://github.com/angular/angular-cli/blob/1b89fd43400455366a3054f6ada10fb0dae5a209/packages/angular_devkit/build_angular/src/builders/extract-i18n/index.ts#L138-L141

About this issue

Original URL
State: open
Created 2 years ago
Reactions: 14
Comments: 17 (3 by maintainers)

Most upvoted comments

Some discussion about this option today. We definitely agree it should be possible to treat duplicate message IDs as an error during extraction.

What confused us somewhat is why this is an option to begin with? In the event of a duplicate translation which is ignored, I’m not sure we can do anything intelligent to pick between them. Any tool which processes the extracted output won’t know which message to use. And when the CLI merges translations back in, we can’t know which one is right for the duplicated ID. I double checked with @AndrewKushnir, who also couldn’t recall a compelling reason for why a user would want to ignore such duplications.

I’m thinking we should delete the option altogether and just always treat duplicate translation IDs as a hard error, both at extraction and merge times.

dgp1130 on Aug 4, 2022

Just confirming my own thoughts here. With these files:

app.component.html:

<span i18n="@@test-first">First</span>

test.component.html:

<span i18n="@@test-first">First</span>

app.component.html

export class AppComponent {
  title = $localize`:@@test-first:First`;
}

There are no warnings from ng extract-i18n and this is the output:

<?xml version="1.0" encoding="UTF-8" ?>
<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
  <file source-language="en-US" datatype="plaintext" original="ng2.template">
    <body>
      <trans-unit id="test-first" datatype="html">
        <source>First</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/app.component.html</context>
          <context context-type="linenumber">1</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/app.component.ts</context>
          <context context-type="linenumber">9</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/test/test.component.html</context>
          <context context-type="linenumber">1</context>
        </context-group>
      </trans-unit>
    </body>
  </file>
</xliff>

However, if I add some whitespace to one of the values, for example:

<span i18n="@@test-first"> First </span>

Then you get a warning, which is 100% what I would expect (although it should be an error 😆)

WARNINGS:
 - Duplicate messages with id "test-first":
   - "First" : src\app\app.component.ts:9
   - " First " : src\app\app.component.html:1
   - "First" : src\app\test\test.component.html:1

So you can use the same ID in multiple templates. You just have to make sure they have the same text value. If they don’t have the same text value, then you absolutely should be alerted about it via an error.

reduckted on Aug 6, 2022

I am hoping that since this issue is not closed, it will eventually be fixed. It would be nice to be able to break builds that introduce i18n bugs.

superole on Apr 16, 2023

@reduckted, I don’t think our behavior needs to change there. Just like how no warning is emitted when the text matches, we don’t have to fail that case. My suggestion is to always error in the case where there are two messages with the same ID and different content.

On a separate point, if you have two locations where you want to use the same translations, I recommend using $localize instead at one place in your code and just binding it in two locations. That way you have a single source of truth and you don’t have to hard-code an ID anywhere.

dgp1130 on Aug 8, 2022

We could expose a similar option to i18nDuplicateTranslation. Let me check with the rest of the team.

alan-agius4 on Jul 25, 2022