From e30e1325f31144396bd012dc9a38dbb4832712bf Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 13 Mar 2020 12:07:47 +0100 Subject: [PATCH] fix(core): properly get root nodes from embedded views with (#36051) This commit fixes 2 separate issues related to root nodes retrieval from embedded views with ``: 1) we did not account for the case where there were no projectable nodes for a given ``; 2) we did not account for the case where projectable nodes for a given `` were represented as an array of native nodes (happens in the case of dynamically created components with projectable nodes); Fixes #35967 PR Close #36051 --- packages/core/src/render3/component_ref.ts | 22 +++-- packages/core/src/render3/view_ref.ts | 23 +++-- .../core/test/acceptance/template_ref_spec.ts | 94 ++++++++++++++++++- 3 files changed, 123 insertions(+), 16 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 86f9908b8a..558c5f01f1 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -26,7 +26,7 @@ import {getComponentDef} from './definition'; import {NodeInjector} from './di'; import {assignTViewNodeToLView, createLView, createTView, elementCreate, locateHostElement, renderView} from './instructions/shared'; import {ComponentDef} from './interfaces/definition'; -import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node'; +import {TContainerNode, TElementContainerNode, TElementNode, TNode} from './interfaces/node'; import {domRendererFactory3, RendererFactory3, RNode} from './interfaces/renderer'; import {LView, LViewFlags, TVIEW, TViewType} from './interfaces/view'; import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces'; @@ -201,15 +201,19 @@ export class ComponentFactory extends viewEngine_ComponentFactory { } } - tElementNode = getTNode(rootLView[TVIEW], 0) as TElementNode; + tElementNode = getTNode(rootTView, 0) as TElementNode; - if (projectableNodes) { - // projectable nodes can be passed as array of arrays or an array of iterables (ngUpgrade - // case). Here we do normalize passed data structure to be an array of arrays to avoid - // complex checks down the line. - tElementNode.projection = projectableNodes.map((nodesforSlot: RNode[]) => { - return Array.from(nodesforSlot); - }); + if (projectableNodes !== undefined) { + const projection: (TNode|RNode[]|null)[] = tElementNode.projection = []; + for (let i = 0; i < this.ngContentSelectors.length; i++) { + const nodesforSlot = projectableNodes[i]; + // Projectable nodes can be passed as array of arrays or an array of iterables (ngUpgrade + // case). Here we do normalize passed data structure to be an array of arrays to avoid + // complex checks down the line. + // We also normalize the length of the passed in projectable nodes (to match the number of + // slots defined by a component). + projection.push(nodesforSlot != null ? Array.from(nodesforSlot) : null); + } } // TODO: should LifecycleHooksFeature and other host features be generated by the compiler and diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 10f67c6b66..a8bf126d9d 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -10,7 +10,7 @@ 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'; @@ -359,11 +359,22 @@ function collectNativeNodes( } else if (tNodeType === TNodeType.Projection) { const componentView = lView[DECLARATION_COMPONENT_VIEW]; const componentHost = componentView[T_HOST] as TElementNode; - const parentView = getLViewParent(componentView); - let firstProjectedNode: TNode|null = - (componentHost.projection as (TNode | null)[])[tNode.projection as number]; - if (firstProjectedNode !== null && parentView !== null) { - collectNativeNodes(parentView[TVIEW], parentView, firstProjectedNode, result, true); + const slotIdx = tNode.projection as number; + ngDevMode && + assertDefined( + componentHost.projection, + 'Components with projection nodes () must have projection slots defined.'); + + const nodesInSlot = componentHost.projection![slotIdx]; + if (Array.isArray(nodesInSlot)) { + result.push(...nodesInSlot); + } else { + const parentView = getLViewParent(componentView)!; + ngDevMode && + assertDefined( + parentView, + 'Component views should always have a parent view (component\'s host view)'); + collectNativeNodes(parentView[TVIEW], parentView, nodesInSlot, result, true); } } tNode = isProjection ? tNode.projectionNext : tNode.next; diff --git a/packages/core/test/acceptance/template_ref_spec.ts b/packages/core/test/acceptance/template_ref_spec.ts index 026dcc2c5a..432ec1ad22 100644 --- a/packages/core/test/acceptance/template_ref_spec.ts +++ b/packages/core/test/acceptance/template_ref_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; +import {Component, ComponentFactoryResolver, Injector, NgModule, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; @@ -180,5 +180,97 @@ describe('TemplateRef', () => { expect(rootNodes.length).toBe(7); } }); + + it('should return an empty array for an embedded view with projection and no projectable nodes', + () => { + const rootNodes = + getRootNodes(``); + // VE will, incorrectly, return an additional comment node in this case + expect(rootNodes.length).toBe(ivyEnabled ? 0 : 1); + }); + + it('should return an empty array for an embedded view with multiple projections and no projectable nodes', + () => { + const rootNodes = getRootNodes( + ``); + // VE will, incorrectly, return an additional comment node in this case + expect(rootNodes.length).toBe(ivyEnabled ? 0 : 1); + }); + + describe('projectable nodes provided to a dynamically created component', () => { + @Component({selector: 'dynamic', template: ''}) + class DynamicCmp { + @ViewChild('templateRef', {static: true}) templateRef!: TemplateRef; + } + + @NgModule({ + declarations: [DynamicCmp], + entryComponents: [DynamicCmp], + }) + class WithDynamicCmpModule { + } + + @Component({selector: 'test', template: ''}) + class TestCmp { + constructor(public cfr: ComponentFactoryResolver) {} + } + + beforeEach(() => { + TestBed.configureTestingModule({ + declarations: [TestCmp], + imports: [WithDynamicCmpModule], + }); + }); + + it('should return projectable nodes when provided', () => { + TestBed.overrideTemplate( + DynamicCmp, ``); + + const fixture = TestBed.createComponent(TestCmp); + const dynamicCmptFactory = + fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp); + + // Number of projectable nodes matches the number of slots - all nodes should be returned + const projectableNodes = [[document.createTextNode('textNode')]]; + const cmptRef = dynamicCmptFactory.create(Injector.NULL, projectableNodes); + const viewRef = cmptRef.instance.templateRef.createEmbeddedView({}); + + // VE will, incorrectly, return an additional comment node in this case + expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 1 : 2); + }); + + it('should return an empty collection when no projectable nodes were provided', () => { + TestBed.overrideTemplate( + DynamicCmp, ``); + + const fixture = TestBed.createComponent(TestCmp); + const dynamicCmptFactory = + fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp); + + // There are slots but projectable nodes were not provided - nothing should be returned + const cmptRef = dynamicCmptFactory.create(Injector.NULL, []); + const viewRef = cmptRef.instance.templateRef.createEmbeddedView({}); + + // VE will, incorrectly, return an additional comment node in this case + expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 0 : 1); + }); + + it('should return an empty collection when projectable nodes were provided but there are no slots', + () => { + TestBed.overrideTemplate(DynamicCmp, ``); + + const fixture = TestBed.createComponent(TestCmp); + const dynamicCmptFactory = + fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp); + + // There are no slots but projectable were provided - nothing should be returned + const projectableNodes = [[document.createTextNode('textNode')]]; + const cmptRef = dynamicCmptFactory.create(Injector.NULL, projectableNodes); + const viewRef = cmptRef.instance.templateRef.createEmbeddedView({}); + + // VE will, incorrectly, return an additional comment node in this case + expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 0 : 1); + }); + }); }); });