Skip to content

Commit

Permalink
fix(core): properly get root nodes from embedded views with <ng-content>
Browse files Browse the repository at this point in the history
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 angular#35967
  • Loading branch information
pkozlowski-opensource committed Apr 20, 2020
1 parent d7e9d87 commit 85080f0
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 17 deletions.
24 changes: 14 additions & 10 deletions packages/core/src/render3/component_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -201,15 +201,19 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
}
}

tElementNode = getTNode(rootLView[TVIEW], 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);
});
tElementNode = getTNode(rootTView, 0) as TElementNode;

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
// <ng-container> 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
Expand Down
23 changes: 17 additions & 6 deletions packages/core/src/render3/view_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 (<ng-content>) 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;
Expand Down
94 changes: 93 additions & 1 deletion packages/core/test/acceptance/template_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(`<ng-template #templateRef><ng-content></ng-content></ng-template>`);
// 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(
`<ng-template #templateRef><ng-content></ng-content><ng-content select="foo"></ng-content></ng-template>`);
// 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<any>;
}

@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, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);

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, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);

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, `<ng-template #templateRef></ng-template>`);

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);
});
});
});
});

0 comments on commit 85080f0

Please sign in to comment.