Skip to content

Commit

Permalink
fix(ivy): process property bindings in i18n blocks similar to non-i18…
Browse files Browse the repository at this point in the history 
…n bindings (#28969)

Prior to this change i18n block bindings were converted to Expressions right away (once we first access them), when in non-i18n cases we processed them differently: the actual conversion happens at instructions generation. Because of this discrepancy, the output for bindings in i18n blocks was generated incorrectly (with invalid indicies in pipeBindN fns and invalid references to non-existent local variables). Now the bindings processing is unified and i18nExp instructions should contain right bind expressions.

PR Close #28969
  • Loading branch information
@AndrewKushnir @benlesh
AndrewKushnir authored and benlesh committed Feb 27, 2019
1 parent 34bdebc commit 40833ba
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 25 deletions.
53 changes: 42 additions & 11 deletions packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,37 @@ describe('i18n support in the view compiler', () => {
verify(input, output, {inputArgs: {interpolation: ['{%', '%}']}});
});

it('should support interpolations with complex expressions', () => {
const input = `
<div i18n>
{{ valueA | async }}
{{ valueA?.a?.b }}
</div>
`;

const output = String.raw `
const $MSG_EXTERNAL_1482713963707913023$$APP_SPEC_TS_0$ = goog.getMsg(" {$interpolation} {$interpolation_1} ", {
"interpolation": "\uFFFD0\uFFFD",
"interpolation_1": "\uFFFD1\uFFFD"
});
…
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵelementStart(0, "div");
$r3$.ɵi18n(1, $MSG_EXTERNAL_1482713963707913023$$APP_SPEC_TS_0$);
$r3$.ɵpipe(2, "async");
$r3$.ɵelementEnd();
}
if (rf & 2) {
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(2, 2, ctx.valueA)));
$r3$.ɵi18nExp($r3$.ɵbind(((ctx.valueA == null) ? null : ((ctx.valueA.a == null) ? null : ctx.valueA.a.b))));
$r3$.ɵi18nApply(1);
}
}
`;
verify(input, output);
});

it('should handle i18n attributes with bindings in content', () => {
const input = `
<div i18n>My i18n block #{{ one }}</div>
Expand Down Expand Up @@ -800,7 +831,7 @@ describe('i18n support in the view compiler', () => {
if (rf & 2) {
$r3$.ɵi18nExp($r3$.ɵbind(ctx.one));
$r3$.ɵi18nApply(1);
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 0, ctx.two)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 3, ctx.two)));
$r3$.ɵi18nApply(3);
$r3$.ɵi18nExp($r3$.ɵbind(((ctx.three + ctx.four) + ctx.five)));
$r3$.ɵi18nApply(6);
Expand Down Expand Up @@ -868,7 +899,7 @@ describe('i18n support in the view compiler', () => {
if (rf & 2) {
$r3$.ɵi18nExp($r3$.ɵbind(ctx.one));
$r3$.ɵi18nApply(1);
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(5, 0, ctx.two)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(5, 3, ctx.two)));
$r3$.ɵi18nExp($r3$.ɵbind(ctx.nestedInBlockTwo));
$r3$.ɵi18nApply(4);
}
Expand Down Expand Up @@ -941,7 +972,7 @@ describe('i18n support in the view compiler', () => {
$r3$.ɵi18nApply(1);
$r3$.ɵi18nExp($r3$.ɵbind(ctx.valueE));
$r3$.ɵi18nApply(8);
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(6, 0, ctx.valueD)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(6, 5, ctx.valueD)));
$r3$.ɵi18nApply(5);
}
}
Expand Down Expand Up @@ -988,7 +1019,7 @@ describe('i18n support in the view compiler', () => {
if (rf & 2) {
const $ctx_r0$ = $r3$.ɵnextContext();
$r3$.ɵi18nExp($r3$.ɵbind($ctx_r0$.valueA));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 0, $ctx_r0$.valueB)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 2, $ctx_r0$.valueB)));
$r3$.ɵi18nApply(2);
}
}
Expand Down Expand Up @@ -1118,7 +1149,7 @@ describe('i18n support in the view compiler', () => {
const $ctx_r0$ = $r3$.ɵnextContext();
$r3$.ɵelementProperty(4, "ngIf", $r3$.ɵbind($ctx_r0$.exists));
$r3$.ɵi18nExp($r3$.ɵbind($ctx_r0$.valueA));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(3, 0, $ctx_r0$.valueB)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(3, 3, $ctx_r0$.valueB)));
$r3$.ɵi18nApply(0);
}
}
Expand Down Expand Up @@ -1148,7 +1179,7 @@ describe('i18n support in the view compiler', () => {
if (rf & 2) {
const $ctx_r1$ = $r3$.ɵnextContext();
$r3$.ɵi18nExp($r3$.ɵbind(($ctx_r1$.valueE + $ctx_r1$.valueF)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(3, 0, $ctx_r1$.valueG)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(3, 2, $ctx_r1$.valueG)));
$r3$.ɵi18nApply(0);
}
}
Expand Down Expand Up @@ -1347,7 +1378,7 @@ describe('i18n support in the view compiler', () => {
$r3$.ɵelementContainerEnd();
}
if (rf & 2) {
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(2, 0, ctx.valueA)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(2, 1, ctx.valueA)));
$r3$.ɵi18nApply(1);
}
}
Expand All @@ -1371,7 +1402,7 @@ describe('i18n support in the view compiler', () => {
$r3$.ɵpipe(1, "uppercase");
} if (rf & 2) {
const $ctx_r0$ = $r3$.ɵnextContext();
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 0, $ctx_r0$.valueA)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 1, $ctx_r0$.valueA)));
$r3$.ɵi18nApply(0);
}
}
Expand Down Expand Up @@ -1412,7 +1443,7 @@ describe('i18n support in the view compiler', () => {
}
if (rf & 2) {
const $ctx_r0$ = $r3$.ɵnextContext();
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 0, $ctx_r0$.valueA)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 1, $ctx_r0$.valueA)));
$r3$.ɵi18nApply(0);
}
}
Expand All @@ -1431,7 +1462,7 @@ describe('i18n support in the view compiler', () => {
$r3$.ɵelementEnd();
}
if (rf & 2) {
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 0, ctx.valueB)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(4, 1, ctx.valueB)));
$r3$.ɵi18nApply(1);
}
}
Expand Down Expand Up @@ -1540,7 +1571,7 @@ describe('i18n support in the view compiler', () => {
}
if (rf & 2) {
const $ctx_r0$ = $r3$.ɵnextContext();
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 0, $ctx_r0$.valueA)));
$r3$.ɵi18nExp($r3$.ɵbind($r3$.ɵpipeBind1(1, 1, $ctx_r0$.valueA)));
$r3$.ɵi18nApply(0);
}
}
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler/src/render3/view/i18n/context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST} from '../../../expression_parser/ast';
import * as i18n from '../../../i18n/i18n_ast';
import * as o from '../../../output/output_ast';

Expand Down Expand Up @@ -40,7 +41,7 @@ function setupRegistry() {
*/
export class I18nContext {
public readonly id: number;
public bindings = new Set<o.Expression>();
public bindings = new Set<AST>();
public placeholders = new Map<string, any[]>();
public isEmitted: boolean = false;

Expand Down Expand Up @@ -75,7 +76,7 @@ export class I18nContext {
}

// public API to accumulate i18n-related content
appendBinding(binding: o.Expression) { this.bindings.add(binding); }
appendBinding(binding: AST) { this.bindings.add(binding); }
appendIcu(name: string, ref: o.Expression) {
updatePlaceholderMap(this._registry.icus, name, ref);
}
Expand Down
15 changes: 8 additions & 7 deletions packages/compiler/src/render3/view/template.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
}

i18nAppendBindings(expressions: AST[]) {
if (!this.i18n || !expressions.length) return;
const implicit = o.variable(CONTEXT_NAME);
expressions.forEach(expression => {
const binding = this.convertExpressionBinding(implicit, expression);
this.i18n !.appendBinding(binding);
});
if (expressions.length > 0) {
expressions.forEach(expression => this.i18n !.appendBinding(expression));
}
}

i18nBindProps(props: {[key: string]: t.Text | t.BoundText}): {[key: string]: o.Expression} {
Expand Down Expand Up @@ -452,7 +449,11 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// setup accumulated bindings
const {index, bindings} = this.i18n;
if (bindings.size) {
bindings.forEach(binding => this.updateInstruction(span, R3.i18nExp, [binding]));
bindings.forEach(binding => {
this.updateInstruction(
span, R3.i18nExp,
() => [this.convertPropertyBinding(o.variable(CONTEXT_NAME), binding)]);
});
this.updateInstruction(span, R3.i18nApply, [o.literal(index)]);
}
if (!selfClosing) {
Expand Down
14 changes: 9 additions & 5 deletions packages/compiler/test/render3/view/i18n_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST} from '../../../src/expression_parser/ast';
import {Lexer} from '../../../src/expression_parser/lexer';
import {Parser} from '../../../src/expression_parser/parser';
import * as i18n from '../../../src/i18n/i18n_ast';
import * as o from '../../../src/output/output_ast';
import * as t from '../../../src/render3/r3_ast';
Expand All @@ -15,6 +18,7 @@ import {I18nMeta, formatI18nPlaceholderName, parseI18nMeta} from '../../../src/r

import {parseR3 as parse} from './util';

const expressionParser = new Parser(new Lexer());
const i18nOf = (element: t.Node & {i18n?: i18n.AST}) => element.i18n !;

describe('I18nContext', () => {
Expand Down Expand Up @@ -44,8 +48,8 @@ describe('I18nContext', () => {

// binding collection checks
expect(ctx.bindings.size).toBe(0);
ctx.appendBinding(o.literal(1));
ctx.appendBinding(o.literal(2));
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '') as AST);
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '') as AST);
expect(ctx.bindings.size).toBe(2);
});

Expand All @@ -72,7 +76,7 @@ describe('I18nContext', () => {

// set data for root ctx
ctx.appendBoundText(i18nOf(boundTextA));
ctx.appendBinding(o.literal('valueA'));
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '') as AST);
ctx.appendElement(i18nOf(elementA), 0);
ctx.appendTemplate(i18nOf(templateA), 1);
ctx.appendElement(i18nOf(elementA), 0, true);
Expand All @@ -88,11 +92,11 @@ describe('I18nContext', () => {
// set data for child context
childCtx.appendElement(i18nOf(elementB), 0);
childCtx.appendBoundText(i18nOf(boundTextB));
childCtx.appendBinding(o.literal('valueB'));
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '') as AST);
childCtx.appendElement(i18nOf(elementC), 1);
childCtx.appendElement(i18nOf(elementC), 1, true);
childCtx.appendBoundText(i18nOf(boundTextC));
childCtx.appendBinding(o.literal('valueC'));
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '') as AST);
childCtx.appendElement(i18nOf(elementB), 0, true);

expect(childCtx.bindings.size).toBe(2);
Expand Down
9 changes: 9 additions & 0 deletions packages/core/test/i18n_integration_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class DirectiveWithTplRef {
class MyComp {
name = 'John';
items = ['1', '2', '3'];
obj = {a: {b: 'value'}};
visible = true;
age = 20;
count = 2;
Expand All @@ -42,6 +43,7 @@ const TRANSLATIONS: any = {
'Item {$interpolation}': 'Article {$interpolation}',
'\'Single quotes\' and "Double quotes"': '\'Guillemets simples\' et "Guillemets doubles"',
'My logo': 'Mon logo',
'{$interpolation} - {$interpolation_1}': '{$interpolation} - {$interpolation_1} (fr)',
'{$startTagSpan}My logo{$tagImg}{$closeTagSpan}':
'{$startTagSpan}Mon logo{$tagImg}{$closeTagSpan}',
'{$startTagNgTemplate} Hello {$closeTagNgTemplate}{$startTagNgContainer} Bye {$closeTagNgContainer}':
Expand Down Expand Up @@ -175,6 +177,13 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
expect(element).toHaveText('Bonjour John');
});

it('should support interpolations with complex expressions', () => {
const template = `<div i18n>{{ name | uppercase }} - {{ obj?.a?.b }}</div>`;
const fixture = getFixtureWithOverrides({template});
const element = fixture.nativeElement.firstChild;
expect(element).toHaveText('JOHN - value (fr)');
});

it('should properly escape quotes in content', () => {
const content = `'Single quotes' and "Double quotes"`;
const template = `<div i18n>${content}</div>`;
Expand Down

0 comments on commit 40833ba

Please sign in to comment.