Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extract-i18n generates wrong interpolation equiv-text in v11 #39654

Closed
julkue opened this issue Nov 12, 2020 · 46 comments
Closed

extract-i18n generates wrong interpolation equiv-text in v11 #39654

julkue opened this issue Nov 12, 2020 · 46 comments
Labels
area: i18n P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Milestone

Comments

@julkue
Copy link
Contributor

julkue commented Nov 12, 2020

🐞 bug report

Is this a regression?

This was working in Angular v10.2.3.

Description

Angular doesn't seem to generate the correct interpolation equiv-text attributes anymore in Angular v11. When running extract-i18n it previously generated for example:

<source>(<x id="INTERPOLATION" equiv-text="{{ start }}"/>–<x id="INTERPOLATION_1" equiv-text="{{ calculatedEnd }}"/> / <x id="INTERPOLATION_2" equiv-text="{{ totalCount }}"/> results)</source>

now it generates:

<source>(<x id="INTERPOLATION" equiv-text="amMap): void { "/>–<x id="INTERPOLATION_1" equiv-text="usually be a pa"/> / <x id="INTERPOLATION_2" equiv-text="ince it doesn't"/> results)</source>

I have no clue where e.g. the equiv-text ince it doesn't is coming from, that string doesn't even exist in my app at all.

🌍 Your Environment

Angular Version:


Angular CLI: 11.0.0
Node: 12.16.3
OS: win32 x64

Angular: 11.0.0
... animations, cli, common, compiler, compiler-cli, core
... elements, forms, language-service, localize
... platform-browser, platform-browser-dynamic, router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1100.0 (cli-only)
@angular-devkit/build-angular   0.1100.0
@angular-devkit/core            11.0.0 (cli-only)
@angular-devkit/schematics      11.0.0
@schematics/angular             11.0.0
@schematics/update              0.1100.0
ng-packagr                      11.0.2
rxjs                            6.6.3
typescript                      4.0.5
@ngbot ngbot bot added this to the needsTriage milestone Nov 12, 2020
@petebacondarwin
Copy link
Member

Is it possible to provide a runnable example of this?
Are these i18n messages in you application or in a library?

@petebacondarwin petebacondarwin added the needs reproduction This issue needs a reproduction in order for the team to investigate further label Nov 12, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 12, 2020
@julkue
Copy link
Contributor Author

julkue commented Nov 12, 2020

@petebacondarwin Yes, it's a translation that I made in a library. Actually I only have library translations.

I tried to set up a reproducable minimal demo near to config of my real application, but it's working. I've uploaded as repo though, in case you still want to have a look at it:
https://github.com/julmot/ng-equiv-text-demo

What I also find strange in this context is that in my real application I'm getting a warning like:

WARNINGS:
 - Duplicate messages with id "productDetailVariantTablePriceColumnLabel":
   - "UVP price" : dist\XXX\src\lib\components\product-variants-table\product-variants-table.component.ts:147
   - "UVP Price" : dist\XXX\src\lib\components\product-variants-table\product-variants-table.component.ts:147

When adding a duplicate translation in the example repo, I'm not getting this message. Use case: adaptive designs, where two different HTML versions have to be generated for different viewports (unfortunately).

@petebacondarwin
Copy link
Member

OK, so I think the warnings you are getting is because you have two messages (on the same line??) that have the same custom id (@@ productDetailVariantTablePriceColumnLabel) but subtly different text within the message. Note the case difference UVP price vs UVP Price. This warning is to try to ensure that you don't inadvertently mark two different messages with the same id. Does this sound right?

@petebacondarwin
Copy link
Member

Regarding your original issue, the equivalent text is computed via the source-mapping, since the compiled code that contains the $localize tagged strings no longer contains the original source text. If there is a problem with the generated source-maps for your libraries then this could have an impact.

@julkue
Copy link
Contributor Author

julkue commented Nov 13, 2020

@petebacondarwin Thanks for your answer, it was very helpful.

but subtly different text within the message

Gotcha, I haven't noticed the capital P here, my fault. 👍🏻

on the same line??

No, the log shows a wrong line number here for both usages. The line number 147 doesn't even exist. They are in the same file though.

If there is a problem with the generated source-maps for your libraries then this could have an impact.

This was the missing peace of the puzzle 👌, I didn't know this is related. Indeed I have many source map related warnings of @apollo/client, see:
apollographql/apollo-client#6921 (comment)

