feat(I18N Expander): do not add extra `<ul>` & `<li>` around ICU messages (#9283)

fixes #9072
This commit is contained in:
Victor Berchet 2016-06-17 11:38:24 -07:00 committed by GitHub
parent 7498050421
commit 721f53f0d6
6 changed files with 162 additions and 155 deletions

View File

@ -34,6 +34,31 @@ export function main() {
});
}));
it('should be applicable to <ng-container> elements',
inject(
[TestComponentBuilder, AsyncTestCompleter],
(tcb: TestComponentBuilder, async: AsyncTestCompleter) => {
var template = '<div>' +
'<ng-container [ngPlural]="switchValue">' +
'<template ngPluralCase="=0">you have no messages.</template>' +
'<template ngPluralCase="=1">you have one message.</template>' +
'</ng-container></div>';
tcb.overrideTemplate(TestComponent, template)
.createAsync(TestComponent)
.then((fixture) => {
fixture.debugElement.componentInstance.switchValue = 0;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('you have no messages.');
fixture.debugElement.componentInstance.switchValue = 1;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('you have one message.');
async.done();
});
}));
it('should display the template according to the category',
inject(
[TestComponentBuilder, AsyncTestCompleter],
@ -124,9 +149,7 @@ export class TestLocalizationMap extends NgLocalization {
}
@Component({selector: 'test-cmp', directives: [NgPlural, NgPluralCase], template: ''})
@Component({selector: 'test-cmp', directives: [NgPluralCase, NgPlural], template: ''})
class TestComponent {
switchValue: number;
constructor() { this.switchValue = null; }
switchValue: number = null;
}

View File

@ -22,17 +22,16 @@ const PLURAL_CASES: string[] = ['zero', 'one', 'two', 'few', 'many', 'other'];
* will be expanded into
*
* ```
* <ul [ngPlural]="messages.length">
* <template ngPluralCase="=0"><li i18n="plural_=0">zero</li></template>
* <template ngPluralCase="=1"><li i18n="plural_=1">one</li></template>
* <template ngPluralCase="other"><li i18n="plural_other">more than one</li></template>
* </ul>
* <ng-container [ngPlural]="messages.length">
* <template ngPluralCase="=0">zero</ng-container>
* <template ngPluralCase="=1">one</ng-container>
* <template ngPluralCase="other">more than one</ng-container>
* </ng-container>
* ```
*/
export function expandNodes(nodes: HtmlAst[]): ExpansionResult {
let e = new _Expander();
let n = htmlVisitAll(e, nodes);
return new ExpansionResult(n, e.expanded, e.errors);
const expander = new _Expander();
return new ExpansionResult(htmlVisitAll(expander, nodes), expander.isExpanded, expander.errors);
}
export class ExpansionResult {
@ -45,7 +44,7 @@ export class ExpansionResult {
* @internal
*/
class _Expander implements HtmlAstVisitor {
expanded: boolean = false;
isExpanded: boolean = false;
errors: ParseError[] = [];
visitElement(ast: HtmlElementAst, context: any): any {
@ -61,8 +60,9 @@ class _Expander implements HtmlAstVisitor {
visitComment(ast: HtmlCommentAst, context: any): any { return ast; }
visitExpansion(ast: HtmlExpansionAst, context: any): any {
this.expanded = true;
return ast.type == 'plural' ? _expandPluralForm(ast, this.errors) : _expandDefaultForm(ast);
this.isExpanded = true;
return ast.type == 'plural' ? _expandPluralForm(ast, this.errors) :
_expandDefaultForm(ast, this.errors);
}
visitExpansionCase(ast: HtmlExpansionCaseAst, context: any): any {
@ -71,49 +71,35 @@ class _Expander implements HtmlAstVisitor {
}
function _expandPluralForm(ast: HtmlExpansionAst, errors: ParseError[]): HtmlElementAst {
let children = ast.cases.map(c => {
const children = ast.cases.map(c => {
if (PLURAL_CASES.indexOf(c.value) == -1 && !c.value.match(/^=\d+$/)) {
errors.push(new I18nError(
c.valueSourceSpan,
`Plural cases should be "=<number>" or one of ${PLURAL_CASES.join(", ")}`));
}
let expansionResult = expandNodes(c.expression);
const expansionResult = expandNodes(c.expression);
errors.push(...expansionResult.errors);
let i18nAttrs = expansionResult.expanded ?
[] :
[new HtmlAttrAst('i18n', `${ast.type}_${c.value}`, c.valueSourceSpan)];
return new HtmlElementAst(
`template`,
[
new HtmlAttrAst('ngPluralCase', c.value, c.valueSourceSpan),
],
[new HtmlElementAst(
`li`, i18nAttrs, expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan)],
c.sourceSpan, c.sourceSpan, c.sourceSpan);
`template`, [new HtmlAttrAst('ngPluralCase', `${c.value}`, c.valueSourceSpan)],
expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan);
});
let switchAttr = new HtmlAttrAst('[ngPlural]', ast.switchValue, ast.switchValueSourceSpan);
const switchAttr = new HtmlAttrAst('[ngPlural]', ast.switchValue, ast.switchValueSourceSpan);
return new HtmlElementAst(
'ul', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
'ng-container', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
}
function _expandDefaultForm(ast: HtmlExpansionAst): HtmlElementAst {
function _expandDefaultForm(ast: HtmlExpansionAst, errors: ParseError[]): HtmlElementAst {
let children = ast.cases.map(c => {
let expansionResult = expandNodes(c.expression);
let i18nAttrs = expansionResult.expanded ?
[] :
[new HtmlAttrAst('i18n', `${ast.type}_${c.value}`, c.valueSourceSpan)];
const expansionResult = expandNodes(c.expression);
errors.push(...expansionResult.errors);
return new HtmlElementAst(
`template`,
[
new HtmlAttrAst('ngSwitchWhen', c.value, c.valueSourceSpan),
],
[new HtmlElementAst(
`li`, i18nAttrs, expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan)],
c.sourceSpan, c.sourceSpan, c.sourceSpan);
`template`, [new HtmlAttrAst('ngSwitchCase', `${c.value}`, c.valueSourceSpan)],
expansionResult.nodes, c.sourceSpan, c.sourceSpan, c.sourceSpan);
});
let switchAttr = new HtmlAttrAst('[ngSwitch]', ast.switchValue, ast.switchValueSourceSpan);
const switchAttr = new HtmlAttrAst('[ngSwitch]', ast.switchValue, ast.switchValueSourceSpan);
return new HtmlElementAst(
'ul', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
'ng-container', [switchAttr], children, ast.sourceSpan, ast.sourceSpan, ast.sourceSpan);
}

View File

@ -4,25 +4,23 @@ import {ParseLocation} from '@angular/compiler/src/parse_util';
import {BaseException} from '../src/facade/exceptions';
export function humanizeDom(parseResult: HtmlParseTreeResult): any[] {
export function humanizeDom(
parseResult: HtmlParseTreeResult, addSourceSpan: boolean = false): any[] {
if (parseResult.errors.length > 0) {
var errorString = parseResult.errors.join('\n');
throw new BaseException(`Unexpected parse errors:\n${errorString}`);
}
var humanizer = new _Humanizer(false);
htmlVisitAll(humanizer, parseResult.rootNodes);
return humanizer.result;
return humanizeNodes(parseResult.rootNodes, addSourceSpan);
}
export function humanizeDomSourceSpans(parseResult: HtmlParseTreeResult): any[] {
if (parseResult.errors.length > 0) {
var errorString = parseResult.errors.join('\n');
throw new BaseException(`Unexpected parse errors:\n${errorString}`);
}
return humanizeDom(parseResult, true);
}
var humanizer = new _Humanizer(true);
htmlVisitAll(humanizer, parseResult.rootNodes);
export function humanizeNodes(nodes: HtmlAst[], addSourceSpan: boolean = false): any[] {
var humanizer = new _Humanizer(addSourceSpan);
htmlVisitAll(humanizer, nodes);
return humanizer.result;
}

View File

@ -0,0 +1,99 @@
import {HtmlAttrAst, HtmlElementAst, HtmlTextAst} from '@angular/compiler/src/html_ast';
import {HtmlParser} from '@angular/compiler/src/html_parser';
import {ExpansionResult, expandNodes} from '@angular/compiler/src/i18n/expander';
import {ParseError} from '@angular/compiler/src/parse_util';
import {humanizeNodes} from '@angular/compiler/test/html_ast_spec_utils';
import {ddescribe, describe, expect, iit, it} from '@angular/core/testing/testing_internal';
export function main() {
describe('Expander', () => {
function expand(template: string): ExpansionResult {
const htmlParser = new HtmlParser();
const res = htmlParser.parse(template, 'url', true);
return expandNodes(res.rootNodes);
}
it('should handle the plural expansion form', () => {
const res = expand(`{messages.length, plural,=0 {zero<b>bold</b>}}`);
expect(humanizeNodes(res.nodes)).toEqual([
[HtmlElementAst, 'ng-container', 0],
[HtmlAttrAst, '[ngPlural]', 'messages.length'],
[HtmlElementAst, 'template', 1],
[HtmlAttrAst, 'ngPluralCase', '=0'],
[HtmlTextAst, 'zero', 2],
[HtmlElementAst, 'b', 2],
[HtmlTextAst, 'bold', 3],
]);
});
it('should handle nested expansion forms', () => {
const res = expand(`{messages.length, plural, =0 { {p.gender, gender, =m {m}} }}`);
expect(humanizeNodes(res.nodes)).toEqual([
[HtmlElementAst, 'ng-container', 0],
[HtmlAttrAst, '[ngPlural]', 'messages.length'],
[HtmlElementAst, 'template', 1],
[HtmlAttrAst, 'ngPluralCase', '=0'],
[HtmlElementAst, 'ng-container', 2],
[HtmlAttrAst, '[ngSwitch]', 'p.gender'],
[HtmlElementAst, 'template', 3],
[HtmlAttrAst, 'ngSwitchCase', '=m'],
[HtmlTextAst, 'm', 4],
[HtmlTextAst, ' ', 2],
]);
});
it('should correctly set source code positions', () => {
const nodes = expand(`{messages.length, plural,=0 {<b>bold</b>}}`).nodes;
const container: HtmlElementAst = <HtmlElementAst>nodes[0];
expect(container.sourceSpan.start.col).toEqual(0);
expect(container.sourceSpan.end.col).toEqual(42);
expect(container.startSourceSpan.start.col).toEqual(0);
expect(container.startSourceSpan.end.col).toEqual(42);
expect(container.endSourceSpan.start.col).toEqual(0);
expect(container.endSourceSpan.end.col).toEqual(42);
const switchExp = container.attrs[0];
expect(switchExp.sourceSpan.start.col).toEqual(1);
expect(switchExp.sourceSpan.end.col).toEqual(16);
const template: HtmlElementAst = <HtmlElementAst>container.children[0];
expect(template.sourceSpan.start.col).toEqual(25);
expect(template.sourceSpan.end.col).toEqual(41);
const switchCheck = template.attrs[0];
expect(switchCheck.sourceSpan.start.col).toEqual(25);
expect(switchCheck.sourceSpan.end.col).toEqual(28);
const b: HtmlElementAst = <HtmlElementAst>template.children[0];
expect(b.sourceSpan.start.col).toEqual(29);
expect(b.endSourceSpan.end.col).toEqual(40);
});
it('should handle other special forms', () => {
const res = expand(`{person.gender, gender,=male {m}}`);
expect(humanizeNodes(res.nodes)).toEqual([
[HtmlElementAst, 'ng-container', 0],
[HtmlAttrAst, '[ngSwitch]', 'person.gender'],
[HtmlElementAst, 'template', 1],
[HtmlAttrAst, 'ngSwitchCase', '=male'],
[HtmlTextAst, 'm', 2],
]);
});
describe('errors', () => {
it('should error on unknown plural cases', () => {
expect(humanizeErrors(expand('{n, plural, unknown {-}}').errors)).toEqual([
`Plural cases should be "=<number>" or one of zero, one, two, few, many, other`,
]);
});
});
});
}
function humanizeErrors(errors: ParseError[]): string[] {
return errors.map(error => error.msg);
}

View File

@ -172,99 +172,6 @@ export function main() {
expect(res[1].sourceSpan.start.offset).toEqual(10);
});
it('should handle the plural expansion form', () => {
let translations: {[key: string]: string} = {};
translations[id(new Message('zero<ph name="e1">bold</ph>', 'plural_=0', null))] =
'ZERO<ph name="e1">BOLD</ph>';
let res = parse(`{messages.length, plural,=0 {zero<b>bold</b>}}`, translations);
expect(humanizeDom(res)).toEqual([
[HtmlElementAst, 'ul', 0],
[HtmlAttrAst, '[ngPlural]', 'messages.length'],
[HtmlElementAst, 'template', 1],
[HtmlAttrAst, 'ngPluralCase', '=0'],
[HtmlElementAst, 'li', 2],
[HtmlTextAst, 'ZERO', 3],
[HtmlElementAst, 'b', 3],
[HtmlTextAst, 'BOLD', 4],
]);
});
it('should handle nested expansion forms', () => {
let translations: {[key: string]: string} = {};
translations[id(new Message('m', 'gender_=m', null))] = 'M';
let res = parse(`{messages.length, plural, =0 { {p.gender, gender, =m {m}} }}`, translations);
expect(humanizeDom(res)).toEqual([
[HtmlElementAst, 'ul', 0], [HtmlAttrAst, '[ngPlural]', 'messages.length'],
[HtmlElementAst, 'template', 1], [HtmlAttrAst, 'ngPluralCase', '=0'],
[HtmlElementAst, 'li', 2],
[HtmlElementAst, 'ul', 3], [HtmlAttrAst, '[ngSwitch]', 'p.gender'],
[HtmlElementAst, 'template', 4], [HtmlAttrAst, 'ngSwitchWhen', '=m'],
[HtmlElementAst, 'li', 5], [HtmlTextAst, 'M', 6],
[HtmlTextAst, ' ', 3]
]);
});
it('should correctly set source code positions', () => {
let translations: {[key: string]: string} = {};
translations[id(new Message('<ph name="e0">bold</ph>', 'plural_=0', null))] =
'<ph name="e0">BOLD</ph>';
let nodes = parse(`{messages.length, plural,=0 {<b>bold</b>}}`, translations).rootNodes;
let ul: HtmlElementAst = <HtmlElementAst>nodes[0];
expect(ul.sourceSpan.start.col).toEqual(0);
expect(ul.sourceSpan.end.col).toEqual(42);
expect(ul.startSourceSpan.start.col).toEqual(0);
expect(ul.startSourceSpan.end.col).toEqual(42);
expect(ul.endSourceSpan.start.col).toEqual(0);
expect(ul.endSourceSpan.end.col).toEqual(42);
let switchExp = ul.attrs[0];
expect(switchExp.sourceSpan.start.col).toEqual(1);
expect(switchExp.sourceSpan.end.col).toEqual(16);
let template: HtmlElementAst = <HtmlElementAst>ul.children[0];
expect(template.sourceSpan.start.col).toEqual(25);
expect(template.sourceSpan.end.col).toEqual(41);
let switchCheck = template.attrs[0];
expect(switchCheck.sourceSpan.start.col).toEqual(25);
expect(switchCheck.sourceSpan.end.col).toEqual(28);
let li: HtmlElementAst = <HtmlElementAst>template.children[0];
expect(li.sourceSpan.start.col).toEqual(25);
expect(li.sourceSpan.end.col).toEqual(41);
let b: HtmlElementAst = <HtmlElementAst>li.children[0];
expect(b.sourceSpan.start.col).toEqual(29);
expect(b.sourceSpan.end.col).toEqual(32);
});
it('should handle other special forms', () => {
let translations: {[key: string]: string} = {};
translations[id(new Message('m', 'gender_=male', null))] = 'M';
let res = parse(`{person.gender, gender,=male {m}}`, translations);
expect(humanizeDom(res)).toEqual([
[HtmlElementAst, 'ul', 0],
[HtmlAttrAst, '[ngSwitch]', 'person.gender'],
[HtmlElementAst, 'template', 1],
[HtmlAttrAst, 'ngSwitchWhen', '=male'],
[HtmlElementAst, 'li', 2],
[HtmlTextAst, 'M', 3],
]);
});
describe('errors', () => {
it('should error when giving an invalid template', () => {
expect(humanizeErrors(parse('<a>a</b>', {}).errors)).toEqual([
@ -311,14 +218,6 @@ export function main() {
humanizeErrors(parse('<div value=\'hi {{a}}\' i18n-value></div>', translations).errors))
.toEqual(['Invalid interpolation name \'99\'']);
});
it('should error on unknown plural cases', () => {
let mid = id(new Message('-', 'plural_unknown', null));
expect(humanizeErrors(parse('{n, plural, unknown {-}}', {mid: ''}).errors)).toEqual([
`Cannot find message for id '${mid}', content '-'.`,
`Plural cases should be "=<number>" or one of zero, one, two, few, many, other`,
]);
});
});
describe('implicit translation', () => {

View File

@ -154,7 +154,9 @@ export function main() {
]);
});
it('should extract messages from expansion forms', () => {
// TODO(vicb) - this should be extracted to a single message
// see https://github.com/angular/angular/issues/9067
xit('should extract messages from expansion forms', () => {
let res = extractor.extract(
`
<div>