Skip to content

Commit 7660d0d

Browse files
AndrewKushnirmhevery
authored andcommitted
fix(ivy): extended next pointer lookup while traversing tNode tree (angular#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 angular#28533
1 parent 264ef72 commit 7660d0d

File tree

3 files changed

+161
-5
lines changed

3 files changed

+161
-5
lines changed

packages/core/src/render3/node_manipulation.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ function walkTNodeTree(
150150
*/
151151
while (!nextTNode) {
152152
// If parent is null, we're crossing the view boundary, so we should get the host TNode.
153-
tNode = tNode.parent || currentView[TVIEW].node;
153+
tNode = tNode.parent || currentView[T_HOST];
154154

155155
if (tNode === null || tNode === rootTNode) return null;
156156

@@ -160,9 +160,26 @@ function walkTNodeTree(
160160
beforeNode = currentView[tNode.index][NATIVE];
161161
}
162162

163-
if (tNode.type === TNodeType.View && currentView[NEXT]) {
164-
currentView = currentView[NEXT] as LView;
165-
nextTNode = currentView[TVIEW].node;
163+
if (tNode.type === TNodeType.View) {
164+
/**
165+
* If current lView doesn't have next pointer, we try to find it by going up parents
166+
* chain until:
167+
* - we find an lView with a next pointer
168+
* - or find a tNode with a parent that has a next pointer
169+
* - or reach root TNode (in which case we exit, since we traversed all nodes)
170+
*/
171+
while (!currentView[NEXT] && currentView[PARENT] &&
172+
!(tNode.parent && tNode.parent.next)) {
173+
if (tNode === rootTNode) return null;
174+
currentView = currentView[PARENT] as LView;
175+
tNode = currentView[T_HOST] !;
176+
}
177+
if (currentView[NEXT]) {
178+
currentView = currentView[NEXT] as LView;
179+
nextTNode = currentView[T_HOST];
180+
} else {
181+
nextTNode = tNode.next;
182+
}
166183
} else {
167184
nextTNode = tNode.next;
168185
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {Component} from '@angular/core';
10+
import {TestBed} from '@angular/core/testing';
11+
import {expect} from '@angular/platform-browser/testing/src/matchers';
12+
13+
describe('projection', () => {
14+
it('should handle projected containers inside other containers', () => {
15+
@Component({
16+
selector: 'child-comp', //
17+
template: '<ng-content></ng-content>'
18+
})
19+
class ChildComp {
20+
}
21+
22+
@Component({
23+
selector: 'root-comp', //
24+
template: '<ng-content></ng-content>'
25+
})
26+
class RootComp {
27+
}
28+
29+
@Component({
30+
selector: 'my-app',
31+
template: `
32+
<root-comp>
33+
<ng-container *ngFor="let item of items; last as last">
34+
<child-comp *ngIf="!last">{{ item }}|</child-comp>
35+
</ng-container>
36+
</root-comp>
37+
`
38+
})
39+
class MyApp {
40+
items: number[] = [1, 2, 3];
41+
}
42+
43+
TestBed.configureTestingModule({declarations: [ChildComp, RootComp, MyApp]});
44+
const fixture = TestBed.createComponent(MyApp);
45+
fixture.detectChanges();
46+
47+
// expecting # of elements to be (items.length - 1), since last element is filtered out by
48+
// *ngIf, this applies to all other assertions below
49+
expect(fixture.nativeElement).toHaveText('1|2|');
50+
51+
fixture.componentInstance.items = [4, 5];
52+
fixture.detectChanges();
53+
expect(fixture.nativeElement).toHaveText('4|');
54+
55+
fixture.componentInstance.items = [6, 7, 8, 9];
56+
fixture.detectChanges();
57+
expect(fixture.nativeElement).toHaveText('6|7|8|');
58+
});
59+
});

packages/core/test/render3/content_spec.ts

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {SelectorFlags} from '@angular/core/src/render3/interfaces/projection';
1010

11-
import {AttributeMarker, defineDirective, detectChanges, directiveInject, loadViewQuery, queryRefresh, reference, templateRefExtractor, viewQuery} from '../../src/render3/index';
11+
import {AttributeMarker, defineComponent, defineDirective, detectChanges, directiveInject, loadViewQuery, queryRefresh, reference, templateRefExtractor, viewQuery} from '../../src/render3/index';
1212

1313
import {bind, container, containerRefreshEnd, containerRefreshStart, element, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, projection, projectionDef, template, text, textBinding, interpolation1} from '../../src/render3/instructions';
1414
import {RenderFlags} from '../../src/render3/interfaces/definition';
@@ -1441,6 +1441,86 @@ describe('content projection', () => {
14411441
expect(toHtml(parent)).toEqual('<child><grand-child>content</grand-child></child>');
14421442
});
14431443

1444+
it('should handle projected containers inside other containers', () => {
1445+
// <div>Child content</div>
1446+
const NestedComp = createComponent('nested-comp', function(rf: RenderFlags, ctx: any) {
1447+
if (rf & RenderFlags.Create) {
1448+
elementStart(0, 'div');
1449+
text(1, 'Child content');
1450+
elementEnd();
1451+
}
1452+
}, 2, 0, []);
1453+
1454+
// <ng-content></ng-content>
1455+
const RootComp = createComponent('root-comp', function(rf: RenderFlags, ctx: any) {
1456+
if (rf & RenderFlags.Create) {
1457+
projectionDef();
1458+
projection(0);
1459+
}
1460+
}, 1, 0, []);
1461+
1462+
// <root-comp>
1463+
// <ng-container *ngFor="let item of items; last as last">
1464+
// <child-comp *ngIf="!last"></child-comp>
1465+
// </ng-container>
1466+
// </root-comp>
1467+
function MyApp_ng_container_1_child_comp_1_Template(rf: RenderFlags, ctx: any) {
1468+
if (rf & RenderFlags.Create) {
1469+
element(0, 'nested-comp');
1470+
}
1471+
}
1472+
function MyApp_ng_container_1_Template(rf: RenderFlags, ctx: any) {
1473+
if (rf & RenderFlags.Create) {
1474+
elementContainerStart(0);
1475+
template(1, MyApp_ng_container_1_child_comp_1_Template, 1, 0, 'nested-comp', [3, 'ngIf']);
1476+
elementContainerEnd();
1477+
}
1478+
if (rf & RenderFlags.Update) {
1479+
const last_r2 = ctx.last;
1480+
elementProperty(1, 'ngIf', bind(!last_r2));
1481+
}
1482+
}
1483+
let myAppInstance: MyApp;
1484+
class MyApp {
1485+
items = [1, 2];
1486+
1487+
static ngComponentDef = defineComponent({
1488+
type: MyApp,
1489+
selectors: [['', 'my-app', '']],
1490+
factory: () => myAppInstance = new MyApp(),
1491+
consts: 2,
1492+
vars: 1,
1493+
template: function MyApp_Template(rf: RenderFlags, ctx: any) {
1494+
if (rf & RenderFlags.Create) {
1495+
elementStart(0, 'root-comp');
1496+
template(
1497+
1, MyApp_ng_container_1_Template, 2, 1, 'ng-container',
1498+
['ngFor', '', 3, 'ngForOf']);
1499+
elementEnd();
1500+
}
1501+
if (rf & RenderFlags.Update) {
1502+
elementProperty(1, 'ngForOf', bind(ctx.items));
1503+
}
1504+
},
1505+
directives: [NgForOf, NgIf, NestedComp, RootComp]
1506+
});
1507+
}
1508+
const fixture = new ComponentFixture(MyApp);
1509+
fixture.update();
1510+
1511+
// expecting # of divs to be (items.length - 1), since last element is filtered out by *ngIf,
1512+
// this applies to all other assertions below
1513+
expect(fixture.hostElement.querySelectorAll('div').length).toBe(1);
1514+
1515+
myAppInstance !.items = [3, 4, 5];
1516+
fixture.update();
1517+
expect(fixture.hostElement.querySelectorAll('div').length).toBe(2);
1518+
1519+
myAppInstance !.items = [6, 7, 8, 9];
1520+
fixture.update();
1521+
expect(fixture.hostElement.querySelectorAll('div').length).toBe(3);
1522+
});
1523+
14441524
describe('with selectors', () => {
14451525

14461526
it('should project nodes using attribute selectors', () => {

0 commit comments

Comments
 (0)