@petebacondarwin
Copy link
Member

I believe that the problems with the equiv-text should now be solved - via a combination of #39717 and #39486

Can you check @julmot ?

@julkue
Copy link
Contributor Author

julkue commented Nov 24, 2020

@petebacondarwin Sure, I'd love to. But how can I test those PRs when they are unreleased?

@petebacondarwin
Copy link
Member

petebacondarwin commented Nov 24, 2020

Agh! What a mess. It appears that #39717 was merged after 11.0.2 was released so is not yet in an official release. Further #39486 was never merged to the 11.0.x branch; and so also does not appear. #39486 did land on 11.0.x - but like #39717 it has not yet been released.

You can test against the bleeding edge build if you want by using this dependency in your package.json:

"@angular/compiler": "https://github.com/angular/compiler-builds"

(this may assume that the rest of the package are on the next dist-tag from npm - i.e. 11.1.x).

@julkue
Copy link
Contributor Author

julkue commented Nov 24, 2020

When installing the package you've mentioned above directly from GitHub, the problem doesn't seem to be solved. It's generating for example:

      <trans-unit id="productDetailPageTitle" datatype="html">
        <source><x id="INTERPOLATION" equiv-text="ectionStrategy."/> product detail page</source>
        <context-group purpose="location">
          <context context-type="sourcefile">dist/cando-cx-ng/b2b-product-detail/projects/cando-cx-ng/b2b-product-detail/src/lib/components/product-detail/product-detail.component.ts</context>
          <context context-type="linenumber">29,33</context>
        </context-group>
        <note priority="1" from="description">Document</note>
        <note priority="1" from="meaning">Page title</note>
      </trans-unit>

So, the equiv-text is ectionStrategy. in this case, instead of {{ product.name }}. The linenumber is wrong too, it's actually 15+16 instead of 29,33. The usage is:

<div [hidden]="true" *ngIf="product">
  <span #pageTitle i18n="Page title|Document@@productDetailPageTitle">{{ product.name }} product detail page</span>
  <span #pageDescription i18n="Page description|Document@@productDetailPageDescription">{{ product.name }} product detail page</span>
</div>

(This was introduced since at this time translation using $localize wasn't supported. Not even sure if it is now. So we're reading the translations from the template to handle it in TypeScript).

@petebacondarwin
Copy link
Member

This line:

          <context context-type="sourcefile">dist/cando-cx-ng/b2b-product-detail/projects/cando-cx-ng/b2b-product-detail/src/lib/components/product-detail/product-detail.component.ts</context>

Implies that you are running extraction on an application that has a dependency on a library, perhaps? Is that correct?
Have all the pieces of the project been rebuilt from scratch with this new Angular compiler package?

@julkue
Copy link
Contributor Author

julkue commented Nov 24, 2020

@petebacondarwin It's a setup like in my example repo that I've created for this issue:
https://github.com/julmot/ng-equiv-text-demo

Just that next to the "shared" library I have further libraries like the mentioned b2b-product-detail in my real application.

@petebacondarwin
Copy link
Member

I created a PR on your demo: julkue/ng-equiv-text-demo#1
This shows that the latest code fixes the problem (at least for the demo).

@ozkoidi
Copy link

ozkoidi commented Nov 27, 2020

Hello, I have the same problem, after uprading to Angular 11 translations that contain interplations are not properly extracted anymore. Running ng extract-i18n on the following example:
<span i18n="@@exampleId">Text to translate: {{ value }}</span>

generates this output in messages.xlf:

<trans-unit id="exampleId" datatype="html">
	<source>Text to translate: <x id="INTERPOLATION" equiv-text=" translate: {{ va"/></source>
	<context-group purpose="location">
	  <context context-type="sourcefile">src/app/modules/.../example.component.html</context>
	  <context context-type="linenumber">13</context>
	</context-group>
  </trans-unit>

I did try the version of angular/compiler you suggested and the result is the same, this is a part of my packages.json file:

...
"@angular/common": "^11.0.2",
"@angular/compiler": "https://github.com/angular/compiler-builds#0755e9b961b3633057c9f44792d39722bb097f70",
"@angular/core": "^11.0.2",
"@angular/localize": "~11.0.2",
...

Also, this same issue is present when translating messages in Typescript files. Here is an example, the following code:

