From caa466633506df7118710e2e6ac2c7dbf9911988 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Thu, 10 Dec 2020 22:12:29 -0800 Subject: [PATCH] fix(compiler): avoid duplicate i18n blocks for i18n attrs on elements with structural directives (#40077) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently when `ɵɵtemplate` and `ɵɵelement` instructions are generated by compiler, all static attributes are duplicated for both instructions. As a part of this duplication, i18n translation blocks for static i18n attributes are generated twice as well, causing duplicate entries in extracted translation files (when Ivy extraction mechanisms are used). This commit fixes this issue by introducing a cache for i18n translation blocks (for static attributes only). Also this commit further aligns `ɵɵtemplate` and `ɵɵelement` instruction attributes, which should help implement more effective attributes deduplication logic. Closes #39942. PR Close #40077 --- .../element_attributes/GOLDEN_PARTIAL.js | 47 +++++++++++++++++++ .../element_attributes/TEST_CASES.json | 14 ++++++ .../static_attributes_structural.js | 21 +++++++++ .../static_attributes_structural.ts | 15 ++++++ .../compiler/src/render3/view/template.ts | 37 +++++++++++++-- packages/core/test/acceptance/i18n_spec.ts | 21 +++++++++ 6 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/GOLDEN_PARTIAL.js index 0db9de41a2..d726080ed2 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/GOLDEN_PARTIAL.js @@ -394,6 +394,53 @@ export declare class MyModule { static ɵinj: i0.ɵɵInjectorDef; } +/**************************************************************************************************** + * PARTIAL FILE: static_attributes_structural.js + ****************************************************************************************************/ +import { Component, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { + constructor() { + this.exp = true; + } +} +MyComponent.ɵfac = function MyComponent_Factory(t) { return new (t || MyComponent)(); }; +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: { source: ` +
+ `, isInline: true } }); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{ + type: Component, + args: [{ + selector: 'my-component', + template: ` +
+ ` + }] + }], null, null); })(); +export class MyModule { +} +MyModule.ɵmod = i0.ɵɵdefineNgModule({ type: MyModule }); +MyModule.ɵinj = i0.ɵɵdefineInjector({ factory: function MyModule_Factory(t) { return new (t || MyModule)(); } }); +(function () { (typeof ngJitMode === "undefined" || ngJitMode) && i0.ɵɵsetNgModuleScope(MyModule, { declarations: [MyComponent] }); })(); +(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyModule, [{ + type: NgModule, + args: [{ declarations: [MyComponent] }] + }], null, null); })(); + +/**************************************************************************************************** + * PARTIAL FILE: static_attributes_structural.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + exp: boolean; + static ɵfac: i0.ɵɵFactoryDef; + static ɵcmp: i0.ɵɵComponentDefWithMeta; +} +export declare class MyModule { + static ɵmod: i0.ɵɵNgModuleDefWithMeta; + static ɵinj: i0.ɵɵInjectorDef; +} + /**************************************************************************************************** * PARTIAL FILE: interpolation_basic.js ****************************************************************************************************/ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/TEST_CASES.json index 94f8aa18ec..6d84fd7bcc 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/TEST_CASES.json @@ -113,6 +113,20 @@ } ] }, + { + "description": "should translate static attributes when used on an element with structural directive", + "inputFiles": [ + "static_attributes_structural.ts" + ], + "expectations": [ + { + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] + }, { "description": "should support interpolation", "inputFiles": [ diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.js new file mode 100644 index 0000000000..d5553054b6 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.js @@ -0,0 +1,21 @@ +function MyComponent_div_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "div", 1); + } +} +… +consts: function() { + __i18nMsg__('introduction', [], {meaning: 'm', desc: 'd'}) + return [ + ["id", "static", "title", $i18n_0$, __AttributeMarker.Template__, "ngIf"], + ["id", "static", "title", $i18n_0$] + ]; +}, +template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_div_0_Template, 1, 0, "div", 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("ngIf", ctx.exp); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.ts new file mode 100644 index 0000000000..a0adbb98c7 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/static_attributes_structural.ts @@ -0,0 +1,15 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + selector: 'my-component', + template: ` +
+ ` +}) +export class MyComponent { + exp = true; +} + +@NgModule({declarations: [MyComponent]}) +export class MyModule { +} \ No newline at end of file diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 4a96cf91ea..429b1b7938 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -108,15 +108,31 @@ export function prepareEventListenerParameters( } // Collects information needed to generate `consts` field of the ComponentDef. -// When a constant requires some pre-processing, the `prepareStatements` section -// contains corresponding statements. export interface ComponentDefConsts { + /** + * When a constant requires some pre-processing (e.g. i18n translation block that includes + * goog.getMsg and $localize calls), the `prepareStatements` section contains corresponding + * statements. + */ prepareStatements: o.Statement[]; + + /** + * Actual expressions that represent constants. + */ constExpressions: o.Expression[]; + + /** + * Cache to avoid generating duplicated i18n translation blocks. + */ + i18nVarRefsCache: Map; } function createComponentDefConsts(): ComponentDefConsts { - return {prepareStatements: [], constExpressions: []}; + return { + prepareStatements: [], + constExpressions: [], + i18nVarRefsCache: new Map(), + }; } export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver { @@ -1300,7 +1316,20 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // Note that static i18n attributes aren't in the i18n array, // because they're treated in the same way as regular attributes. if (attr.i18n) { - attrExprs.push(o.literal(attr.name), this.i18nTranslate(attr.i18n as i18n.Message)); + // When i18n attributes are present on elements with structural directives + // (e.g. `
`), we want to avoid generating + // duplicate i18n translation blocks for `ɵɵtemplate` and `ɵɵelement` instruction + // attributes. So we do a cache lookup to see if suitable i18n translation block + // already exists. + const {i18nVarRefsCache} = this._constants; + let i18nVarRef: o.ReadVarExpr; + if (i18nVarRefsCache.has(attr.i18n)) { + i18nVarRef = i18nVarRefsCache.get(attr.i18n)!; + } else { + i18nVarRef = this.i18nTranslate(attr.i18n as i18n.Message); + i18nVarRefsCache.set(attr.i18n, i18nVarRef); + } + attrExprs.push(o.literal(attr.name), i18nVarRef); } else { attrExprs.push( ...getAttributeNameLiterals(attr.name), trustedConstAttribute(elementName, attr)); diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index 315e92d448..cc7fb156ce 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -1730,6 +1730,27 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { expect(titleDirInstances[0].title).toBe('Bonjour'); }); + it('should support static i18n attributes on inline templates', () => { + loadTranslations({[computeMsgId('Hello')]: 'Bonjour'}); + @Component({ + selector: 'my-cmp', + template: ` +
+ `, + }) + class Cmp { + } + + TestBed.configureTestingModule({ + imports: [CommonModule], + declarations: [Cmp], + }); + const fixture = TestBed.createComponent(Cmp); + fixture.detectChanges(); + + expect(fixture.nativeElement.firstChild.title).toBe('Bonjour'); + }); + it('should allow directive inputs (as an interpolated prop) on ', () => { loadTranslations({[computeMsgId('Hello {$INTERPOLATION}')]: 'Bonjour {$INTERPOLATION}'});