8000 ng extract-i18n extracts incorrect `equiv-text` value · Issue #39671 · angular/angular · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

ng extract-i18n extracts incorrect equiv-text value #39671

Closed
kamilchlebek opened this issue Nov 13, 2020 · 7 comments
Closed

ng extract-i18n extracts incorrect equiv-text value #39671

kamilchlebek opened this issue Nov 13, 2020 · 7 comments
Labels
area: i18n Issues related to localization and internationalization P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone
Backlog

Comments

@kamilchlebek
Copy link

🐞 bug report

Affected Package

The issue is caused by package @angular/localize

Is this a regression?

Yes, the previous version in which this bug was not present was: v10

Description

Component template (it's usually formatted like this by prettier):

<div i18n>
  test {{ title }}
</div>

Running ng xi18n in angular v10, produced output that contained source element that looked:

<source>
   test <x id="INTERPOLATION" equiv-text="{{ title }}"/>
</source>

Now after running ng extract-i18n it's:

<source> test <x id="INTERPOLATION" equiv-text="t {{ title "/>
</source>

🔬 Minimal Reproduction

https://github.com/kamilchlebek/equiv-text-angular11 (see messages.xlf)

🌍 Your Environment

(it's a brand new project created using ng new)

Angular Version:

Angular CLI: 11.0.1
Node: 12.16.1
OS: linux x64

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

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1100.1
@angular-devkit/build-angular   0.1100.1
@angular-devkit/core            11.0.1
@angular-devkit/schematics      11.0.1
@angular/cli                    11.0.1
@schematics/angular             11.0.1
@schematics/update              0.1100.1
rxjs                            6.6.3
typescript                      4.0.5
@petebacondarwin petebacondarwin added area: i18n Issues related to localization and internationalization P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Nov 13, 2020
@ngbot ngbot bot added this to the Backlog milestone Nov 13, 2020
@petebacondarwin
Copy link
Contributor

I can confirm this bug. I am looking into it now.

@petebacondarwin
Copy link
Contributor

Just to give an update... these equiv-text properties are computed via the source-map from the generated code (e.g. app.component.js) back to the original source (e.g. app.component.html). In this case the source-map that was generated for this file is not accurate. See this visualization.

I think this is a bug in the Angular compiler, similar to one that I fixed recently. See #39486.

Investigating further...

@kamilchlebek
Copy link
Author

That's bad 😞

As I workaround I could switch prettier htmlWhitespaceSensitivity to strict. Then the markup will look ugly:

<div i18n
  >test {{ title }}</div
>

but messages.xlf are okay.

Of course I'd rather avoid that for div and other elements being display:block by default.

https://prettier.io/blog/2018/11/07/1.15.0.html#whitespace-sensitive-formatting

@petebacondarwin
Copy link
Contributor

I have found the cause of the problem. When we process the i18n tags, we run the processing twice. Once before whitespace has been removed (which results in correct source-spans) and then again after whitespace has been collapsed (which results in incorrect source-spans for placeholders).

See

angular/packages/compiler/src/render3/view/template.ts

Lines 2085 to 2088 in 1f95618

if (i18nMetaVisitor.hasI18nMeta) {
rootNodes = html.visitAll(
new I18nMetaVisitor(interpolationConfig, /* keepI18nAttrs */ false), rootNodes);
}

A fix is in the works...

@kamilchlebek kamilchlebek mentioned this issue Nov 16, 2020
Closed
15 tasks
@petebacondarwin petebacondarwin mentioned this issue Nov 16, 2020
Closed
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Nov 16, 2020
@petebacondarwin
fix(compiler): ensure that placeholders have the correct sourceSpan
06d498c 
When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.

Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.

This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.

Fixes angular#39671
@petebacondarwin petebacondarwin mentioned this issue Nov 16, 2020
Closed
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Nov 16, 2020
@petebacondarwin
fix(compiler): ensure that placeholders have the correct sourceSpan
73df49d 
When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.

Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.

This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.

Fixes angular#39671
@petebacondarwin
Copy link
Contributor

Here's the fix: #39717. It was not trivial so it took a bit longer than I thought.

@kamilchlebek
Copy link
Author

Great, thank you! Can't wait to see it released 🚀

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Nov 19, 2020
@petebacondarwin
fix(co 8000 mpiler): ensure that placeholders have the correct sourceSpan
25ef0e0 
When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.

Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.

This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.

Fixes angular#39671
AndrewKushnir pushed a commit that referenced this issue Nov 20, 2020
@petebacondarwin @AndrewKushnir
fix(compiler): ensure that placeholders have the correct sourceSpan (#…
8ec7156 
…39717)

When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.

Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.

This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.

Fixes #39671

PR Close #39717
@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 Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: i18n Issues related to localization and internationalization P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

fix(compiler): ensure that placeholders have the correct sourceSpan petebacondarwin/angular
2 participants
@petebacondarwin @kamilchlebek
0