Skip to content

Commit e30e132

Browse files
pkozlowski-opensourcealxhub
authored andcommitted
fix(core): properly get root nodes from embedded views with <ng-content> (#36051)
This commit fixes 2 separate issues related to root nodes retrieval from embedded views with `<ng-content>`: 1) we did not account for the case where there were no projectable nodes for a given `<ng-content>`; 2) we did not account for the case where projectable nodes for a given `<ng-content>` were represented as an array of native nodes (happens in the case of dynamically created components with projectable nodes); Fixes #35967 PR Close #36051
1 parent fc6c3ae commit e30e132

File tree

3 files changed

+124
-17
lines changed

3 files changed

+124
-17
lines changed

packages/core/src/render3/component_ref.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {getComponentDef} from './definition';
2626
import {NodeInjector} from './di';
2727
import {assignTViewNodeToLView, createLView, createTView, elementCreate, locateHostElement, renderView} from './instructions/shared';
2828
import {ComponentDef} from './interfaces/definition';
29-
import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node';
29+
import {TContainerNode, TElementContainerNode, TElementNode, TNode} from './interfaces/node';
3030
import {domRendererFactory3, RendererFactory3, RNode} from './interfaces/renderer';
3131
import {LView, LViewFlags, TVIEW, TViewType} from './interfaces/view';
3232
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
@@ -201,15 +201,19 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
201201
}
202202
}
203203

