From 7660d0d74a52536cbc55065323deb40460de01d5 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Fri, 1 Feb 2019 17:04:02 -0800 Subject: [PATCH] fix(ivy): extended next pointer lookup while traversing tNode tree (#28533) Prior to this change we only checked whether current lView has a next pointer while traversing tNode tree. However in some cases this pointer can be undefined and we need to look up parents chain to find suitable next pointer. This commit adds the logic of searching for the next pointer taking parents chain into account. PR Close #28533 --- .../core/src/render3/node_manipulation.ts | 25 +++++- packages/core/test/acceptance/content_spec.ts | 59 +++++++++++++ packages/core/test/render3/content_spec.ts | 82 ++++++++++++++++++- 3 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 packages/core/test/acceptance/content_spec.ts diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index d0307d63cc..13066d82c6 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -150,7 +150,7 @@ function walkTNodeTree( */ while (!nextTNode) { // If parent is null, we're crossing the view boundary, so we should get the host TNode. - tNode = tNode.parent || currentView[TVIEW].node; + tNode = tNode.parent || currentView[T_HOST]; if (tNode === null || tNode === rootTNode) return null; @@ -160,9 +160,26 @@ function walkTNodeTree( beforeNode = currentView[tNode.index][NATIVE]; } - if (tNode.type === TNodeType.View && currentView[NEXT]) { - currentView = currentView[NEXT] as LView; - nextTNode = currentView[TVIEW].node; + if (tNode.type === TNodeType.View) { + /** + * If current lView doesn't have next pointer, we try to find it by going up parents + * chain until: + * - we find an lView with a next pointer + * - or find a tNode with a parent that has a next pointer + * - or reach root TNode (in which case we exit, since we traversed all nodes) + */ + while (!currentView[NEXT] && currentView[PARENT] && + !(tNode.parent && tNode.parent.next)) { + if (tNode === rootTNode) return null; + currentView = currentView[PARENT] as LView; + tNode = currentView[T_HOST] !; + } + if (currentView[NEXT]) { + currentView = currentView[NEXT] as LView; + nextTNode = currentView[T_HOST]; + } else { + nextTNode = tNode.next; + } } else { nextTNode = tNode.next; } diff --git a/packages/core/test/acceptance/content_spec.ts b/packages/core/test/acceptance/content_spec.ts new file mode 100644 index 0000000000..da7853c9d1 --- /dev/null +++ b/packages/core/test/acceptance/content_spec.ts @@ -0,0 +1,59 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Component} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import {expect} from '@angular/platform-browser/testing/src/matchers'; + +describe('projection', () => { + it('should handle projected containers inside other containers', () => { + @Component({ + selector: 'child-comp', // + template: '' + }) + class ChildComp { + } + + @Component({ + selector: 'root-comp', // + template: '' + }) + class RootComp { + } + + @Component({ + selector: 'my-app', + template: ` + + + {{ item }}| + + + ` + }) + class MyApp { + items: number[] = [1, 2, 3]; + } + + TestBed.configureTestingModule({declarations: [ChildComp, RootComp, MyApp]}); + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + + // expecting # of elements to be (items.length - 1), since last element is filtered out by + // *ngIf, this applies to all other assertions below + expect(fixture.nativeElement).toHaveText('1|2|'); + + fixture.componentInstance.items = [4, 5]; + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveText('4|'); + + fixture.componentInstance.items = [6, 7, 8, 9]; + fixture.detectChanges(); + expect(fixture.nativeElement).toHaveText('6|7|8|'); + }); +}); \ No newline at end of file diff --git a/packages/core/test/render3/content_spec.ts b/packages/core/test/render3/content_spec.ts index 00d9a0320b..a4d82b307c 100644 --- a/packages/core/test/render3/content_spec.ts +++ b/packages/core/test/render3/content_spec.ts @@ -8,7 +8,7 @@ import {SelectorFlags} from '@angular/core/src/render3/interfaces/projection'; -import {AttributeMarker, defineDirective, detectChanges, directiveInject, loadViewQuery, queryRefresh, reference, templateRefExtractor, viewQuery} from '../../src/render3/index'; +import {AttributeMarker, defineComponent, defineDirective, detectChanges, directiveInject, loadViewQuery, queryRefresh, reference, templateRefExtractor, viewQuery} from '../../src/render3/index'; import {bind, container, containerRefreshEnd, containerRefreshStart, element, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, projection, projectionDef, template, text, textBinding, interpolation1} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; @@ -1441,6 +1441,86 @@ describe('content projection', () => { expect(toHtml(parent)).toEqual('content'); }); + it('should handle projected containers inside other containers', () => { + //
Child content
+ const NestedComp = createComponent('nested-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div'); + text(1, 'Child content'); + elementEnd(); + } + }, 2, 0, []); + + // + const RootComp = createComponent('root-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(); + projection(0); + } + }, 1, 0, []); + + // + // + // + // + // + function MyApp_ng_container_1_child_comp_1_Template(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'nested-comp'); + } + } + function MyApp_ng_container_1_Template(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementContainerStart(0); + template(1, MyApp_ng_container_1_child_comp_1_Template, 1, 0, 'nested-comp', [3, 'ngIf']); + elementContainerEnd(); + } + if (rf & RenderFlags.Update) { + const last_r2 = ctx.last; + elementProperty(1, 'ngIf', bind(!last_r2)); + } + } + let myAppInstance: MyApp; + class MyApp { + items = [1, 2]; + + static ngComponentDef = defineComponent({ + type: MyApp, + selectors: [['', 'my-app', '']], + factory: () => myAppInstance = new MyApp(), + consts: 2, + vars: 1, + template: function MyApp_Template(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'root-comp'); + template( + 1, MyApp_ng_container_1_Template, 2, 1, 'ng-container', + ['ngFor', '', 3, 'ngForOf']); + elementEnd(); + } + if (rf & RenderFlags.Update) { + elementProperty(1, 'ngForOf', bind(ctx.items)); + } + }, + directives: [NgForOf, NgIf, NestedComp, RootComp] + }); + } + const fixture = new ComponentFixture(MyApp); + fixture.update(); + + // expecting # of divs to be (items.length - 1), since last element is filtered out by *ngIf, + // this applies to all other assertions below + expect(fixture.hostElement.querySelectorAll('div').length).toBe(1); + + myAppInstance !.items = [3, 4, 5]; + fixture.update(); + expect(fixture.hostElement.querySelectorAll('div').length).toBe(2); + + myAppInstance !.items = [6, 7, 8, 9]; + fixture.update(); + expect(fixture.hostElement.querySelectorAll('div').length).toBe(3); + }); + describe('with selectors', () => { it('should project nodes using attribute selectors', () => {