this.translatedMessage = $localize`:@@exampleIdInTypescript:Message with interpolations ${this.test} included`;

generates this output in messages.xlf:

<trans-unit id="exampleIdInTypescript" datatype="html">
	<source>Message with interpolations <x id="PH" equiv-text="
con"/> included</source>
	<context-group purpose="location">
	  <context context-type="sourcefile">src/app/modules/.../test.component.ts</context>
	  <context context-type="linenumber">491</context>
	</context-group>
 </trans-unit>

Meaning that now every translation that contains interpolations in our application is broken.
Do you think a fix will be available soon? Should I downgrade Angular in the meantime or is there any workaround I could use for this issue?

Thanks in advance

@julkue
Copy link
Contributor Author

julkue commented Nov 27, 2020

@petebacondarwin

This shows that the latest code fixes the problem

I'm afraid, as mentioned, the repo of mine only demonstrates how my application is set up, it doesn't demonstrate the problem itself, sorry. So, it might fix a manually created invalid equiv-text, but in my real application it doesn't fix the problem. The reason is, that at the time I created the demo-project I didn't know that the issue is caused by source map warnings like this. Therefore, in the demo project apollo-client isn't used, which is – according to the information I have currently – the cause of the issue.

I'm hoping it's solved as soon as apollo is fixing the issue, but if there's any chance working around I'd be very happy to hear your thoughts.

@julkue
Copy link
Contributor Author

julkue commented Nov 27, 2020

@ozkoidi Thanks for participating.

Also, this same issue is present when translating messages in Typescript files. Here is an example, the following code:

Is this even valid? I know of this option since Q4 2019, but I couldn't find $localize in the Angular docs, so I thought and still think it's experimental.

@petebacondarwin
Copy link
Member

@ozkoidi - I can't help resolve this problem without a reproduction that I can look at. Do you have one?
@julmot - $localize is still not "officially" supported, which is why there are no docs. But it has been talked about enough, and is stable enough that it might as well be.

@ozkoidi
Copy link

ozkoidi commented Nov 30, 2020

Meaning that now every translation that contains interpolations in our application is broken.

This statement is incorrect. Angular is able to display the proper text (with interpolated values) even though all the equiv-text are wrong, so the issue is not as bad as I thought (since the user can still see the proper content).

@julmot, I also had source map related warnings related to a third party library (class-transformer).
In my case, removing the library (and thus the warnings) did not solve the issue, all equiv-text are still incorrect.

@petebacondarwin
Copy link
Member

Angular is able to display the proper text (with interpolated values) even though all the equiv-text are wrong

Yes, as you say, the equivText attributes play no part in the Angular translation process. They are only there to help translators understand where the placeholders come from.

There are a couple of fixes awaiting release (probably on Wednesday). Please try again when this is released.

@anne-gropler
Copy link

anne-gropler commented Dec 4, 2020

I just updated to 11.0.3. The equiv-texts changed, but they are still broken as described above. Is 11.0.3 the targeted version for this fix?

@julkue
Copy link
Contributor Author

julkue commented Dec 4, 2020

@anne-gropler Do you also have source map warnings during extract-i18n? If so, from which package?

@anne-gropler
Copy link

anne-gropler commented Dec 4, 2020

Mmh.. You're are right, there is a warning:

Warning: ./node_modules/MYLIB/lib/MYLIB.js Module Warning (from ./node_modules/@angular-devkit/build-angular/src/extract-i18n/ivy-extract-loader.js): (Emitted value instead of an instance of Error) Unable to fully load C:\Users\anne.gropler\Documents\node_modules\webgl-operate\lib\webgl-operate.js for source-map flattening: Unknown file requested: C:\Users\anne.gropler\Documents\node_modules\webgl-operate\lib\webgl-operate.js.map

My webgl-operate is a private (and old) fork, which I haven't changed in a while. It does not use Angular. "MYLIB" is a private lib that I actively maintain and which is used by MYAPP, the app that needs those translation files.

The paths seem odd. There is no Documents\node_modules so some relative path seems to be wrong. EDIT: It should be Documents\MYAPP\node_modules.

@petebacondarwin
Copy link
Member

@anne-gropler - sorry this is a problem for you. If you are able to provide me with a repo that I can debug then I am confident I could find out what is happening. If the paths are missing MYAPP then I suspect there might be a problem with the base urls that the source-mapping tooling is using.

