From 40da51f641657baaa0d6d4a50cf01871516ae691 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 25 Feb 2020 22:59:34 -0800 Subject: [PATCH] fix(compiler): support i18n attributes on `` tags (#35681) Prior to this commit, i18n attributes defined on `` tags were not processed by the compiler. This commit adds the necessary logic to handle i18n attributes in the same way how these attrs are processed for regular elements. PR Close #35681 --- .../compliance/r3_view_compiler_i18n_spec.ts | 72 +++++++++++++ .../compiler/src/render3/view/template.ts | 100 ++++++++++-------- packages/core/test/acceptance/i18n_spec.ts | 65 ++++++++++++ 3 files changed, 191 insertions(+), 46 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index 556d3385d7..9b449b49f9 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -331,6 +331,78 @@ describe('i18n support in the template compiler', () => { verify(input, output); }); + it('should support i18n attributes on explicit elements', () => { + const input = ` + + `; + + // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form + // into the `consts` array on Component def. + const output = String.raw ` + var $I18N_0$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + const $MSG_EXTERNAL_6616505470450179563$$APP_SPEC_TS_1$ = goog.getMsg("Hello"); + $I18N_0$ = $MSG_EXTERNAL_6616505470450179563$$APP_SPEC_TS_1$; + } + else { + $I18N_0$ = $localize \`Hello\`; + } + const $_c2$ = ["title", $I18N_0$]; + … + consts: [["title", "Hello"]], + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0); + $r3$.ɵɵi18nAttributes(1, $_c2$); + } + } + `; + verify(input, output); + }); + + it('should support i18n attributes on explicit with structural directives', + () => { + const input = ` + Test + `; + + // TODO (FW-1942): update the code to avoid adding `title` attribute in plain form + // into the `consts` array on Component def. + const output = String.raw ` + var $I18N_0$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + const $MSG_EXTERNAL_6616505470450179563$$APP_SPEC_TS_1$ = goog.getMsg("Hello"); + $I18N_0$ = $MSG_EXTERNAL_6616505470450179563$$APP_SPEC_TS_1$; + } + else { + $I18N_0$ = $localize \`Hello\`; + } + const $_c2$ = ["title", $I18N_0$]; + function MyComponent_0_ng_template_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtext(0, "Test"); + } + } + function MyComponent_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_ng_template_0_Template, 1, 0, "ng-template", 1); + $r3$.ɵɵi18nAttributes(1, $_c2$); + } + } + … + consts: [[${AttributeMarker.Template}, "ngIf"], ["title", "Hello"]], + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 0, undefined, 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("ngIf", ctx.visible); + } + } + `; + verify(input, output); + }); + it('should not create translations for empty attributes', () => { const input = `
diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index d252d4532d..abea5c6fbe 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -472,6 +472,46 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.i18n = null; // reset local i18n context } + private i18nAttributesInstruction( + nodeIndex: number, attrs: (t.TextAttribute|t.BoundAttribute)[], + sourceSpan: ParseSourceSpan): void { + let hasBindings: boolean = false; + const i18nAttrArgs: o.Expression[] = []; + const bindings: ChainableBindingInstruction[] = []; + attrs.forEach(attr => { + const message = attr.i18n !as i18n.Message; + if (attr instanceof t.TextAttribute) { + i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message)); + } else { + const converted = attr.value.visit(this._valueConverter); + this.allocateBindingSlots(converted); + if (converted instanceof Interpolation) { + const placeholders = assembleBoundTextPlaceholders(message); + const params = placeholdersToParams(placeholders); + i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message, params)); + converted.expressions.forEach(expression => { + hasBindings = true; + bindings.push({ + sourceSpan, + value: () => this.convertPropertyBinding(expression), + }); + }); + } + } + }); + if (bindings.length > 0) { + this.updateInstructionChainWithAdvance(nodeIndex, R3.i18nExp, bindings); + } + if (i18nAttrArgs.length > 0) { + const index: o.Expression = o.literal(this.allocateDataSlot()); + const args = this.constantPool.getConstLiteral(o.literalArr(i18nAttrArgs), true); + this.creationInstruction(sourceSpan, R3.i18nAttributes, [index, args]); + if (hasBindings) { + this.updateInstruction(sourceSpan, R3.i18nApply, [index]); + } + } + } + private getNamespaceInstruction(namespaceKey: string|null) { switch (namespaceKey) { case 'math': @@ -548,10 +588,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver stylingBuilder.registerClassAttr(value); } else { if (attr.i18n) { - // Place attributes into a separate array for i18n processing, but also keep such - // attributes in the main list to make them available for directive matching at runtime. - // TODO(FW-1248): prevent attributes duplication in `i18nAttributes` and `elementStart` - // arguments i18nAttrs.push(attr); } else { outputAttrs.push(attr); @@ -575,10 +611,6 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const stylingInputWasSet = stylingBuilder.registerBoundInput(input); if (!stylingInputWasSet) { if (input.type === BindingType.Property && input.i18n) { - // Place attributes into a separate array for i18n processing, but also keep such - // attributes in the main list to make them available for directive matching at runtime. - // TODO(FW-1248): prevent attributes duplication in `i18nAttributes` and `elementStart` - // arguments i18nAttrs.push(input); } else { allOtherInputs.push(input); @@ -631,43 +663,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.creationInstruction(element.sourceSpan, R3.disableBindings); } - // process i18n element attributes - if (i18nAttrs.length) { - let hasBindings: boolean = false; - const i18nAttrArgs: o.Expression[] = []; - const bindings: ChainableBindingInstruction[] = []; - i18nAttrs.forEach(attr => { - const message = attr.i18n !as i18n.Message; - if (attr instanceof t.TextAttribute) { - i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message)); - } else { - const converted = attr.value.visit(this._valueConverter); - this.allocateBindingSlots(converted); - if (converted instanceof Interpolation) { - const placeholders = assembleBoundTextPlaceholders(message); - const params = placeholdersToParams(placeholders); - i18nAttrArgs.push(o.literal(attr.name), this.i18nTranslate(message, params)); - converted.expressions.forEach(expression => { - hasBindings = true; - bindings.push({ - sourceSpan: element.sourceSpan, - value: () => this.convertPropertyBinding(expression) - }); - }); - } - } - }); - if (bindings.length) { - this.updateInstructionChainWithAdvance(elementIndex, R3.i18nExp, bindings); - } - if (i18nAttrArgs.length) { - const index: o.Expression = o.literal(this.allocateDataSlot()); - const args = this.constantPool.getConstLiteral(o.literalArr(i18nAttrArgs), true); - this.creationInstruction(element.sourceSpan, R3.i18nAttributes, [index, args]); - if (hasBindings) { - this.updateInstruction(element.sourceSpan, R3.i18nApply, [index]); - } - } + if (i18nAttrs.length > 0) { + this.i18nAttributesInstruction(elementIndex, i18nAttrs, element.sourceSpan); } // Generate Listeners (outputs) @@ -850,6 +847,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver this.matchDirectives(NG_TEMPLATE_TAG_NAME, template); // prepare attributes parameter (including attributes used for directive matching) + // TODO (FW-1942): exclude i18n attributes from the main attribute list and pass them + // as an `i18nAttrs` argument of the `getAttributeExpressions` function below. const attrsExprs: o.Expression[] = this.getAttributeExpressions( template.attributes, template.inputs, template.outputs, undefined, template.templateAttrs, undefined); @@ -894,8 +893,17 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // handle property bindings e.g. ɵɵproperty('ngForOf', ctx.items), et al; this.templatePropertyBindings(templateIndex, template.templateAttrs); - // Only add normal input/output binding instructions on explicit ng-template elements. + // Only add normal input/output binding instructions on explicit elements. if (template.tagName === NG_TEMPLATE_TAG_NAME) { + // Add i18n attributes that may act as inputs to directives. If such attributes are present, + // generate `i18nAttributes` instruction. Note: we generate it only for explicit + // elements, in case of inline templates, corresponding instructions will be generated in the + // nested template function. + const i18nAttrs: t.TextAttribute[] = template.attributes.filter(attr => !!attr.i18n); + if (i18nAttrs.length > 0) { + this.i18nAttributesInstruction(templateIndex, i18nAttrs, template.sourceSpan); + } + // Add the input bindings this.templatePropertyBindings(templateIndex, template.inputs); // Generate listeners for directive output diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index 37712a7aea..1a45e7cacc 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -1358,6 +1358,71 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { expect(element.title).toBe('Bonjour Angular'); }); + it('should process i18n attributes on explicit elements', () => { + const titleDirInstances: TitleDir[] = []; + loadTranslations({[computeMsgId('Hello')]: 'Bonjour'}); + + @Directive({ + selector: '[title]', + }) + class TitleDir { + @Input() title = ''; + constructor() { titleDirInstances.push(this); } + } + + @Component({ + selector: 'comp', + template: '', + }) + class Comp { + } + + TestBed.configureTestingModule({ + declarations: [Comp, TitleDir], + }); + + const fixture = TestBed.createComponent(Comp); + fixture.detectChanges(); + + // make sure we only match `TitleDir` once + expect(titleDirInstances.length).toBe(1); + + expect(titleDirInstances[0].title).toBe('Bonjour'); + }); + + it('should match directive only once in case i18n attrs are present on inline template', () => { + const titleDirInstances: TitleDir[] = []; + loadTranslations({[computeMsgId('Hello')]: 'Bonjour'}); + + @Directive({selector: '[title]'}) + class TitleDir { + @Input() title: string = ''; + constructor(public elRef: ElementRef) { titleDirInstances.push(this); } + } + + @Component({ + selector: 'my-cmp', + template: ` + + `, + }) + class Cmp { + } + + TestBed.configureTestingModule({ + imports: [CommonModule], + declarations: [Cmp, TitleDir], + }); + const fixture = TestBed.createComponent(Cmp); + fixture.detectChanges(); + + // make sure we only match `TitleDir` once and on the right element + expect(titleDirInstances.length).toBe(1); + expect(titleDirInstances[0].elRef.nativeElement instanceof HTMLButtonElement).toBeTruthy(); + + expect(titleDirInstances[0].title).toBe('Bonjour'); + }); + it('should apply i18n attributes during second template pass', () => { loadTranslations({[computeMsgId('Set')]: 'Set'}); @Directive({