From b4c5bdb093b38f41f160c2f17c47ef13956cb6ce Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 16 Dec 2019 11:55:50 -0800 Subject: [PATCH] fix(ivy): append `advance` instructions before `i18nExp` (#34436) Prior to this commit, there were no `advance` instructions generated before `i18nExp` instructions and as a result, lifecycle hooks for components used inside i18n blocks were flushed too late. This commit adds the logic to generate `advance` instructions in front of `i18nExp` ones (similar to what we have in other places like interpolations, property bindings, etc), so that the necessary lifecycle hooks are flushed before expression value is captured. PR Close #34436 --- .../compliance/r3_view_compiler_i18n_spec.ts | 45 +++++++++++++++- .../compiler/src/render3/view/template.ts | 7 ++- packages/core/test/acceptance/i18n_spec.ts | 51 ++++++++++++++++++- 3 files changed, 99 insertions(+), 4 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 78de4a6099..1d4c89145f 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 @@ -513,6 +513,7 @@ describe('i18n support in the template compiler', () => { if (rf & 2) { $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 6, ctx.valueA))(ctx.valueB); $r3$.ɵɵi18nApply(2); + $r3$.ɵɵadvance(3); $r3$.ɵɵi18nExp(ctx.valueA)(ctx.valueB)(ctx.valueA + ctx.valueB)(ctx.valueC); $r3$.ɵɵi18nApply(4); } @@ -597,6 +598,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $outer_r1$ = ctx.$implicit; + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, 1, $outer_r1$)); $r3$.ɵɵi18nApply(3); } @@ -767,6 +769,7 @@ describe('i18n support in the template compiler', () => { if (rf & 2) { $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 6, ctx.valueA))(ctx.valueB); $r3$.ɵɵi18nApply(2); + $r3$.ɵɵadvance(3); $r3$.ɵɵi18nExp(ctx.valueA)(ctx.valueB)(ctx.valueA + ctx.valueB)(ctx.valueC); $r3$.ɵɵi18nApply(4); } @@ -811,6 +814,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $outer_r1$ = ctx.$implicit; + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, 1, $outer_r1$)); $r3$.ɵɵi18nApply(3); } @@ -1081,6 +1085,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.valueA)(ctx.valueB); $r3$.ɵɵi18nApply(1); } @@ -1115,6 +1120,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.valueA); $r3$.ɵɵi18nApply(1); } @@ -1159,6 +1165,7 @@ describe('i18n support in the template compiler', () => { if (rf & 2) { var $tmp_2_0$ = null; const $currVal_2$ = ($tmp_2_0$ = ctx.valueA.getRawValue()) == null ? null : $tmp_2_0$.getTitle(); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, 3, ctx.valueA))(ctx.valueA == null ? null : ctx.valueA.a == null ? null : ctx.valueA.a.b)($currVal_2$); $r3$.ɵɵi18nApply(1); } @@ -1225,10 +1232,13 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.one); $r3$.ɵɵi18nApply(1); + $r3$.ɵɵadvance(3); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(4, 3, ctx.two)); $r3$.ɵɵi18nApply(3); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp(ctx.three + ctx.four + ctx.five); $r3$.ɵɵi18nApply(6); } @@ -1318,8 +1328,10 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp(ctx.one); $r3$.ɵɵi18nApply(1); + $r3$.ɵɵadvance(6); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(5, 3, ctx.two))(ctx.nestedInBlockTwo); $r3$.ɵɵi18nApply(4); } @@ -1425,12 +1437,16 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp(ctx.valueB)(ctx.valueC); $r3$.ɵɵi18nApply(3); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.valueA); $r3$.ɵɵi18nApply(1); + $r3$.ɵɵadvance(4); $r3$.ɵɵi18nExp(ctx.valueE); $r3$.ɵɵi18nApply(8); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(6, 5, ctx.valueD)); $r3$.ɵɵi18nApply(5); } @@ -1487,6 +1503,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(4); $r3$.ɵɵi18nExp($ctx_r0$.valueA)($r3$.ɵɵpipeBind1(4, 2, $ctx_r0$.valueB)); $r3$.ɵɵi18nApply(2); } @@ -1607,6 +1624,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r2$ = $r3$.ɵɵnextContext(2); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($ctx_r2$.valueC)($ctx_r2$.valueD); $r3$.ɵɵi18nApply(0); } @@ -1686,6 +1704,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r1$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(3); $r3$.ɵɵi18nExp($ctx_r1$.valueE + $ctx_r1$.valueF)($r3$.ɵɵpipeBind1(3, 2, $ctx_r1$.valueG)); $r3$.ɵɵi18nApply(0); } @@ -1747,6 +1766,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($ctx_r0$.valueA); $r3$.ɵɵi18nApply(1); } @@ -1853,6 +1873,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.age); $r3$.ɵɵi18nApply(1); } @@ -1976,6 +1997,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementContainerEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, 1, ctx.valueA)); $r3$.ɵɵi18nApply(1); } @@ -2008,6 +2030,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵpipe(1, "uppercase"); } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 1, $ctx_r0$.valueA)); $r3$.ɵɵi18nApply(0); } @@ -2062,6 +2085,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 1, $ctx_r0$.valueA)); $r3$.ɵɵi18nApply(0); } @@ -2080,6 +2104,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(4); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(4, 1, ctx.valueB)); $r3$.ɵɵi18nApply(1); } @@ -2139,6 +2164,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementContainerEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp(ctx.age); $r3$.ɵɵi18nApply(2); } @@ -2182,6 +2208,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r1$ = $r3$.ɵɵnextContext(2); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($ctx_r1$.valueB); $r3$.ɵɵi18nApply(0); } @@ -2222,6 +2249,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(1, 1, $ctx_r0$.valueA)); $r3$.ɵɵi18nApply(0); } @@ -2293,6 +2321,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵtemplate(2, MyComponent_ng_template_2_Template, 1, 1, "ng-template"); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender); $r3$.ɵɵi18nApply(1); } @@ -2629,6 +2658,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender); $r3$.ɵɵi18nApply(1); } @@ -2738,6 +2768,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($ctx_r0$.age); $r3$.ɵɵi18nApply(2); } @@ -2764,6 +2795,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r1$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(2); $r3$.ɵɵi18nExp($ctx_r1$.count)($ctx_r1$.count); $r3$.ɵɵi18nApply(2); } @@ -2781,9 +2813,10 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵtemplate(3, MyComponent_div_3_Template, 4, 2, "div", 1); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender); $r3$.ɵɵi18nApply(1); - $r3$.ɵɵadvance(2); + $r3$.ɵɵadvance(1); $r3$.ɵɵproperty("ngIf", ctx.visible); $r3$.ɵɵadvance(1); $r3$.ɵɵproperty("ngIf", ctx.available); @@ -2820,6 +2853,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.age)(ctx.other); $r3$.ɵɵi18nApply(1); } @@ -2895,6 +2929,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(4); $r3$.ɵɵi18nExp(ctx.gender); $r3$.ɵɵi18nApply(1); } @@ -2932,6 +2967,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender)(ctx.ageA + ctx.ageB + ctx.ageC); $r3$.ɵɵi18nApply(1); } @@ -2994,6 +3030,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender)(ctx.age); $r3$.ɵɵi18nApply(1); } @@ -3081,6 +3118,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($ctx_r0$.gender); $r3$.ɵɵi18nApply(0); } @@ -3155,6 +3193,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.age)(ctx.gender); $r3$.ɵɵi18nApply(1); } @@ -3203,6 +3242,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.name)(ctx.count)(ctx.count); $r3$.ɵɵi18nApply(1); } @@ -3270,6 +3310,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($ctx_r0$.age); $r3$.ɵɵi18nApply(0); } @@ -3359,6 +3400,7 @@ describe('i18n support in the template compiler', () => { } if (rf & 2) { const $ctx_r0$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp($ctx_r0$.age)($ctx_r0$.otherAge); $r3$.ɵɵi18nApply(0); } @@ -3423,6 +3465,7 @@ describe('i18n support in the template compiler', () => { $r3$.ɵɵelementEnd(); } if (rf & 2) { + $r3$.ɵɵadvance(1); $r3$.ɵɵi18nExp(ctx.gender)(ctx.weight)(ctx.height)(ctx.age); $r3$.ɵɵi18nApply(1); } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index a6fcf5eb08..06f8d84887 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -455,7 +455,10 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver bindings.forEach(binding => { chainBindings.push({sourceSpan: span, value: () => this.convertPropertyBinding(binding)}); }); - this.updateInstructionChain(R3.i18nExp, chainBindings); + // for i18n block, advance to the most recent element index (by taking the current number of + // elements and subtracting one) before invoking `i18nExp` instructions, to make sure the + // necessary lifecycle hooks of components/directives are properly flushed. + this.updateInstructionChainWithAdvance(this.getConstCount() - 1, R3.i18nExp, chainBindings); this.updateInstruction(span, R3.i18nApply, [o.literal(index)]); } if (!selfClosing) { @@ -671,7 +674,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } }); if (bindings.length) { - this.updateInstructionChain(R3.i18nExp, bindings); + this.updateInstructionChainWithAdvance(elementIndex, R3.i18nExp, bindings); } if (i18nAttrArgs.length) { const index: o.Expression = o.literal(this.allocateDataSlot()); diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index a1e0be58dd..8b5c83e323 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -10,7 +10,7 @@ import '@angular/localize/init'; import {CommonModule, registerLocaleData} from '@angular/common'; import localeRo from '@angular/common/locales/ro'; -import {Component, ContentChild, ContentChildren, Directive, HostBinding, Input, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, Pipe, PipeTransform, NO_ERRORS_SCHEMA} from '@angular/core'; +import {Component, ContentChild, ElementRef, ContentChildren, Directive, HostBinding, Input, LOCALE_ID, QueryList, TemplateRef, Type, ViewChild, ViewContainerRef, Pipe, PipeTransform, NO_ERRORS_SCHEMA} from '@angular/core'; import {setDelayProjection} from '@angular/core/src/render3/instructions/projection'; import {TestBed} from '@angular/core/testing'; import {loadTranslations, clearTranslations} from '@angular/localize'; @@ -2015,6 +2015,55 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { }); }); + it('should reflect lifecycle hook changes in text interpolations in i18n block', () => { + @Directive({selector: 'input'}) + class InputsDir { + constructor(private elementRef: ElementRef) {} + ngOnInit() { this.elementRef.nativeElement.value = 'value set in Directive.ngOnInit'; } + } + + @Component({ + template: ` + +
{{myinput.value}}
+ ` + }) + class App { + } + + TestBed.configureTestingModule({declarations: [App, InputsDir]}); + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent).toContain('value set in Directive.ngOnInit'); + }); + + it('should reflect lifecycle hook changes in text interpolations in i18n attributes', () => { + @Directive({selector: 'input'}) + class InputsDir { + constructor(private elementRef: ElementRef) {} + ngOnInit() { this.elementRef.nativeElement.value = 'value set in Directive.ngOnInit'; } + } + + @Component({ + template: ` + +
+ ` + }) + class App { + } + + TestBed.configureTestingModule({declarations: [App, InputsDir]}); + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('div').title) + .toContain('value set in Directive.ngOnInit'); + }); + it('should not alloc expando slots when there is no new variable to create', () => { loadTranslations({ [computeMsgId('{$START_TAG_DIV} Some content {$CLOSE_TAG_DIV}')]: