From 8d371fac472739858c1e62c135fd8d51ad6d545c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Mon, 28 Oct 2019 16:46:05 -0700 Subject: [PATCH 1/5] fix(ivy): `ViewRef.rootNodes` returns rootNodes from child views as well. The idea behind `rootNodes` is to return all of the nodes which need to be added to the DOM in order for the `ViewRef` to be rendered. The new implementation correctly takes into account the case where one of the rootNodes is a container containing other `ViewRef`s. --- packages/core/src/render3/view_ref.ts | 36 +++++++----- .../acceptance/view_container_ref_spec.ts | 56 ++++++++++++++++++- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 1dbd3211064a8..45ef298c950d9 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -10,11 +10,13 @@ import {ApplicationRef} from '../application_ref'; import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref'; import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref'; +import {assertDefined} from '../util/assert'; import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared'; +import {CONTAINER_HEADER_OFFSET} from './interfaces/container'; import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; -import {CONTEXT, FLAGS, HOST, LView, LViewFlags, T_HOST} from './interfaces/view'; -import {destroyLView, renderDetachView} from './node_manipulation'; +import {CONTEXT, FLAGS, HOST, LView, LViewFlags, TView, T_HOST} from './interfaces/view'; +import {destroyLView, getLContainer, renderDetachView} from './node_manipulation'; import {findComponentView, getLViewParent} from './util/view_traversal_utils'; import {getNativeByTNode, getNativeByTNodeOrNull} from './util/view_utils'; @@ -38,7 +40,7 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int get rootNodes(): any[] { if (this._lView[HOST] == null) { const tView = this._lView[T_HOST] as TViewNode; - return collectNativeNodes(this._lView, tView, []); + return collectNativeNodes(this._lView, tView.child, []); } return []; } @@ -299,27 +301,35 @@ export class RootViewRef extends ViewRef { get context(): T { return null !; } } -function collectNativeNodes(lView: LView, parentTNode: TNode, result: any[]): any[] { - let tNodeChild = parentTNode.child; - - while (tNodeChild) { - const nativeNode = getNativeByTNodeOrNull(tNodeChild, lView); +function collectNativeNodes(lView: LView, tNode: TNode | null, result: any[]): any[] { + while (tNode) { + const nativeNode = getNativeByTNodeOrNull(tNode, lView); nativeNode && result.push(nativeNode); - if (tNodeChild.type === TNodeType.ElementContainer) { - collectNativeNodes(lView, tNodeChild, result); - } else if (tNodeChild.type === TNodeType.Projection) { + if (tNode.type === TNodeType.ElementContainer) { + collectNativeNodes(lView, tNode.child, result); + } else if (tNode.type === TNodeType.Container) { + const lContainer = lView[tNode.index]; + const containerTView = tNode.tViews !as TView; + ngDevMode && assertDefined(containerTView, 'TView expected'); + const containerFirstChildTNode = containerTView.firstChild !; + if (containerFirstChildTNode) { + for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) { + collectNativeNodes(lContainer[i], containerFirstChildTNode, result); + } + } + } else if (tNode.type === TNodeType.Projection) { const componentView = findComponentView(lView); const componentHost = componentView[T_HOST] as TElementNode; const parentView = getLViewParent(componentView); let currentProjectedNode: TNode|null = - (componentHost.projection as(TNode | null)[])[tNodeChild.projection as number]; + (componentHost.projection as(TNode | null)[])[tNode.projection as number]; while (currentProjectedNode && parentView) { result.push(getNativeByTNode(currentProjectedNode, parentView)); currentProjectedNode = currentProjectedNode.next; } } - tNodeChild = tNodeChild.next; + tNode = tNode.next; } return result; diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index e393c932f4ce3..131f34c4ffed1 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -8,7 +8,7 @@ import {CommonModule, DOCUMENT} from '@angular/common'; import {computeMsgId} from '@angular/compiler'; -import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; +import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ViewRef} from '@angular/core'; import {Input} from '@angular/core/src/metadata'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed, TestComponentRenderer} from '@angular/core/testing'; @@ -872,6 +872,60 @@ describe('ViewContainerRef', () => { '**A****C****C****B**'); }); + it('should recurs into child views when computing rootNodes elements', () => { + class MyDirEmbedded { + // This is a marker for easier debugging. + } + + @Directive({selector: '[dir]'}) + class MyDir { + viewRef: EmbeddedViewRef<{}>|null = null; + + @Input() dir: any; + + constructor(public template: TemplateRef<{}>, public viewContainerRef: ViewContainerRef) { + this.viewRef = + this.viewContainerRef.createEmbeddedView(this.template, new MyDirEmbedded()); + this.viewRef.detectChanges(); + } + } + + @Component({ + selector: 'edit-form', + template: ` + +
Text
+ + `, + }) + class MyComp { + @ViewChild(MyDir, {static: true}) dir !: MyDir; + } + + TestBed.configureTestingModule({ + declarations: [MyComp, MyDir], + imports: [CommonModule], + }); + + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + + const dirRef = fixture.componentInstance.dir; + + // Expecting: + // - One comment node for ngIf anchor. + // - One
Text
node for the ngIf content. + // - VE ONLY: Extra comment node It is unclear why the VE adds the last one. + const rootNodes = dirRef.viewRef !.rootNodes; + expect(rootNodes.length).toBe(ivyEnabled ? 2 : 3); + expect(rootNodes[0].outerHTML).toBe(''); + expect(rootNodes[1].outerHTML).toBe('
Text
'); + if (!ivyEnabled) { + expect(rootNodes[2].outerHTML) + .toBe(''); // It is unclear why the VE adds the last one. + } + }); + }); describe('createComponent', () => { From dc424ff43cdaa633349860700b7596bbb44c9237 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 28 Oct 2019 21:30:50 -0700 Subject: [PATCH 2/5] fixup! fix(ivy): `ViewRef.rootNodes` returns rootNodes from child views as well. --- packages/core/test/acceptance/view_container_ref_spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 131f34c4ffed1..f77fc293c5879 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -918,7 +918,7 @@ describe('ViewContainerRef', () => { // - VE ONLY: Extra comment node It is unclear why the VE adds the last one. const rootNodes = dirRef.viewRef !.rootNodes; expect(rootNodes.length).toBe(ivyEnabled ? 2 : 3); - expect(rootNodes[0].outerHTML).toBe(''); + expect(rootNodes[0].textContent).toBe('bindings={\n "ng-reflect-ng-if": "true"\n}'); expect(rootNodes[1].outerHTML).toBe('
Text
'); if (!ivyEnabled) { expect(rootNodes[2].outerHTML) From 6a4f95351d91929a82f1cb9e8834d2f6fc18e889 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 28 Oct 2019 21:30:50 -0700 Subject: [PATCH 3/5] fixup! fix(ivy): `ViewRef.rootNodes` returns rootNodes from child views as well. --- packages/core/src/render3/view_ref.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 45ef298c950d9..8c4211d36915e 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -11,12 +11,11 @@ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detec import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref'; import {assertDefined} from '../util/assert'; - import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared'; import {CONTAINER_HEADER_OFFSET} from './interfaces/container'; import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; import {CONTEXT, FLAGS, HOST, LView, LViewFlags, TView, T_HOST} from './interfaces/view'; -import {destroyLView, getLContainer, renderDetachView} from './node_manipulation'; +import {destroyLView, renderDetachView} from './node_manipulation'; import {findComponentView, getLViewParent} from './util/view_traversal_utils'; import {getNativeByTNode, getNativeByTNodeOrNull} from './util/view_utils'; @@ -312,7 +311,7 @@ function collectNativeNodes(lView: LView, tNode: TNode | null, result: any[]): a const containerTView = tNode.tViews !as TView; ngDevMode && assertDefined(containerTView, 'TView expected'); const containerFirstChildTNode = containerTView.firstChild !; - if (containerFirstChildTNode) { + if (containerFirstChildTNode !== null) { for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) { collectNativeNodes(lContainer[i], containerFirstChildTNode, result); } From 7cfeec973c8fc4bb015bc9e017121c0a9b1eb486 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 28 Oct 2019 21:30:50 -0700 Subject: [PATCH 4/5] fixup! fix(ivy): `ViewRef.rootNodes` returns rootNodes from child views as well. --- packages/core/src/render3/view_ref.ts | 6 ++++++ .../test/acceptance/view_container_ref_spec.ts | 15 ++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 8c4211d36915e..fa2794621290e 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -11,10 +11,12 @@ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detec import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref'; import {assertDefined} from '../util/assert'; + import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared'; import {CONTAINER_HEADER_OFFSET} from './interfaces/container'; import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node'; import {CONTEXT, FLAGS, HOST, LView, LViewFlags, TView, T_HOST} from './interfaces/view'; +import {assertNodeOfPossibleTypes} from './node_assert'; import {destroyLView, renderDetachView} from './node_manipulation'; import {findComponentView, getLViewParent} from './util/view_traversal_utils'; import {getNativeByTNode, getNativeByTNodeOrNull} from './util/view_utils'; @@ -302,8 +304,12 @@ export class RootViewRef extends ViewRef { function collectNativeNodes(lView: LView, tNode: TNode | null, result: any[]): any[] { while (tNode) { + ngDevMode && assertNodeOfPossibleTypes( + tNode, TNodeType.Element, TNodeType.Container, TNodeType.Projection, + TNodeType.ElementContainer); const nativeNode = getNativeByTNodeOrNull(tNode, lView); nativeNode && result.push(nativeNode); + if (tNode.type === TNodeType.ElementContainer) { collectNativeNodes(lView, tNode.child, result); } else if (tNode.type === TNodeType.Container) { diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index f77fc293c5879..a2282414efce5 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -895,6 +895,9 @@ describe('ViewContainerRef', () => { template: `
Text
+ container-content +
assert TNodeType.IcuContainer is never a root node
+ assert TNodeType.IcuContainer is never a root node `, }) @@ -917,13 +920,15 @@ describe('ViewContainerRef', () => { // - One
Text
node for the ngIf content. // - VE ONLY: Extra comment node It is unclear why the VE adds the last one. const rootNodes = dirRef.viewRef !.rootNodes; - expect(rootNodes.length).toBe(ivyEnabled ? 2 : 3); + expect(rootNodes.length).toBe(7); expect(rootNodes[0].textContent).toBe('bindings={\n "ng-reflect-ng-if": "true"\n}'); expect(rootNodes[1].outerHTML).toBe('
Text
'); - if (!ivyEnabled) { - expect(rootNodes[2].outerHTML) - .toBe(''); // It is unclear why the VE adds the last one. - } + expect(rootNodes[2].textContent).toBe(ivyEnabled ? 'ng-container' : ''); + expect(rootNodes[3].outerHTML).toBe('container-content'); + expect(rootNodes[4].outerHTML) + .toBe('
assert TNodeType.IcuContainer is never a root node
'); + expect(rootNodes[5].textContent).toBe(ivyEnabled ? 'ng-container' : ''); + expect(rootNodes[6].textContent).toBe('assert TNodeType.IcuContainer is never a root node'); }); }); From f27a91eb6a686e7dfd025dcf288a2ef75f9e84be Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 28 Oct 2019 21:30:50 -0700 Subject: [PATCH 5/5] fixup! fix(ivy): `ViewRef.rootNodes` returns rootNodes from child views as well. --- packages/core/test/acceptance/view_container_ref_spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index a2282414efce5..7798018404e00 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -924,7 +924,7 @@ describe('ViewContainerRef', () => { expect(rootNodes[0].textContent).toBe('bindings={\n "ng-reflect-ng-if": "true"\n}'); expect(rootNodes[1].outerHTML).toBe('
Text
'); expect(rootNodes[2].textContent).toBe(ivyEnabled ? 'ng-container' : ''); - expect(rootNodes[3].outerHTML).toBe('container-content'); + expect(rootNodes[3].textContent).toBe('container-content'); expect(rootNodes[4].outerHTML) .toBe('
assert TNodeType.IcuContainer is never a root node
'); expect(rootNodes[5].textContent).toBe(ivyEnabled ? 'ng-container' : '');