From 9d109929be2860e7857275225ff15366f6191e52 Mon Sep 17 00:00:00 2001 From: Olivier Combe Date: Thu, 7 Feb 2019 16:57:37 +0100 Subject: [PATCH] fix(ivy): remove nested placeholders with i18n (#28595) PR Close #28595 --- packages/core/src/render3/i18n.ts | 34 +++++++--- packages/core/test/i18n_integration_spec.ts | 73 ++++++++++++++++++++- packages/core/test/render3/i18n_spec.ts | 2 +- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index a791d7f4a7..4ceecc2465 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -614,6 +614,16 @@ 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. */ @@ -629,12 +639,17 @@ function i18nEndFirstPass(tView: TView) { // The last placeholder that was added before `i18nEnd` const previousOrParentTNode = getPreviousOrParentTNode(); - const visitedPlaceholders = readCreateOpCodes(rootIndex, tI18n.create, tI18n.icus, viewData); + const visitedNodes = readCreateOpCodes(rootIndex, tI18n.create, tI18n.icus, viewData); - // Remove deleted placeholders - // The last placeholder that was added before `i18nEnd` is `previousOrParentTNode` - for (let i = rootIndex + 1; i <= previousOrParentTNode.index - HEADER_OFFSET; i++) { - if (visitedPlaceholders.indexOf(i) === -1) { + // Find the last node that was added before `i18nEnd` + let lastCreatedNode = previousOrParentTNode; + if (lastCreatedNode.child) { + lastCreatedNode = findLastNode(lastCreatedNode.child); + } + + // Remove deleted nodes + for (let i = rootIndex + 1; i <= lastCreatedNode.index - HEADER_OFFSET; i++) { + if (visitedNodes.indexOf(i) === -1) { removeNode(i, viewData); } } @@ -646,7 +661,7 @@ function readCreateOpCodes( const renderer = getLView()[RENDERER]; let currentTNode: TNode|null = null; let previousTNode: TNode|null = null; - const visitedPlaceholders: number[] = []; + const visitedNodes: number[] = []; for (let i = 0; i < createOpCodes.length; i++) { const opCode = createOpCodes[i]; if (typeof opCode == 'string') { @@ -655,6 +670,7 @@ function readCreateOpCodes( ngDevMode && ngDevMode.rendererCreateTextNode++; previousTNode = currentTNode; currentTNode = createNodeAtIndex(textNodeIndex, TNodeType.Element, textRNode, null, null); + visitedNodes.push(textNodeIndex); setIsParent(false); } else if (typeof opCode == 'number') { switch (opCode & I18nMutateOpCode.MASK_OPCODE) { @@ -677,7 +693,7 @@ function readCreateOpCodes( break; case I18nMutateOpCode.Select: const nodeIndex = opCode >>> I18nMutateOpCode.SHIFT_REF; - visitedPlaceholders.push(nodeIndex); + visitedNodes.push(nodeIndex); previousTNode = currentTNode; currentTNode = getTNode(nodeIndex, viewData); if (currentTNode) { @@ -715,6 +731,7 @@ function readCreateOpCodes( previousTNode = currentTNode; currentTNode = createNodeAtIndex(commentNodeIndex, TNodeType.IcuContainer, commentRNode, null, null); + visitedNodes.push(commentNodeIndex); attachPatchData(commentRNode, viewData); (currentTNode as TIcuContainerNode).activeCaseIndex = null; // We will add the case nodes later, during the update phase @@ -731,6 +748,7 @@ function readCreateOpCodes( previousTNode = currentTNode; currentTNode = createNodeAtIndex( elementNodeIndex, TNodeType.Element, elementRNode, tagNameValue, null); + visitedNodes.push(elementNodeIndex); break; default: throw new Error(`Unable to determine the type of mutate operation for "${opCode}"`); @@ -740,7 +758,7 @@ function readCreateOpCodes( setIsParent(false); - return visitedPlaceholders; + return visitedNodes; } function readUpdateOpCodes( diff --git a/packages/core/test/i18n_integration_spec.ts b/packages/core/test/i18n_integration_spec.ts index 0e01f39edf..06f51e631d 100644 --- a/packages/core/test/i18n_integration_spec.ts +++ b/packages/core/test/i18n_integration_spec.ts @@ -6,10 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, TemplateRef, ViewContainerRef} from '@angular/core'; +import {Component, ContentChild, ContentChildren, Directive, QueryList, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {fixmeIvy, onlyInIvy, polyfillGoogGetMsg} from '@angular/private/testing'; +import {onlyInIvy, polyfillGoogGetMsg} from '@angular/private/testing'; @Directive({ selector: '[tplRef]', @@ -59,7 +59,9 @@ const TRANSLATIONS: any = { '{VAR_SELECT, select, 10 {10 - {$startBoldText}ten{$closeBoldText}} 20 {20 - {$startItalicText}twenty{$closeItalicText}} other {{$startTagDiv}{$startUnderlinedText}other{$closeUnderlinedText}{$closeTagDiv}}}': '{VAR_SELECT, select, 10 {10 - {$startBoldText}dix{$closeBoldText}} 20 {20 - {$startItalicText}vingt{$closeItalicText}} other {{$startTagDiv}{$startUnderlinedText}autres{$closeUnderlinedText}{$closeTagDiv}}}', '{VAR_SELECT_2, select, 10 {ten - {VAR_SELECT, select, 1 {one} 2 {two} other {more than two}}} 20 {twenty - {VAR_SELECT_1, select, 1 {one} 2 {two} other {more than two}}} other {other}}': - '{VAR_SELECT_2, select, 10 {dix - {VAR_SELECT, select, 1 {un} 2 {deux} other {plus que deux}}} 20 {vingt - {VAR_SELECT_1, select, 1 {un} 2 {deux} other {plus que deux}}} other {autres}}' + '{VAR_SELECT_2, select, 10 {dix - {VAR_SELECT, select, 1 {un} 2 {deux} other {plus que deux}}} 20 {vingt - {VAR_SELECT_1, select, 1 {un} 2 {deux} other {plus que deux}}} other {autres}}', + '{$startTagNgTemplate}{$startTagDiv_1}{$startTagDiv}{$startTagSpan}Content{$closeTagSpan}{$closeTagDiv}{$closeTagDiv}{$closeTagNgTemplate}': + '{$startTagNgTemplate}Contenu{$closeTagNgTemplate}' }; const getFixtureWithOverrides = (overrides = {}) => { @@ -514,4 +516,69 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() { expect(element).toHaveText('vingt'); }); }); + + describe('queries', () => { + function toHtml(element: Element): string { + return element.innerHTML.replace(/\sng-reflect-\S*="[^"]*"/g, '') + .replace(//g, ''); + } + + it('detached nodes should still be part of query', () => { + const template = ` + + +
+
+ Content +
+
+ + + `; + + @Directive({selector: '[text]', inputs: ['text'], exportAs: 'textDir'}) + class TextDirective { + // TODO(issue/24571): remove '!'. + text !: string; + constructor() {} + } + + @Component({selector: 'div-query', template: ''}) + class DivQuery { + // TODO(issue/24571): remove '!'. + @ContentChild(TemplateRef) template !: TemplateRef; + + // TODO(issue/24571): remove '!'. + @ViewChild('vc', {read: ViewContainerRef}) + vc !: ViewContainerRef; + + // TODO(issue/24571): remove '!'. + @ContentChildren(TextDirective, {descendants: true}) + query !: QueryList; + + create() { this.vc.createEmbeddedView(this.template); } + + destroy() { this.vc.clear(); } + } + + TestBed.configureTestingModule({declarations: [TextDirective, DivQuery]}); + const fixture = getFixtureWithOverrides({template}); + const q = fixture.debugElement.children[0].references.q; + expect(q.query.length).toEqual(0); + + // Create embedded view + q.create(); + fixture.detectChanges(); + expect(q.query.length).toEqual(1); + expect(toHtml(fixture.nativeElement)) + .toEqual(`Contenu`); + + // Disable ng-if + fixture.componentInstance.visible = false; + fixture.detectChanges(); + expect(q.query.length).toEqual(0); + expect(toHtml(fixture.nativeElement)) + .toEqual(`Contenu`); + }); + }); }); diff --git a/packages/core/test/render3/i18n_spec.ts b/packages/core/test/render3/i18n_spec.ts index 91da947040..fc97c5aa04 100644 --- a/packages/core/test/render3/i18n_spec.ts +++ b/packages/core/test/render3/i18n_spec.ts @@ -1771,7 +1771,7 @@ describe('Runtime i18n', () => { selectors: [['parent']], directives: [Child], factory: () => new Parent(), - consts: 2, + consts: 3, vars: 0, template: (rf: RenderFlags, cmp: Parent) => { if (rf & RenderFlags.Create) {