204-
tElementNode = getTNode(rootLView[TVIEW], 0) as TElementNode;
205-
206-
if (projectableNodes) {
207-
// projectable nodes can be passed as array of arrays or an array of iterables (ngUpgrade
208-
// case). Here we do normalize passed data structure to be an array of arrays to avoid
209-
// complex checks down the line.
210-
tElementNode.projection = projectableNodes.map((nodesforSlot: RNode[]) => {
211-
return Array.from(nodesforSlot);
212-
});
204+
tElementNode = getTNode(rootTView, 0) as TElementNode;
205+
206+
if (projectableNodes !== undefined) {
207+
const projection: (TNode|RNode[]|null)[] = tElementNode.projection = [];
208+
for (let i = 0; i < this.ngContentSelectors.length; i++) {
209+
const nodesforSlot = projectableNodes[i];
210+
// Projectable nodes can be passed as array of arrays or an array of iterables (ngUpgrade
211+
// case). Here we do normalize passed data structure to be an array of arrays to avoid
212+
// complex checks down the line.
213+
// We also normalize the length of the passed in projectable nodes (to match the number of
214+
// <ng-container> slots defined by a component).
215+
projection.push(nodesforSlot != null ? Array.from(nodesforSlot) : null);
216+
}
213217
}
214218

215219
// TODO: should LifecycleHooksFeature and other host features be generated by the compiler and

packages/core/src/render3/view_ref.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {ApplicationRef} from '../application_ref';
1010
import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref';
1111
import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref';
1212
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref';
13-
13+
import {assertDefined} from '../util/assert';
1414
import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared';
1515
import {CONTAINER_HEADER_OFFSET} from './interfaces/container';
1616
import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
@@ -359,11 +359,22 @@ function collectNativeNodes(
359359
} else if (tNodeType === TNodeType.Projection) {
360360
const componentView = lView[DECLARATION_COMPONENT_VIEW];
361361
const componentHost = componentView[T_HOST] as TElementNode;
362-
const parentView = getLViewParent(componentView);
363-
let firstProjectedNode: TNode|null =
364-
(componentHost.projection as (TNode | null)[])[tNode.projection as number];
365-
if (firstProjectedNode !== null && parentView !== null) {
366-
collectNativeNodes(parentView[TVIEW], parentView, firstProjectedNode, result, true);
362+
const slotIdx = tNode.projection as number;
363+
ngDevMode &&
364+
assertDefined(
365+
componentHost.projection,
366+
'Components with projection nodes (<ng-content>) must have projection slots defined.');
367+
368+
const nodesInSlot = componentHost.projection![slotIdx];
369+
if (Array.isArray(nodesInSlot)) {
370+
result.push(...nodesInSlot);
371+
} else {
372+
const parentView = getLViewParent(componentView)!;
373+
ngDevMode &&
374+
assertDefined(
375+
parentView,
376+
'Component views should always have a parent view (component\'s host view)');
377+
collectNativeNodes(parentView[TVIEW], parentView, nodesInSlot, result, true);
367378
}
368379
}
369380
tNode = isProjection ? tNode.projectionNext : tNode.next;

packages/core/test/acceptance/template_ref_spec.ts

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Component, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
9+
import {Component, ComponentFactoryResolver, Injector, NgModule, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
1010
import {TestBed} from '@angular/core/testing';
1111
import {expect} from '@angular/platform-browser/testing/src/matchers';
1212
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
@@ -180,5 +180,97 @@ describe('TemplateRef', () => {
180180
expect(rootNodes.length).toBe(7);
181181
}
182182
});
183+
184+
it('should return an empty array for an embedded view with projection and no projectable nodes',
185+
() => {
186+
const rootNodes =
187+
getRootNodes(`<ng-template #templateRef><ng-content></ng-content></ng-template>`);
188+
// VE will, incorrectly, return an additional comment node in this case
189+
expect(rootNodes.length).toBe(ivyEnabled ? 0 : 1);
190+
});
191+
192+
it('should return an empty array for an embedded view with multiple projections and no projectable nodes',
193+
() => {
194+
const rootNodes = getRootNodes(
195+
`<ng-template #templateRef><ng-content></ng-content><ng-content select="foo"></ng-content></ng-template>`);
196+
// VE will, incorrectly, return an additional comment node in this case
197+
expect(rootNodes.length).toBe(ivyEnabled ? 0 : 1);
198+
});
199+
200+
describe('projectable nodes provided to a dynamically created component', () => {
201+
@Component({selector: 'dynamic', template: ''})
202+
class DynamicCmp {
203+
@ViewChild('templateRef', {static: true}) templateRef!: TemplateRef<any>;
204+
}
205+
206+
@NgModule({
207+
declarations: [DynamicCmp],
208+
entryComponents: [DynamicCmp],
209+
})
210+
class WithDynamicCmpModule {
211+
}
212+
213+
@Component({selector: 'test', template: ''})
214+
class TestCmp {
215+
constructor(public cfr: ComponentFactoryResolver) {}
216+
}
217+
218+
beforeEach(() => {
219+
TestBed.configureTestingModule({
220+
declarations: [TestCmp],
221+
imports: [WithDynamicCmpModule],
222+
});
223+
});
224+
225+
it('should return projectable nodes when provided', () => {
226+
TestBed.overrideTemplate(
227+
DynamicCmp, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);
228+
229+
const fixture = TestBed.createComponent(TestCmp);
230+
const dynamicCmptFactory =
231+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp);
232+
233+
// Number of projectable nodes matches the number of slots - all nodes should be returned
234+
const projectableNodes = [[document.createTextNode('textNode')]];
235+
const cmptRef = dynamicCmptFactory.create(Injector.NULL, projectableNodes);
236+
const viewRef = cmptRef.instance.templateRef.createEmbeddedView({});
237+
238+
// VE will, incorrectly, return an additional comment node in this case
239+
expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 1 : 2);
240+
});
241+
242+
it('should return an empty collection when no projectable nodes were provided', () => {
243+
TestBed.overrideTemplate(
244+
DynamicCmp, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);
245+
246+
const fixture = TestBed.createComponent(TestCmp);
247+
const dynamicCmptFactory =
248+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp);
249+
250+
// There are slots but projectable nodes were not provided - nothing should be returned
251+
const cmptRef = dynamicCmptFactory.create(Injector.NULL, []);
252+
const viewRef = cmptRef.instance.templateRef.createEmbeddedView({});
253+
254+
// VE will, incorrectly, return an additional comment node in this case
255+
expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 0 : 1);
256+
});
257+
258+
it('should return an empty collection when projectable nodes were provided but there are no slots',
259+
() => {
260+
TestBed.overrideTemplate(DynamicCmp, `<ng-template #templateRef></ng-template>`);
261+
262+
const fixture = TestBed.createComponent(TestCmp);
263+
const dynamicCmptFactory =
264+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicCmp);
265+
266+
// There are no slots but projectable were provided - nothing should be returned
267+
const projectableNodes = [[document.createTextNode('textNode')]];
268+
const cmptRef = dynamicCmptFactory.create(Injector.NULL, projectableNodes);
269+
const viewRef = cmptRef.instance.templateRef.createEmbeddedView({});
270+
271+
// VE will, incorrectly, return an additional comment node in this case
272+
expect(viewRef.rootNodes.length).toBe(ivyEnabled ? 0 : 1);
273+
});
274+
});
183275
});
184276
});

0 commit comments

Comments
 (0)