@petebacondarwin
Copy link
Member

11.0.3 does contain all the fixes I have provided for this. So if you are still getting the problem then there is something else going on...

@julkue
Copy link
Contributor Author

julkue commented Dec 4, 2020

@petebacondarwin Is your expectation for v11.0.3 that it's fixed, even with source map warnings?

@petebacondarwin
Copy link
Member

If the i18n messages (i.e. the $localize tagged strings) are in code that source-maps to a valid source, then I believe it should all work with 11.0.3. If the original source containing the i18n message is not accessible (because it is not there, or because the source-map is invalid) then I would not expect this to work.

@petebacondarwin
Copy link
Member

Thanks for the update @terencehonles - I am glad that your issues appear to be solved.

@terencehonles
Copy link
Contributor

I was able to find some issues, and I think this might be related to ngcc (and possibly it not updating the source maps) and why people are seeing this in libraries.

Running the following should reproduce:

# with angular 11.0.4
npm install @ng-bootstrap/ng-bootstrap
npx ngcc \                                                              
    --properties es2015 browser module main \                              
    --first-only \                                                         
    --create-ivy-entry-points
node_modules/.bin/localize-extract \
    -s node_modules/@ng-bootstrap/ng-bootstrap/__ivy_ngcc__/fesm2015/ng-bootstrap.js \
    -f xlf \
    -o /dev/stdout

The following appears to be what I'd see in messages.xlf (along with the messages from my application)

<?xml version="1.0" encoding="UTF-8" ?>
<xliff version="1.2" xmlns="urn:oasis:names:tc:xliff:document:1.2">
  <file source-language="en" datatype="plaintext" original="ng2.template">
    <body>
      <trans-unit id="ngb.alert.close" datatype="html">
        <source>Close</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/alert/alert.ts</context>
          <context context-type="linenumber">55,58</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/alert/alert.ts</context>
          <context context-type="linenumber">70,71</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.carousel.slide-number" datatype="html">
        <source> Slide <x id="INTERPOLATION" equiv-text="ate _pauseOnHov"/> of <x id="INTERPOLATION_1" equiv-text="ect(false);
  p"/> </source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/carousel/carousel.ts</context>
          <context context-type="linenumber">114,118</context>
        </context-group>
        <note priority="1" from="description">Currently selected slide number read by screen reader</note>
      </trans-unit>
      <trans-unit id="ngb.carousel.previous" datatype="html">
        <source>Previous</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/carousel/carousel.ts</context>
          <context context-type="linenumber">132,134</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.carousel.next" datatype="html">
        <source>Next</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/carousel/carousel.ts</context>
          <context context-type="linenumber">147,151</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.datepicker.previous-month" datatype="html">
        <source>Previous month</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation.ts</context>
          <context context-type="linenumber">24,27</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation.ts</context>
          <context context-type="linenumber">34,35</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.datepicker.next-month" datatype="html">
        <source>Next month</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation.ts</context>
          <context context-type="linenumber">44,48</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation.ts</context>
          <context context-type="linenumber">57,61</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.datepicker.select-month" datatype="html">
        <source>Select month</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation-select.ts</context>
          <context context-type="linenumber">44,49</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation-select.ts</context>
          <context context-type="linenumber">49,50</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.datepicker.select-year" datatype="html">
        <source>Select year</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation-select.ts</context>
          <context context-type="linenumber">59,63</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/datepicker/datepicker-navigation-select.ts</context>
          <context context-type="linenumber">72,74</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.first" datatype="html">
        <source>««</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">147,148</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.previous" datatype="html">
        <source>«</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">153,154</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.next" datatype="html">
        <source>»</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">158,159</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.last" datatype="html">
        <source>»»</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">164,165</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.first-aria" datatype="html">
        <source>First</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">168,172</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.previous-aria" datatype="html">
        <source>Previous</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">176,177</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.next-aria" datatype="html">
        <source>Next</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">188,189</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.pagination.last-aria" datatype="html">
        <source>Last</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/pagination/pagination.ts</context>
          <context context-type="linenumber">195,200</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.progressbar.value" datatype="html">
        <source><x id="INTERPOLATION" equiv-text="turn this._max;"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/progressbar/progressbar.ts</context>
          <context context-type="linenumber">31,38</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.HH" datatype="html">
        <source>HH</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">46,47</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.hours" datatype="html">
        <source>Hours</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">50,51</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.MM" datatype="html">
        <source>MM</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">55,58</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.minutes" datatype="html">
        <source>Minutes</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">62,64</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.increment-hours" datatype="html">
        <source>Increment hours</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">68,69</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.decrement-hours" datatype="html">
        <source>Decrement hours</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">73,74</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.increment-minutes" datatype="html">
        <source>Increment minutes</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">80,82</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.decrement-minutes" datatype="html">
        <source>Decrement minutes</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">86,88</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.SS" datatype="html">
        <source>SS</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">91,92</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.seconds" datatype="html">
        <source>Seconds</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">96</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.increment-seconds" datatype="html">
        <source>Increment seconds</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">103,104</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.decrement-seconds" datatype="html">
        <source>Decrement seconds</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">109,115</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.PM" datatype="html">
        <source><x id="INTERPOLATION" equiv-text="sible.
   */
  "/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">131,136</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.timepicker.AM" datatype="html">
        <source><x id="INTERPOLATION" equiv-text="nts ControlValu"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/timepicker/timepicker.ts</context>
          <context context-type="linenumber">154,118</context>
        </context-group>
      </trans-unit>
      <trans-unit id="ngb.toast.close-aria" datatype="html">
        <source>Close</source>
        <context-group purpose="location">
          <context context-type="sourcefile">node_modules/@ng-bootstrap/src/toast/toast.ts</context>
          <context context-type="linenumber">78,85</context>
        </context-group>
      </trans-unit>
    </body>
  </file>
