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

Jump to bottom

target-language was not needed in VE but seems to be required on Ivy i18n #33323

Closed
filipesilva opened this issue Oct 22, 2019 · 3 comments
Closed

target-language was not needed in VE but seems to be required on Ivy i18n #33323

filipesilva opened this issue Oct 22, 2019 · 3 comments
Assignees
@petebacondarwin
Labels
area: i18n state: has PR
Milestone
needsTriage

Comments

@filipesilva
Copy link
Contributor

filipesilva commented Oct 22, 2019 •
edited

🐞 bug report

Affected Package

The issue is caused by package @angular/localize

Is this a regression?

No

Description

target-language was not needed in VE but seems to be required on Ivy.

@clydin looked into this and it seems like the xliff 1.2 parser lists it as optional in http://docs.oasis-open.org/xliff/v1.2/os/xliff-core.html.

But @angular/localize will throw if it's not there:

angular/packages/localize/src/tools/src/translate/translation_files/translation_parsers/xliff1/xliff1_translation_parser.ts

Line 56 in ec482da

locale: getAttrOrThrow(element, 'target-language'),

@petebacondarwin can you shed some light on this? Is this intended, and if so, is there documentation telling users they must do it?

🔬 Minimal Reproduction

The e2e test in angular/angular-cli#15913

🔥 Exception or Error

🌍 Your Environment

Angular Version:


@angular/localize@9.0.0-next.12

Anything else relevant?
@filipesilva filipesilva mentioned this issue Oct 22, 2019
Closed
@ngbot ngbot bot added this to the needsTriage milestone Oct 22, 2019
@IgorMinar
Copy link
Contributor

We discussed this in the tooling sync and concluded that if the xliff spec doesn't require the language to be specified inline, we should not require it as otherwise we'd be making existing translation files without the property incompatible.

Instead, we should allow for the locale id / language id to be set via a command line flag. And error only if it was not provided inline or on the command line. The CLI already passes the locale info into the localize code, so no special work is needed on the CLI side.

@petebacondarwin petebacondarwin self-assigned this Oct 24, 2019
@petebacondarwin
Copy link
Member

I will deal with this in the next few days but not in time for RC.

petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 24, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
9af849e 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
@petebacondarwin petebacondarwin mentioned this issue Oct 24, 2019
Closed
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
0b7b730 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

NOTE to early adopters: if you were using `TranslationParser`s directly
then the return type for the `parse()` method has changed. The properties
of the returned object changed from:

```
interface TranslationBundle {
  locale: string;
  translations: Record<ɵMessageId, ɵParsedTranslation>;
}
```

to

```
interface ParsedTranslationBundle {
  parsedLocale: string|undefined;
  parsedTranslations: Record<ɵMessageId, ɵParsedTranslation>;
}
```

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
90a82b1 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

NOTE to early adopters: if you were using `TranslationParser`s directly
then the return type for the `parse()` method has changed. The properties
of the returned object changed from:

```
interface TranslationBundle {
  locale: string;
  translations: Record<ɵMessageId, ɵParsedTranslation>;
}
```

to

```
interface ParsedTranslationBundle {
  parsedLocale: string|undefined;
  parsedTranslations: Record<ɵMessageId, ɵParsedTranslation>;
}
```

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
1f729c5 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
55dce6b 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
c432d6f 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
4c230c8 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
petebacondarwin added a commit to petebacondarwin/angular that referenced this issue Oct 25, 2019
@petebacondarwin
fix(ivy): i18n - support setting locales for each translation file
185f747 
Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323
mohaxspb pushed a commit to mohaxspb/angular that referenced this issue Nov 7, 2019
@petebacondarwin @mohaxspb
fix(ivy): i18n - support setting locales for each translation file (a…
5b27664 
…ngular#33381)

Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323

PR Close angular#33381
mohaxspb pushed a commit to mohaxspb/angular that referenced this issue Nov 7, 2019
@petebacondarwin @mohaxspb
fix(ivy): i18n - support setting locales for each translation file (a…
6a69b55 
…ngular#33381)

Previously the target locale of a translation file had to be extracted
from the contents of the translation file. Therefore it was an error if
the translation file did not provide a target locale.

Now an array of locales can be provided via the `translationFileLocales`
option that overrides any target locale extracted from the file.
This allows us to support translation files that do not have a target
locale specified in their contents.

// FW-1644
Fixes angular#33323

PR Close angular#33381
@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 Nov 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@petebacondarwin petebacondarwin

Labels
area: i18n state: has PR
Projects
None yet
Milestone
needsTriage
Development

Successfully merging a pull request may close this issue.

fix(ivy): i18n - support setting locales for each translation file petebacondarwin/angular
4 participants
@petebacondarwin @matsko @IgorMinar @filipesilva