Skip to content

feat(compiler): add target locale to the translation bundles #14290

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

Closed
vicb wants to merge 1 commit into from
Closed

feat(compiler): add target locale to the translation bundles #14290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion modules/@angular/compiler/src/i18n/serializers/serializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export abstract class Serializer {
// - Placeholder names are already map to public names using the provided mapper
abstract write(messages: i18n.Message[]): string;

abstract load(content: string, url: string): {[msgId: string]: i18n.Node[]};
abstract load(content: string, url: string):
{locale: string | null, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}};

abstract digest(message: i18n.Message): string;

Expand Down
18 changes: 15 additions & 3 deletions modules/@angular/compiler/src/i18n/serializers/xliff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const _XMLNS = 'urn:oasis:names:tc:xliff:document:1.2';
const _SOURCE_LANG = 'en';
const _PLACEHOLDER_TAG = 'x';

const _FILE_TAG = 'file';
const _SOURCE_TAG = 'source';
const _TARGET_TAG = 'target';
const _UNIT_TAG = 'trans-unit';
Expand Down Expand Up @@ -68,10 +69,11 @@ export class Xliff extends Serializer {
]);
}

load(content: string, url: string): {[msgId: string]: i18n.Node[]} {
load(content: string, url: string):
{locale: string, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
// xliff to xml nodes
const xliffParser = new XliffParser();
const {mlNodesByMsgId, errors} = xliffParser.parse(content, url);
const {locale, mlNodesByMsgId, errors} = xliffParser.parse(content, url);

// xml nodes to i18n nodes
const i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {};
Expand All @@ -86,7 +88,7 @@ export class Xliff extends Serializer {
throw new Error(`xliff parse errors:\n${errors.join('\n')}`);
}

return i18nNodesByMsgId;
return {locale, i18nNodesByMsgId};
}

digest(message: i18n.Message): string { return digest(message); }
Expand Down Expand Up @@ -154,6 +156,7 @@ class XliffParser implements ml.Visitor {
private _unitMlNodes: ml.Node[];
private _errors: I18nError[];
private _mlNodesByMsgId: {[msgId: string]: ml.Node[]};
private _locale: string|null = null;

parse(xliff: string, url: string) {
this._unitMlNodes = [];
Expand All @@ -167,6 +170,7 @@ class XliffParser implements ml.Visitor {
return {
mlNodesByMsgId: this._mlNodesByMsgId,
errors: this._errors,
locale: this._locale,
};
}

Expand Down Expand Up @@ -200,6 +204,14 @@ class XliffParser implements ml.Visitor {
this._unitMlNodes = element.children;
break;

case _FILE_TAG:
const localeAttr = element.attrs.find((attr) => attr.name === 'target-language');
if (localeAttr) {
this._locale = localeAttr.value;
}
ml.visitAll(this, element.children, null);
break;

default:
// TODO(vicb): assert file structure, xliff version
// For now only recurse on unhandled nodes
Expand Down
3 changes: 2 additions & 1 deletion modules/@angular/compiler/src/i18n/serializers/xmb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ export class Xmb extends Serializer {
]);
}

load(content: string, url: string): {[msgId: string]: i18n.Node[]} {
load(content: string, url: string):
{locale: string, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
throw new Error('Unsupported');
}

Expand Down
13 changes: 10 additions & 3 deletions modules/@angular/compiler/src/i18n/serializers/xtb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ const _PLACEHOLDER_TAG = 'ph';
export class Xtb extends Serializer {
write(messages: i18n.Message[]): string { throw new Error('Unsupported'); }

load(content: string, url: string): {[msgId: string]: i18n.Node[]} {
load(content: string, url: string):
{locale: string, i18nNodesByMsgId: {[msgId: string]: i18n.Node[]}} {
// xtb to xml nodes
const xtbParser = new XtbParser();
const {msgIdToHtml, errors} = xtbParser.parse(content, url);
const {locale, msgIdToHtml, errors} = xtbParser.parse(content, url);

// xml nodes to i18n nodes
const i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {};
Expand All @@ -48,7 +49,7 @@ export class Xtb extends Serializer {
throw new Error(`xtb parse errors:\n${errors.join('\n')}`);
}

return i18nNodesByMsgId;
return {locale, i18nNodesByMsgId};
}

digest(message: i18n.Message): string { return digest(message); }
Expand Down Expand Up @@ -76,6 +77,7 @@ class XtbParser implements ml.Visitor {
private _bundleDepth: number;
private _errors: I18nError[];
private _msgIdToHtml: {[msgId: string]: string};
private _locale: string|null = null;

parse(xtb: string, url: string) {
this._bundleDepth = 0;
Expand All @@ -91,6 +93,7 @@ class XtbParser implements ml.Visitor {
return {
msgIdToHtml: this._msgIdToHtml,
errors: this._errors,
locale: this._locale,
};
}

Expand All @@ -101,6 +104,10 @@ class XtbParser implements ml.Visitor {
if (this._bundleDepth > 1) {
this._addError(element, `<${_TRANSLATIONS_TAG}> elements can not be nested`);
}
const langAttr = element.attrs.find((attr) => attr.name === 'lang');
if (langAttr) {
this._locale = langAttr.value;
}
ml.visitAll(this, element.children, null);
this._bundleDepth--;
break;
Expand Down
16 changes: 9 additions & 7 deletions modules/@angular/compiler/src/i18n/translation_bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,25 @@ export class TranslationBundle {
private _i18nToHtml: I18nToHtmlVisitor;

constructor(
private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {},
private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}, locale: string|null,
public digest: (m: i18n.Message) => string,
public mapperFactory?: (m: i18n.Message) => PlaceholderMapper,
missingTranslationStrategy: MissingTranslationStrategy = MissingTranslationStrategy.Warning,
console?: Console) {
this._i18nToHtml = new I18nToHtmlVisitor(
_i18nNodesByMsgId, digest, mapperFactory, missingTranslationStrategy, console);
_i18nNodesByMsgId, locale, digest, mapperFactory, missingTranslationStrategy, console);
}

// Creates a `TranslationBundle` by parsing the given `content` with the `serializer`.
static load(
content: string, url: string, serializer: Serializer,
missingTranslationStrategy: MissingTranslationStrategy,
console?: Console): TranslationBundle {
const i18nNodesByMsgId = serializer.load(content, url);
const {locale, i18nNodesByMsgId} = serializer.load(content, url);
const digestFn = (m: i18n.Message) => serializer.digest(m);
const mapperFactory = (m: i18n.Message) => serializer.createNameMapper(m);
return new TranslationBundle(
i18nNodesByMsgId, digestFn, mapperFactory, missingTranslationStrategy, console);
i18nNodesByMsgId, locale, digestFn, mapperFactory, missingTranslationStrategy, console);
}

// Returns the translation as HTML nodes from the given source message.
Expand All @@ -65,7 +65,7 @@ class I18nToHtmlVisitor implements i18n.Visitor {
private _mapper: (name: string) => string;

constructor(
private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {},
private _i18nNodesByMsgId: {[msgId: string]: i18n.Node[]} = {}, private _locale: string|null,
private _digest: (m: i18n.Message) => string,
private _mapperFactory: (m: i18n.Message) => PlaceholderMapper,
private _missingTranslationStrategy: MissingTranslationStrategy, private _console?: Console) {
Expand Down Expand Up @@ -166,11 +166,13 @@ class I18nToHtmlVisitor implements i18n.Visitor {
// - use the nodes from the original message
// - placeholders are already internal and need no mapper
if (this._missingTranslationStrategy === MissingTranslationStrategy.Error) {
this._addError(srcMsg.nodes[0], `Missing translation for message "${id}"`);
const ctx = this._locale ? ` for locale "${this._locale}"` : '';
this._addError(srcMsg.nodes[0], `Missing translation for message "${id}"${ctx}`);
} else if (
this._console &&
this._missingTranslationStrategy === MissingTranslationStrategy.Warning) {
this._console.warn(`Missing translation for message "${id}"`);
const ctx = this._locale ? ` for locale "${this._locale}"` : '';
this._console.warn(`Missing translation for message "${id}"${ctx}`);
}
nodes = srcMsg.nodes;
this._mapper = (name: string) => name;
Expand Down
6 changes: 3 additions & 3 deletions modules/@angular/compiler/test/i18n/extractor_merger_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ export function main() {
expect(messages.length).toEqual(1);
const i18nMsgMap: {[id: string]: i18n.Node[]} = {};
i18nMsgMap[digest(messages[0])] = [];
const translations = new TranslationBundle(i18nMsgMap, digest);
const translations = new TranslationBundle(i18nMsgMap, null, digest);

const output =
mergeTranslations(htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, [], {});
Expand Down Expand Up @@ -450,7 +450,7 @@ export function main() {
expect(messages.length).toEqual(1);
const i18nMsgMap: {[id: string]: i18n.Node[]} = {};
i18nMsgMap[digest(messages[0])] = [];
const translations = new TranslationBundle(i18nMsgMap, digest);
const translations = new TranslationBundle(i18nMsgMap, null, digest);

const output =
mergeTranslations(htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, [], {});
Expand Down Expand Up @@ -488,7 +488,7 @@ function fakeTranslate(
i18nMsgMap[id] = [new i18n.Text(`**${text}**`, null)];
});

const translations = new TranslationBundle(i18nMsgMap, digest);
const translations = new TranslationBundle(i18nMsgMap, null, digest);

const output = mergeTranslations(
htmlNodes, translations, DEFAULT_INTERPOLATION_CONFIG, implicitTags, implicitAttrs);
Expand Down
5 changes: 4 additions & 1 deletion modules/@angular/compiler/test/i18n/message_bundle_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ class _TestSerializer extends Serializer {
.join('//');
}

load(content: string, url: string): {} { return null; }
load(content: string, url: string):
{locale: string | null, i18nNodesByMsgId: {[id: string]: i18n.Node[]}} {
return {locale: null, i18nNodesByMsgId: {}};
}

digest(msg: i18n.Message): string { return msg.id || `default`; }
}
Expand Down
8 changes: 6 additions & 2 deletions modules/@angular/compiler/test/i18n/serializers/xliff_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const WRITE_XLIFF = `<?xml version="1.0" encoding="UTF-8" ?>

const LOAD_XLIFF = `<?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">
<file source-language="en" target-language="fr" datatype="plaintext" original="ng2.template">
<body>
<trans-unit id="983775b9a51ce14b036be72d4cfd65d68d64e231" datatype="html">
<source>translatable attribute</source>
Expand Down Expand Up @@ -114,7 +114,7 @@ export function main(): void {
}

function loadAsMap(xliff: string): {[id: string]: string} {
const i18nNodesByMsgId = serializer.load(xliff, 'url');
const {i18nNodesByMsgId} = serializer.load(xliff, 'url');

const msgMap: {[id: string]: string} = {};
Object.keys(i18nNodesByMsgId)
Expand Down Expand Up @@ -143,6 +143,10 @@ export function main(): void {
});
});

it('should return the target locale',
() => { expect(serializer.load(LOAD_XLIFF, 'url').locale).toEqual('fr'); });


describe('structure errors', () => {
it('should throw when a trans-unit has no translation', () => {
const XLIFF = `<?xml version="1.0" encoding="UTF-8" ?>
Expand Down
14 changes: 12 additions & 2 deletions modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function main(): void {
const serializer = new Xtb();

function loadAsMap(xtb: string): {[id: string]: string} {
const i18nNodesByMsgId = serializer.load(xtb, 'url');
const {i18nNodesByMsgId} = serializer.load(xtb, 'url');
const msgMap: {[id: string]: string} = {};
Object.keys(i18nNodesByMsgId).forEach(id => {
msgMap[id] = serializeNodes(i18nNodesByMsgId[id]).join('');
Expand Down Expand Up @@ -54,6 +54,14 @@ export function main(): void {
expect(loadAsMap(XTB)).toEqual({'8841459487341224498': 'rab'});
});

it('should return the target locale', () => {
const XTB = `<?xml version="1.0" encoding="UTF-8"?>
<translationbundle lang='fr'>
<translation id="8841459487341224498">rab</translation>
</translationbundle>`;

expect(serializer.load(XTB, 'url').locale).toEqual('fr');
});

it('should load XTB files with placeholders', () => {
const XTB = `<?xml version="1.0" encoding="UTF-8"?>
Expand Down Expand Up @@ -111,7 +119,9 @@ export function main(): void {

// Invalid messages should not cause the parser to throw
let i18nNodesByMsgId: {[id: string]: i18n.Node[]};
expect(() => { i18nNodesByMsgId = serializer.load(XTB, 'url'); }).not.toThrow();
expect(() => {
i18nNodesByMsgId = serializer.load(XTB, 'url').i18nNodesByMsgId;
}).not.toThrow();

expect(Object.keys(i18nNodesByMsgId).length).toEqual(2);
expect(serializeNodes(i18nNodesByMsgId['angular']).join('')).toEqual('is great');
Expand Down
29 changes: 16 additions & 13 deletions modules/@angular/compiler/test/i18n/translation_bundle_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function main(): void {

it('should translate a plain message', () => {
const msgMap = {foo: [new i18n.Text('bar', null)]};
const tb = new TranslationBundle(msgMap, (_) => 'foo');
const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd', 'i');
expect(serializeNodes(tb.get(msg))).toEqual(['bar']);
});
Expand All @@ -38,7 +38,7 @@ export function main(): void {
const phMap = {
ph1: '*phContent*',
};
const tb = new TranslationBundle(msgMap, (_) => 'foo');
const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i');
expect(serializeNodes(tb.get(msg))).toEqual(['bar*phContent*']);
});
Expand All @@ -58,7 +58,7 @@ export function main(): void {
const msg = new i18n.Message([srcNode], {}, {ph1: refMsg}, 'm', 'd', 'i');
let count = 0;
const digest = (_: any) => count++ ? 'ref' : 'foo';
const tb = new TranslationBundle(msgMap, digest);
const tb = new TranslationBundle(msgMap, null, digest);

expect(serializeNodes(tb.get(msg))).toEqual(['--*refMsg*++']);
});
Expand All @@ -70,13 +70,13 @@ export function main(): void {

const digest = (_: any) => `no matching id`;
// Empty message map -> use source messages in Ignore mode
let tb = new TranslationBundle({}, digest, null, MissingTranslationStrategy.Ignore);
let tb = new TranslationBundle({}, null, digest, null, MissingTranslationStrategy.Ignore);
expect(serializeNodes(tb.get(messages[0])).join('')).toEqual(src);
// Empty message map -> use source messages in Warning mode
tb = new TranslationBundle({}, digest, null, MissingTranslationStrategy.Warning);
tb = new TranslationBundle({}, null, digest, null, MissingTranslationStrategy.Warning);
expect(serializeNodes(tb.get(messages[0])).join('')).toEqual(src);
// Empty message map -> throw in Error mode
tb = new TranslationBundle({}, digest, null, MissingTranslationStrategy.Error);
tb = new TranslationBundle({}, null, digest, null, MissingTranslationStrategy.Error);
expect(() => serializeNodes(tb.get(messages[0])).join('')).toThrow();
});

Expand All @@ -88,13 +88,14 @@ export function main(): void {
new i18n.Placeholder('', 'ph1', span),
]
};
const tb = new TranslationBundle(msgMap, (_) => 'foo');
const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd', 'i');
expect(() => tb.get(msg)).toThrowError(/Unknown placeholder/);
});

it('should report missing translation', () => {
const tb = new TranslationBundle({}, (_) => 'foo', null, MissingTranslationStrategy.Error);
const tb =
new TranslationBundle({}, null, (_) => 'foo', null, MissingTranslationStrategy.Error);
const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd', 'i');
expect(() => tb.get(msg)).toThrowError(/Missing translation for message "foo"/);
});
Expand All @@ -107,16 +108,17 @@ export function main(): void {
};

const tb = new TranslationBundle(
{}, (_) => 'foo', null, MissingTranslationStrategy.Warning, console);
{}, 'en', (_) => 'foo', null, MissingTranslationStrategy.Warning, console);
const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd', 'i');

expect(() => tb.get(msg)).not.toThrowError();
expect(log.length).toEqual(1);
expect(log[0]).toMatch(/Missing translation for message "foo"/);
expect(log[0]).toMatch(/Missing translation for message "foo" for locale "en"/);
});

it('should not report missing translation with MissingTranslationStrategy.Ignore', () => {
const tb = new TranslationBundle({}, (_) => 'foo', null, MissingTranslationStrategy.Ignore);
const tb =
new TranslationBundle({}, null, (_) => 'foo', null, MissingTranslationStrategy.Ignore);
const msg = new i18n.Message([srcNode], {}, {}, 'm', 'd', 'i');
expect(() => tb.get(msg)).not.toThrowError();
});
Expand All @@ -129,7 +131,8 @@ export function main(): void {
const msg = new i18n.Message([srcNode], {}, {ph1: refMsg}, 'm', 'd', 'i');
let count = 0;
const digest = (_: any) => count++ ? 'ref' : 'foo';
const tb = new TranslationBundle(msgMap, digest, null, MissingTranslationStrategy.Error);
const tb =
new TranslationBundle(msgMap, null, digest, null, MissingTranslationStrategy.Error);
expect(() => tb.get(msg)).toThrowError(/Missing translation for message "ref"/);
});

Expand All @@ -143,7 +146,7 @@ export function main(): void {
const phMap = {
ph1: '</b>',
};
const tb = new TranslationBundle(msgMap, (_) => 'foo');
const tb = new TranslationBundle(msgMap, null, (_) => 'foo');
const msg = new i18n.Message([srcNode], phMap, {}, 'm', 'd', 'i');
expect(() => tb.get(msg)).toThrowError(/Unexpected closing tag "b"/);
});
Expand Down