</xliff>

@petebacondarwin
Copy link
Member

Thanks for the reproduction. You can see here that the output from ngcc does not map the $localize calls at all, so the localize-extract is trying to guess the position of the placeholders in the original source.

I don't think the origjnal build of the library (before ngcc runs) maps the template string either, so it might be that ngcc doesn't know where to map the ivy instructions (and i18n $localize strings) to. I need to check if this is the problem.

@petebacondarwin
Copy link
Member

OK, so I reminded myself of the ngcc code that converts ViewEngine library code into Ivy library code. It does very broad brush source-mapping. Using magic-string each whole block of code that represents a component definition will have a single mapping, so any fine-grained mappings between Ivy instructions (and also $localize tagged messages) and the original template string are lost.

This will be rectified with the introduction of the new "ng-linker" for libraries, which will have much better source-mapping. Since we are well on our way with the linker (see https://github.com/orgs/angular/projects/2), and ngcc will become deprecated when it is available, I do not see this as something we will try to fix in ngcc.

Are there any cases where the i18n translations in the actual application (rather than in ngcc processed libraries in node_modules) are actually affected by this?

@anne-gropler
Copy link

Thanks everyone for digging! :)

@petebacondarwin Yes, this happens for my application. I don't translate anything located in node_modules. I still get wrong equiv-texts. This is not breaking my application, but it makes merging really hard, because a few lines diff in code now result in ~1000 lines diff in the translation files.

      <trans-unit id="singleStuffBroken" datatype="html">
        <source>Warning:
          <x id="INTERPOLATION" equiv-text="an i18n=&quot;@@multip"/> stuff is broken!
        </source>
        <target state="translated">Achtung:
          <x id="INTERPOLATION" equiv-text="{{ data.failed }}"/> Zeug ist kaputt!
        </target>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/core/i18n/dummyI18N.component.html</context>
          <context context-type="linenumber">196</context>
        </context-group>
      </trans-unit>

Funny though that the equiv-texts for the target seem mostly fine.

@petebacondarwin
Copy link
Member

@anne-gropler - the target elements are not touched by the extraction. It generates a file that only contains source elements. You must be using some kind of merge tool to copy those sources into the translation file?
Please can you try to recreate the problem in a repository that I can debug?

@terencehonles
Copy link
Contributor

terencehonles commented Dec 16, 2020

@petebacondarwin not sure where to put this question so apologies if there's a better place. This is related the comment about equiv-text in the target not matching in the source.

I'm using xliffmerge which likely @anne-gropler is using also. I was wondering if there has been any discussion of adding a "merge" step in addition to the localize-extract and localize-translate functionality that already exists. One downside of using xliffmerge and xlf-merge (they perform different tasks) is that they come with their own xml parsers and they potentially munge the output in different ways. They also are additional tools that have to be configured and that configuration roughly lives in angular.json's "i18n" section. The localize project already contains the serializers and the parsers needed to load and modify the localization files, and I was wondering if it or parts of it could be made public or if the merging functionality could be pulled into Angular.

