fix(ivy): trigger directive inputs with i18n translations (#30402)

Changed runtime i18n to define attributes with bindings, or matching directive inputs/outputs as element properties as we are supposed to do in Angular.
This PR fixes the issue where directive inputs wouldn't be trigged.

FW-1315 #resolve
PR Close #30402
This commit is contained in:
Olivier Combe 2019-05-15 14:27:07 +02:00 committed by Matias Niemelä
parent 41f372fe79
commit 91699259b2
5 changed files with 105 additions and 48 deletions

View File

@ -197,7 +197,7 @@ describe('i18n support in the view compiler', () => {
else {
$I18N_0$ = $r3$.ɵɵi18nLocalize("Content A");
}
const $_c2$ = ["title", "Title B"];
const $_c2$ = [${AttributeMarker.Bindings}, "title"];
var $I18N_3$;
if (ngI18nClosureMode) {
/**
@ -211,7 +211,6 @@ describe('i18n support in the view compiler', () => {
$I18N_3$ = $r3$.ɵɵi18nLocalize("Title B");
}
const $_c5$ = ["title", $I18N_3$];
const $_c6$ = ["title", "Title C"];
var $I18N_7$;
if (ngI18nClosureMode) {
/**
@ -224,7 +223,6 @@ describe('i18n support in the view compiler', () => {
$I18N_7$ = $r3$.ɵɵi18nLocalize("Title C");
}
const $_c9$ = ["title", $I18N_7$];
const $_c10$ = ["title", "Title D"];
var $I18N_11$;
if (ngI18nClosureMode) {
/**
@ -238,7 +236,6 @@ describe('i18n support in the view compiler', () => {
$I18N_11$ = $r3$.ɵɵi18nLocalize("Title D");
}
const $_c13$ = ["title", $I18N_11$];
const $_c14$ = ["title", "Title E"];
var $I18N_15$;
if (ngI18nClosureMode) {
/**
@ -251,7 +248,6 @@ describe('i18n support in the view compiler', () => {
$I18N_15$ = $r3$.ɵɵi18nLocalize("Title E");
}
const $_c17$ = ["title", $I18N_15$];
const $_c18$ = ["title", "Title F"];
var $I18N_19$;
if (ngI18nClosureMode) {
const $MSG_EXTERNAL_idF$$APP_SPEC_TS_20$ = goog.getMsg("Title F");
@ -261,7 +257,6 @@ describe('i18n support in the view compiler', () => {
$I18N_19$ = $r3$.ɵɵi18nLocalize("Title F");
}
const $_c21$ = ["title", $I18N_19$];
const $_c22$ = ["title", "Title G"];
var $I18N_23$;
if (ngI18nClosureMode) {
/**
@ -335,7 +330,7 @@ describe('i18n support in the view compiler', () => {
`;
const output = `
const $_c0$ = ["id", "static", "title", "introduction"];
const $_c0$ = ["id", "static", ${AttributeMarker.Bindings}, "title"];
var $I18N_1$;
if (ngI18nClosureMode) {
/**
@ -376,7 +371,7 @@ describe('i18n support in the view compiler', () => {
`;
const output = String.raw `
const $_c0$ = ["id", "dynamic-1", "aria-roledescription", "static text", ${AttributeMarker.Bindings}, "title", "aria-label"];
const $_c0$ = ["id", "dynamic-1", ${AttributeMarker.Bindings}, "aria-roledescription", "title", "aria-label"];
var $I18N_1$;
if (ngI18nClosureMode) {
const $MSG_EXTERNAL_5526535577705876535$$APP_SPEC_TS_1$ = goog.getMsg("static text");
@ -603,8 +598,8 @@ describe('i18n support in the view compiler', () => {
const output = String.raw `
const $_c0$ = [
"id", "dynamic-1", "aria-roledescription", "static text",
${AttributeMarker.Bindings}, "title", "aria-label"
"id", "dynamic-1",
${AttributeMarker.Bindings}, "aria-roledescription", "title", "aria-label"
];
var $I18N_1$;
if (ngI18nClosureMode) {
@ -781,7 +776,7 @@ describe('i18n support in the view compiler', () => {
`;
const output = String.raw `
const $_c0$ = ["title", "Element title"];
const $_c0$ = [${AttributeMarker.Bindings}, "title"];
var $I18N_0$;
if (ngI18nClosureMode) {
/**

View File

@ -518,6 +518,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
const i18nAttrs: (t.TextAttribute | t.BoundAttribute)[] = [];
const outputAttrs: t.TextAttribute[] = [];
const allOtherInputs: (t.TextAttribute | t.BoundAttribute)[] = [];
const [namespaceKey, elementName] = splitNsName(element.name);
const isNgContainer = checkIsNgContainer(element.name);
@ -538,8 +539,14 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// TODO(FW-1248): prevent attributes duplication in `i18nAttributes` and `elementStart`
// arguments
i18nAttrs.push(attr);
// We treat this attribute as if it was a binding so that we don't risk calling inputs
// with the untranslated value.
// Also this generates smaller templates until FW-1248 is fixed.
// TODO(FW-1332): Create an AttributeMarker for i18n attributes
allOtherInputs.push(attr);
} else {
outputAttrs.push(attr);
}
outputAttrs.push(attr);
}
}
@ -554,7 +561,6 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
// Add the attributes
const attributes: o.Expression[] = [];
const allOtherInputs: t.BoundAttribute[] = [];
element.inputs.forEach((input: t.BoundAttribute) => {
const stylingInputWasSet = stylingBuilder.registerBoundInput(input);
@ -1148,7 +1154,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
* because those values are intended to always be generated as property instructions.
*/
private prepareNonRenderAttrs(
inputs: t.BoundAttribute[], outputs: t.BoundEvent[], styles?: StylingBuilder,
inputs: (t.TextAttribute|t.BoundAttribute)[], outputs: t.BoundEvent[],
styles?: StylingBuilder,
templateAttrs: (t.BoundAttribute|t.TextAttribute)[] = []): o.Expression[] {
const alreadySeen = new Set<string>();
const attrExprs: o.Expression[] = [];
@ -1176,7 +1183,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
const attrsStartIndex = attrExprs.length;
for (let i = 0; i < inputs.length; i++) {
const input = inputs[i];
const input = inputs[i] as t.BoundAttribute;
if (input.type !== BindingType.Animation) {
addAttrExpr(input.name);
}

View File

@ -15,8 +15,8 @@ import {addAllToArray} from '../util/array_utils';
import {assertDataInRange, assertDefined, assertEqual, assertGreaterThan} from '../util/assert';
import {attachPatchData} from './context_discovery';
import {attachI18nOpCodesDebug} from './debug';
import {ɵɵelementAttribute, ɵɵload, ɵɵtextBinding} from './instructions/all';
import {allocExpando, getOrCreateTNode} from './instructions/shared';
import {elementAttributeInternal, ɵɵload, ɵɵtextBinding} from './instructions/all';
import {allocExpando, elementPropertyInternal, getOrCreateTNode, setInputsForProperty} from './instructions/shared';
import {LContainer, NATIVE} from './interfaces/container';
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, IcuType, TI18n, TIcu} from './interfaces/i18n';
import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
@ -30,6 +30,7 @@ import {NO_CHANGE} from './tokens';
import {renderStringify} from './util/misc_utils';
import {getNativeByIndex, getNativeByTNode, getTNode, isLContainer} from './util/view_utils';
const MARKER = `<EFBFBD>`;
const ICU_BLOCK_REGEXP = /^\s*(<28>\d+:?\d*<2A>)\s*,\s*(select|plural)\s*,/;
const SUBTEMPLATE_REGEXP = /<2F>\/?\*(\d+:\d+)<29>/gi;
@ -735,7 +736,10 @@ function readCreateOpCodes(
const elementNodeIndex = opCode >>> I18nMutateOpCode.SHIFT_REF;
const attrName = createOpCodes[++i] as string;
const attrValue = createOpCodes[++i] as string;
ɵɵelementAttribute(elementNodeIndex, attrName, attrValue);
const renderer = viewData[RENDERER];
// This code is used for ICU expressions only, since we don't support
// directives/components in ICUs, we don't need to worry about inputs here
elementAttributeInternal(elementNodeIndex, attrName, attrValue, viewData, renderer);
break;
default:
throw new Error(`Unable to determine the type of mutate operation for "${opCode}"`);
@ -810,9 +814,9 @@ function readUpdateOpCodes(
let icuTNode: TIcuContainerNode;
switch (opCode & I18nUpdateOpCode.MASK_OPCODE) {
case I18nUpdateOpCode.Attr:
const attrName = updateOpCodes[++j] as string;
const propName = updateOpCodes[++j] as string;
const sanitizeFn = updateOpCodes[++j] as SanitizerFn | null;
ɵɵelementAttribute(nodeIndex, attrName, value, sanitizeFn);
elementPropertyInternal(nodeIndex, propName, value, sanitizeFn);
break;
case I18nUpdateOpCode.Text:
ɵɵtextBinding(nodeIndex, value);
@ -954,6 +958,7 @@ function i18nAttributesFirstPass(tView: TView, index: number, values: string[])
if (j & 1) {
// Odd indexes are ICU expressions
// TODO(ocombe): support ICU expressions in attributes
throw new Error('ICU expressions are not yet supported in attributes');
} else if (value !== '') {
// Even indexes are text (including bindings)
const hasBinding = !!value.match(BINDING_REGEXP);
@ -961,7 +966,15 @@ function i18nAttributesFirstPass(tView: TView, index: number, values: string[])
addAllToArray(
generateBindingUpdateOpCodes(value, previousElementIndex, attrName), updateOpCodes);
} else {
ɵɵelementAttribute(previousElementIndex, attrName, value);
const lView = getLView();
const renderer = lView[RENDERER];
elementAttributeInternal(previousElementIndex, attrName, value, lView, renderer);
// Check if that attribute is a directive input
const tNode = getTNode(previousElementIndex, lView);
const dataValue = tNode.inputs && tNode.inputs[attrName];
if (dataValue) {
setInputsForProperty(lView, dataValue, value);
}
}
}
}

View File

@ -11,10 +11,10 @@ import {assertHasParent} from '../assert';
import {attachPatchData} from '../context_discovery';
import {registerPostOrderHooks} from '../hooks';
import {TAttributes, TNodeFlags, TNodeType} from '../interfaces/node';
import {RElement, isProceduralRenderer} from '../interfaces/renderer';
import {RElement, Renderer3, isProceduralRenderer} from '../interfaces/renderer';
import {SanitizerFn} from '../interfaces/sanitization';
import {StylingContext} from '../interfaces/styling';
import {BINDING_INDEX, HEADER_OFFSET, QUERIES, RENDERER, TVIEW, T_HOST} from '../interfaces/view';
import {BINDING_INDEX, HEADER_OFFSET, LView, QUERIES, RENDERER, TVIEW, T_HOST} from '../interfaces/view';
import {assertNodeType} from '../node_assert';
import {appendChild} from '../node_manipulation';
import {applyOnCreateInstructions} from '../node_util';
@ -27,12 +27,10 @@ import {NO_CHANGE} from '../tokens';
import {attrsStylingIndexOf, setUpAttributes} from '../util/attrs_utils';
import {renderStringify} from '../util/misc_utils';
import {getNativeByIndex, getNativeByTNode, getTNode} from '../util/view_utils';
import {createDirectivesAndLocals, elementCreate, executeContentQueries, getOrCreateTNode, initializeTNodeInputs, setInputsForProperty, setNodeStylingTemplate} from './shared';
import {getActiveDirectiveStylingIndex} from './styling';
/**
* Create DOM element. The instruction must later be followed by `elementEnd()` call.
*
@ -202,7 +200,7 @@ export function ɵɵelement(
/**
* Updates the value or removes an attribute on an Element.
*
* @param number index The index of the element in the data array
* @param index The index of the element in the data array
* @param name name The name of the attribute.
* @param value value The attribute is removed when value is `null` or `undefined`.
* Otherwise the attribute value is set to the stringified value.
@ -215,27 +213,33 @@ export function ɵɵelementAttribute(
index: number, name: string, value: any, sanitizer?: SanitizerFn | null,
namespace?: string): void {
if (value !== NO_CHANGE) {
ngDevMode && validateAgainstEventAttributes(name);
const lView = getLView();
const renderer = lView[RENDERER];
const element = getNativeByIndex(index, lView) as RElement;
if (value == null) {
ngDevMode && ngDevMode.rendererRemoveAttribute++;
isProceduralRenderer(renderer) ? renderer.removeAttribute(element, name, namespace) :
element.removeAttribute(name);
elementAttributeInternal(index, name, value, lView, renderer, sanitizer, namespace);
}
}
export function elementAttributeInternal(
index: number, name: string, value: any, lView: LView, renderer: Renderer3,
sanitizer?: SanitizerFn | null, namespace?: string) {
ngDevMode && validateAgainstEventAttributes(name);
const element = getNativeByIndex(index, lView) as RElement;
if (value == null) {
ngDevMode && ngDevMode.rendererRemoveAttribute++;
isProceduralRenderer(renderer) ? renderer.removeAttribute(element, name, namespace) :
element.removeAttribute(name);
} else {
ngDevMode && ngDevMode.rendererSetAttribute++;
const tNode = getTNode(index, lView);
const strValue =
sanitizer == null ? renderStringify(value) : sanitizer(value, tNode.tagName || '', name);
if (isProceduralRenderer(renderer)) {
renderer.setAttribute(element, name, strValue, namespace);
} else {
ngDevMode && ngDevMode.rendererSetAttribute++;
const tNode = getTNode(index, lView);
const strValue =
sanitizer == null ? renderStringify(value) : sanitizer(value, tNode.tagName || '', name);
if (isProceduralRenderer(renderer)) {
renderer.setAttribute(element, name, strValue, namespace);
} else {
namespace ? element.setAttributeNS(namespace, name, strValue) :
element.setAttribute(name, strValue);
}
namespace ? element.setAttributeNS(namespace, name, strValue) :
element.setAttribute(name, strValue);
}
}
}

View File

@ -8,7 +8,7 @@
import {registerLocaleData} from '@angular/common';
import localeRo from '@angular/common/locales/ro';
import {Component, ContentChild, ContentChildren, Directive, HostBinding, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, ɵi18nConfigureLocalize} from '@angular/core';
import {Component, ContentChild, ContentChildren, Directive, HostBinding, Input, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, ɵi18nConfigureLocalize} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {onlyInIvy} from '@angular/private/testing';
@ -625,14 +625,14 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
{translations: {'hello {$interpolation}': 'bonjour {$interpolation}'}});
const fixture = initWithTemplate(
AppComp,
`<div i18n i18n-title title="hello {{name}}" i18n-aria-label aria-label="hello {{name}}"></div>`);
`<input i18n i18n-title title="hello {{name}}" i18n-placeholder placeholder="hello {{name}}">`);
expect(fixture.nativeElement.innerHTML)
.toEqual(`<div title="bonjour Angular" aria-label="bonjour Angular"></div>`);
.toEqual(`<input title="bonjour Angular" placeholder="bonjour Angular">`);
fixture.componentRef.instance.name = 'John';
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML)
.toEqual(`<div title="bonjour John" aria-label="bonjour John"></div>`);
.toEqual(`<input title="bonjour John" placeholder="bonjour John">`);
});
it('on removed elements', () => {
@ -732,6 +732,44 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
`<div test="" title="début 3 milieu 2 fin" class="bar"> traduction: 2 emails<!--ICU 19--></div><div test="" class="bar"></div>`);
});
it('should handle i18n attribute with directive inputs', () => {
let calledTitle = false;
let calledValue = false;
@Component({selector: 'my-comp', template: ''})
class MyComp {
t !: string;
@Input()
get title() { return this.t; }
set title(title) {
calledTitle = true;
this.t = title;
}
@Input()
get value() { return this.val; }
set value(value: string) {
calledValue = true;
this.val = value;
}
val !: string;
}
TestBed.configureTestingModule({declarations: [AppComp, MyComp]});
ɵi18nConfigureLocalize({
translations: {'Hello {$interpolation}': 'Bonjour {$interpolation}', 'works': 'fonctionne'}
});
const fixture = initWithTemplate(
AppComp,
`<my-comp i18n i18n-title title="works" i18n-value="hi" value="Hello {{name}}"></my-comp>`);
fixture.detectChanges();
const directive = fixture.debugElement.children[0].injector.get(MyComp);
expect(calledValue).toEqual(true);
expect(calledTitle).toEqual(true);
expect(directive.value).toEqual(`Bonjour Angular`);
expect(directive.title).toEqual(`fonctionne`);
});
it('should support adding/moving/removing nodes', () => {
ɵi18nConfigureLocalize({
translations: {