From 7c297e05f3cb5022b3f5cd769fbb5dec4445ceea Mon Sep 17 00:00:00 2001 From: Olivier Combe Date: Thu, 14 Mar 2019 16:26:01 +0100 Subject: [PATCH] fix(ivy): correctly remove placeholders inside of *ngFor with runtime i18n (#29308) Following my previous change for placeholders removal, some special code that was used to find the last created node was no longer needed and had wrong interactions with the *ngFor directive. Removing it fixed the issue. PR Close #29308 --- packages/core/src/render3/i18n.ts | 14 +---- packages/core/test/i18n_integration_spec.ts | 16 +++++ packages/core/test/render3/i18n_spec.ts | 69 ++++++++++++++++++++- 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index 60ec580fe2..23097c21ad 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -626,16 +626,6 @@ export function i18nEnd(): void { i18nEndFirstPass(tView); } -function findLastNode(node: TNode): TNode { - while (node.next) { - node = node.next; - } - if (node.child) { - return findLastNode(node.child); - } - return node; -} - /** * See `i18nEnd` above. */ @@ -651,10 +641,8 @@ function i18nEndFirstPass(tView: TView) { // Find the last node that was added before `i18nEnd` let lastCreatedNode = getPreviousOrParentTNode(); - if (lastCreatedNode.child) { - lastCreatedNode = findLastNode(lastCreatedNode.child); - } + // Read the instructions to insert/move/remove DOM elements const visitedNodes = readCreateOpCodes(rootIndex, tI18n.create, tI18n.icus, viewData); // Remove deleted nodes diff --git a/packages/core/test/i18n_integration_spec.ts b/packages/core/test/i18n_integration_spec.ts index e89b15139c..2abdebc725 100644 --- a/packages/core/test/i18n_integration_spec.ts +++ b/packages/core/test/i18n_integration_spec.ts @@ -621,4 +621,20 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() { expect(fixture.nativeElement.innerHTML) .toBe('
Section 1
Section 2
Section 3
'); }); + + it('should handle multiple i18n sections inside of *ngFor', () => { + const template = ` + + `; + const fixture = getFixtureWithOverrides({template}); + const element = fixture.nativeElement; + for (let i = 0; i < element.children.length; i++) { + const child = element.children[i]; + expect(child.innerHTML).toBe(`
  • Section 1
  • Section 2
  • Section 3
    • `); + } + }); }); diff --git a/packages/core/test/render3/i18n_spec.ts b/packages/core/test/render3/i18n_spec.ts index 1bd175fd6a..6fe8421375 100644 --- a/packages/core/test/render3/i18n_spec.ts +++ b/packages/core/test/render3/i18n_spec.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {NgForOfContext} from '@angular/common'; import {noop} from '../../../compiler/src/render3/view/util'; import {Component as _Component} from '../../src/core'; import {defineComponent, defineDirective} from '../../src/render3/definition'; @@ -13,8 +14,8 @@ import {getTranslationForTemplate, i18n, i18nApply, i18nAttributes, i18nEnd, i18 import {RenderFlags} from '../../src/render3/interfaces/definition'; import {AttributeMarker} from '../../src/render3/interfaces/node'; import {getNativeByIndex, getTNode} from '../../src/render3/util/view_utils'; -import {NgIf} from './common_with_def'; -import {allocHostVars, element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions'; +import {NgForOf, NgIf} from './common_with_def'; +import {allocHostVars, element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd, textBinding} from '../../src/render3/instructions'; import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nUpdateOpCode, I18nUpdateOpCodes, TI18n} from '../../src/render3/interfaces/i18n'; import {HEADER_OFFSET, LView, TVIEW} from '../../src/render3/interfaces/view'; import {ComponentFixture, TemplateFixture} from './render_util'; @@ -1334,6 +1335,70 @@ describe('Runtime i18n', () => { .toEqual(`
    Section 1
    Section 2
    Section 3
    `); }); + it('should support multiple sibling i18n blocks inside of *ngFor', () => { + // Translated template: + // + + const MSG_DIV_1 = `Section 1`; + const MSG_DIV_2 = `Section 2`; + const MSG_DIV_3 = `Section 3`; + + function liTemplate(rf: RenderFlags, ctx: NgForOfContext) { + if (rf & RenderFlags.Create) { + elementStart(0, 'ul'); + elementStart(1, 'li'); + { i18n(2, MSG_DIV_1); } + elementEnd(); + elementStart(3, 'li'); + { i18n(4, MSG_DIV_2); } + elementEnd(); + elementStart(5, 'li'); + { i18n(6, MSG_DIV_3); } + elementEnd(); + elementEnd(); + } + if (rf & RenderFlags.Update) { + i18nApply(2); + i18nApply(4); + i18nApply(6); + } + } + + class MyApp { + items: string[] = ['1', '2', '3']; + + static ngComponentDef = defineComponent({ + type: MyApp, + selectors: [['my-app']], + factory: () => new MyApp(), + consts: 2, + vars: 1, + template: (rf: RenderFlags, ctx: MyApp) => { + if (rf & RenderFlags.Create) { + elementStart(0, 'div'); + { + template(1, liTemplate, 7, 0, 'ul', [AttributeMarker.Template, 'ngFor', 'ngForOf']); + } + elementEnd(); + } + if (rf & RenderFlags.Update) { + elementProperty(1, 'ngForOf', bind(ctx.items)); + } + }, + directives: () => [NgForOf] + }); + } + + const fixture = new ComponentFixture(MyApp); + expect(fixture.html) + .toEqual( + `
    • Section 1
    • Section 2
    • Section 3
    • Section 1
    • Section 2
    • Section 3
    • Section 1
    • Section 2
    • Section 3
    `); + }); + it('should support attribute translations on removed elements', () => { // Translated template: //