I'm willing to take a stab at it if it's something that's likely to be merged, but I don't want to waste my time if not.

I also noticed that xml:space="preserve" is not added on the xliff files there's no way to add it via the @angular-devkit/build-angular:extract-i18n builder. Is there a reason it's not added by default, and is there a reason it's not used only when {preserveWhitespace: true} rather than on the whole document? Ideally a formatter could be allowed to strip whitespace outside of the <source /> and <target /> tags, but it should be at least be included by default (I'm not sure why you wouldn't want to include it by default or if that was an oversight on the xml formats)

xml.startTag('source', {}, {preserveWhitespace: true});

format options are missing here:
https://github.com/angular/angular-cli/blob/d7494c4d8dfa8f30b36412aab7e5fd1f9f08ac23/packages/angular_devkit/build_angular/src/extract-i18n/index.ts#L67

and not default here:

private formatOptions: FormatOptions = {}, private fs: FileSystem = getFileSystem()) {

@petebacondarwin
Copy link
Member

Hi @terencehonles - thanks for this request and query.

There is already an open issue for adding support for merging of extracted translations at #37655. I suggest that we continue the discussion of that feature there.

(The short answer is that it is not a trivial tool to create since people can have a wide range of setups - e.g. where the translation files are stored, what to do when a translation has been added/changed/removed, and how to do notification - and so we don't have capacity to implement that ourselves. But I am open to making the pieces of @angular/localize, such as the parsers and serializers, publicly available to allow a 3rd party to implement such a tool. This is basically what https://www.locl.app/ have been doing - but using the private APIs.)

Regarding the xml:space question. Let's open up a new issue to track that. There was some initial implementation of support for this here #38737. And there was some discussion of it and in particular why it was not the default here #38679 (comment).

@c-goldschmidt
Copy link

Hi @petebacondarwin,

together with @anne-gropler, i created an MWE repository here: https://github.com/c-goldschmidt/mwe_angular_translations.
I added my current environment to the README.md.

We found that the broken equiv-texts do not originate in the library error, as the MWE does not use the library we had previously thought to be causing the error. As @terencehonles correctly stated, we are indeed using xliffmerge. However, this also does not seem to be the origin of this problem. I included an i18n-no-merge script in the package.json, which also leads to these broken equiv-texts.

It looks like in general, having HTML inside an element using the i18n directive will lead to broken equiv-texts.

@terencehonles
Copy link
Contributor

@c-goldschmidt yeah, xliffmerge will only leave bad equiv-text in the target (and never update after creation) but the localize extractor is creating the bad source equiv-texts.

@petebacondarwin
Copy link
Member

petebacondarwin commented Dec 17, 2020

@c-goldschmidt - thanks for the reproduction. I will look at this first thing tomorrow.
I suspect it might be the same problem as #40169 given that you are running Windows.

@petebacondarwin
Copy link
Member

Indeed it does appear that the \r\n line endings are triggering the bad extraction and the change described in #40169 (comment) fixes it.

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 18, 2020
…ping

Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.

Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.

Fixes angular#40169
Fixes angular#39654
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 18, 2020
…ping

Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.

Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.

Fixes angular#40169
Fixes angular#39654
@petebacondarwin petebacondarwin added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR and removed needs reproduction This issue needs a reproduction in order for the team to investigate further labels Dec 18, 2020
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 22, 2020
…ping

Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.

Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.

Fixes angular#40169
Fixes angular#39654
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Dec 22, 2020
…ping

Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.

Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.

Fixes angular#40169
Fixes angular#39654
josephperrott pushed a commit that referenced this issue Dec 22, 2020
…ping (#40187)

Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.

Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.

Fixes #40169
Fixes #39654

PR Close #40187
@c-goldschmidt
Copy link

@petebacondarwin the PR was closed without fix, aswell as this ticket. is there a replacement issue for this?

@petebacondarwin
Copy link
Member

The PR and issue were closed because we merged in the fix from it.

@jessebuitenhuis
Copy link

Great news! When will the release be ready? Can we use the code right now?

@petebacondarwin
Copy link
Member

This fix should appear in the next patch release 11.0.6, which should be released this week or next.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: i18n P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants