Skip to content

Commit

Permalink
fix(compiler): avoid duplicate i18n blocks for i18n attrs on elements…
Browse files Browse the repository at this point in the history
… with structural directives (#40077)

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
  • Loading branch information
AndrewKushnir authored and alxhub committed Dec 15, 2020
1 parent 245dccc commit caa4666
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,53 @@ export declare class MyModule {
static ɵinj: i0.ɵɵInjectorDef<MyModule>;
}

/****************************************************************************************************
* 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: `
<div *ngIf="exp" id="static" i18n-title="m|d" title="introduction"></div>
`, isInline: true } });
(function () { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadata(MyComponent, [{
type: Component,
args: [{
selector: 'my-component',
template: `
<div *ngIf="exp" id="static" i18n-title="m|d" title="introduction"></div>
`
}]
}], 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<MyComponent, never>;
static ɵcmp: i0.ɵɵComponentDefWithMeta<MyComponent, "my-component", never, {}, {}, never, never>;
}
export declare class MyModule {
static ɵmod: i0.ɵɵNgModuleDefWithMeta<MyModule, [typeof MyComponent], never, never>;
static ɵinj: i0.ɵɵInjectorDef<MyModule>;
}

/****************************************************************************************************
* PARTIAL FILE: interpolation_basic.js
****************************************************************************************************/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {Component, NgModule} from '@angular/core';

@Component({
selector: 'my-component',
template: `
<div *ngIf="exp" id="static" i18n-title="m|d" title="introduction"></div>
`
})
export class MyComponent {
exp = true;
}

@NgModule({declarations: [MyComponent]})
export class MyModule {
}
37 changes: 33 additions & 4 deletions packages/compiler/src/render3/view/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<i18n.I18nMeta, o.ReadVarExpr>;
}

function createComponentDefConsts(): ComponentDefConsts {
return {prepareStatements: [], constExpressions: []};
return {
prepareStatements: [],
constExpressions: [],
i18nVarRefsCache: new Map(),
};
}

export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver {
Expand Down Expand Up @@ -1300,7 +1316,20 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, 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. `<div *ngIf title="Hello" i18n-title>`), 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));
Expand Down
21 changes: 21 additions & 0 deletions packages/core/test/acceptance/i18n_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
<div *ngIf="true" i18n-title title="Hello"></div>
`,
})
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 <ng-template>', () => {
loadTranslations({[computeMsgId('Hello {$INTERPOLATION}')]: 'Bonjour {$INTERPOLATION}'});

Expand Down

0 comments on commit caa4666

Please sign in to comment.