Skip to content

Commit

Permalink
fix(core): properly move embedded views of dynamic component's projec…
Browse files Browse the repository at this point in the history
…table nodes (#37167)

This commit fixes the issue of the ASSERTION ERROR issue when
a projected node(RNode) inside an array is checked against the types
of TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer,
TNodeType.IcuContainer, TNodeType.Projection. As it's inside an array,
it doesn't fall into any of those types, as a result, it throws
the ASSERTION ERROR.

PR Close #37120

PR Close #37167
  • Loading branch information
Shijir authored and alxhub committed Feb 10, 2021
1 parent 95446fb commit 9ae21ee
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 29 deletions.
18 changes: 16 additions & 2 deletions packages/core/src/render3/assert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
*/

import {assertDefined, assertEqual, assertNumber, throwError} from '../util/assert';

import {getComponentDef, getNgModuleDef} from './definition';
import {LContainer} from './interfaces/container';
import {DirectiveDef} from './interfaces/definition';
import {TIcu} from './interfaces/i18n';
import {NodeInjectorOffset} from './interfaces/injector';
import {TNode} from './interfaces/node';
import {isLContainer, isLView} from './interfaces/type_checks';
import {HEADER_OFFSET, LView, TVIEW, TView} from './interfaces/view';

import {DECLARATION_COMPONENT_VIEW, HEADER_OFFSET, LView, T_HOST, TVIEW, TView} from './interfaces/view';

// [Assert functions do not constraint type when they are guarded by a truthy
// expression.](https://github.com/microsoft/TypeScript/issues/37295)
Expand Down Expand Up @@ -135,6 +135,20 @@ export function assertBetween(lower: number, upper: number, index: number) {
}
}

export function assertProjectionSlots(lView: LView, errMessage?: string) {
assertDefined(lView[DECLARATION_COMPONENT_VIEW], 'Component views should exist.');
assertDefined(
lView[DECLARATION_COMPONENT_VIEW][T_HOST]!.projection,
errMessage ||
'Components with projection nodes (<ng-content>) must have projection slots defined.');
}

export function assertParentView(lView: LView|null, errMessage?: string) {
assertDefined(
lView,
errMessage || 'Component views should always have a parent view (component\'s host view)');
}


/**
* This is a basic sanity check that the `injectorIndex` seems to point to what looks like a
Expand Down
23 changes: 6 additions & 17 deletions packages/core/src/render3/collect_native_nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
* found in the LICENSE file at https://angular.io/license
*/

import {assertDefined} from '../util/assert';

import {assertParentView} from './assert';
import {icuContainerIterate} from './i18n/i18n_tree_shaking';
import {CONTAINER_HEADER_OFFSET} from './interfaces/container';
import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
import {TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
import {RNode} from './interfaces/renderer_dom';
import {isLContainer} from './interfaces/type_checks';
import {DECLARATION_COMPONENT_VIEW, LView, T_HOST, TVIEW, TView} from './interfaces/view';
import {assertTNodeType} from './node_assert';
import {getProjectionNodes} from './node_manipulation';
import {getLViewParent} from './util/view_traversal_utils';
import {unwrapRNode} from './util/view_utils';

Expand Down Expand Up @@ -58,23 +58,12 @@ export function collectNativeNodes(
result.push(rNode);
}
} else if (tNodeType & TNodeType.Projection) {
const componentView = lView[DECLARATION_COMPONENT_VIEW];
const componentHost = componentView[T_HOST] as TElementNode;
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];
const nodesInSlot = getProjectionNodes(lView, tNode);
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)');
const parentView = getLViewParent(lView[DECLARATION_COMPONENT_VIEW])!;
ngDevMode && assertParentView(parentView);
collectNativeNodes(parentView[TVIEW], parentView, nodesInSlot, result, true);
}
}
Expand Down
30 changes: 21 additions & 9 deletions packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {RendererStyleFlags2} from '../render/api_flags';
import {addToArray, removeFromArray} from '../util/array_utils';
import {assertDefined, assertDomNode, assertEqual, assertFunction, assertString} from '../util/assert';
import {escapeCommentText} from '../util/dom';
import {assertLContainer, assertLView, assertTNodeForLView} from './assert';

import {assertLContainer, assertLView, assertParentView, assertProjectionSlots, assertTNodeForLView} from './assert';
import {attachPatchData} from './context_discovery';
import {icuContainerIterate} from './i18n/i18n_tree_shaking';
import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
Expand Down Expand Up @@ -772,14 +773,14 @@ function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
// If the ICU container has no nodes, than we use the ICU anchor as the node.
return rNode || unwrapRNode(lView[tNode.index]);
} else {
const componentView = lView[DECLARATION_COMPONENT_VIEW];
const componentHost = componentView[T_HOST] as TElementNode;
const parentView = getLViewParent(componentView);
const firstProjectedTNode: TNode|null =
(componentHost.projection as (TNode | null)[])[tNode.projection as number];

if (firstProjectedTNode != null) {
return getFirstNativeNode(parentView!, firstProjectedTNode);
const projectionNodes = getProjectionNodes(lView, tNode);
if (projectionNodes !== null) {
if (Array.isArray(projectionNodes)) {
return projectionNodes[0];
}
const parentView = getLViewParent(lView[DECLARATION_COMPONENT_VIEW]);
ngDevMode && assertParentView(parentView);
return getFirstNativeNode(parentView!, projectionNodes);
} else {
return getFirstNativeNode(lView, tNode.next);
}
Expand All @@ -789,6 +790,17 @@ function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
return null;
}

export function getProjectionNodes(lView: LView, tNode: TNode|null): TNode|RNode[]|null {
if (tNode !== null) {
const componentView = lView[DECLARATION_COMPONENT_VIEW];
const componentHost = componentView[T_HOST] as TElementNode;
const slotIdx = tNode.projection as number;
ngDevMode && assertProjectionSlots(lView);
return componentHost.projection![slotIdx];
}
return null;
}

export function getBeforeNodeForView(viewIndexInContainer: number, lContainer: LContainer): RNode|
null {
const nextViewIndex = CONTAINER_HEADER_OFFSET + viewIndexInContainer + 1;
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/forms/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,9 @@
{
"name": "getPreviousIndex"
},
{
"name": "getProjectionNodes"
},
{
"name": "getPromiseCtor"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,9 @@
{
"name": "getPreviousIndex"
},
{
"name": "getProjectionNodes"
},
{
"name": "getPromiseCtor"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,9 @@
{
"name": "getPreviousIndex"
},
{
"name": "getProjectionNodes"
},
{
"name": "getSelectedIndex"
},
Expand Down
155 changes: 154 additions & 1 deletion packages/core/test/linker/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {CommonModule, DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
import {Compiler, ComponentFactory, ComponentRef, ErrorHandler, EventEmitter, Host, Inject, Injectable, InjectionToken, Injector, NgModule, NgModuleRef, NO_ERRORS_SCHEMA, OnDestroy, SkipSelf, ViewRef, ɵivyEnabled as ivyEnabled} from '@angular/core';
import {Compiler, ComponentFactory, ComponentRef, ErrorHandler, EventEmitter, Host, Inject, Injectable, InjectionToken, Injector, NgModule, NgModuleRef, NO_ERRORS_SCHEMA, OnDestroy, SkipSelf, ViewChild, ViewRef, ɵivyEnabled as ivyEnabled} from '@angular/core';
import {ChangeDetectionStrategy, ChangeDetectorRef, PipeTransform} from '@angular/core/src/change_detection/change_detection';
import {getDebugContext} from '@angular/core/src/errors';
import {ComponentFactoryResolver} from '@angular/core/src/linker/component_factory_resolver';
Expand Down Expand Up @@ -1641,6 +1641,159 @@ function declareTests(config?: {useJit: boolean}) {
expect(fixture.nativeElement).toHaveText('');
});


describe('moving embedded views of projectable nodes in a dynamic component', () => {
@Component({selector: 'menu-item', template: ''})
class DynamicMenuItem {
@ViewChild('templateRef', {static: true}) templateRef!: TemplateRef<any>;
itemContent!: string;
}

@NgModule({
declarations: [DynamicMenuItem],
entryComponents: [DynamicMenuItem],
})
class DynamicMenuItemModule {
}

@Component({selector: 'test', template: `<ng-container #menuItemsContainer></ng-container>`})
class TestCmp {
constructor(public cfr: ComponentFactoryResolver) {}
@ViewChild('menuItemsContainer', {static: true, read: ViewContainerRef})
menuItemsContainer!: ViewContainerRef;
}

beforeEach(() => {
TestBed.configureTestingModule({
declarations: [TestCmp],
imports: [DynamicMenuItemModule],
});
});

const createElWithContent = (content: string, tagName = 'span') => {
const element = document.createElement(tagName);
element.textContent = content;
return element;
};

it('should support moving embedded views of projectable nodes', () => {
TestBed.overrideTemplate(
DynamicMenuItem, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);

const fixture = TestBed.createComponent(TestCmp);
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
const dynamicCmptFactory =
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);

const cmptRefWithAa =
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Aa')]]);
const cmptRefWithBb =
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Bb')]]);
const cmptRefWithCc =
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Cc')]]);

menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));

menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
expect(fixture.nativeElement.textContent).toBe('BbAaCc');
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
expect(fixture.nativeElement.textContent).toBe('BbCcAa');
});

it('should support moving embedded views of projectable nodes in multiple slots', () => {
TestBed.overrideTemplate(
DynamicMenuItem,
`<ng-template #templateRef><ng-content select="span"></ng-content><ng-content select="button"></ng-content></ng-template>`);

const fixture = TestBed.createComponent(TestCmp);
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
const dynamicCmptFactory =
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);

const cmptRefWithAa = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('A')], [createElWithContent('a', 'button')]]);
const cmptRefWithBb = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('B')], [createElWithContent('b', 'button')]]);
const cmptRefWithCc = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('C')], [createElWithContent('c', 'button')]]);

menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));

menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
expect(fixture.nativeElement.textContent).toBe('BbAaCc');
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
expect(fixture.nativeElement.textContent).toBe('BbCcAa');
});

it('should support moving embedded views of projectable nodes in multiple slots and interpolations',
() => {
TestBed.overrideTemplate(
DynamicMenuItem,
`<ng-template #templateRef><ng-content select="span"></ng-content>{{itemContent}}<ng-content select="button"></ng-content></ng-template>`);

TestBed.configureTestingModule(
{declarations: [TestCmp], imports: [DynamicMenuItemModule]});

const fixture = TestBed.createComponent(TestCmp);
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
const dynamicCmptFactory =
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);

const cmptRefWithAa = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('A')], [createElWithContent('a', 'button')]]);
const cmptRefWithBb = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('B')], [createElWithContent('b', 'button')]]);
const cmptRefWithCc = dynamicCmptFactory.create(
Injector.NULL, [[createElWithContent('C')], [createElWithContent('c', 'button')]]);

menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));

cmptRefWithAa.instance.itemContent = '0';
cmptRefWithBb.instance.itemContent = '1';
cmptRefWithCc.instance.itemContent = '2';

fixture.detectChanges();

menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
expect(fixture.nativeElement.textContent).toBe('B1bA0aC2c');
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
expect(fixture.nativeElement.textContent).toBe('B1bC2cA0a');
});

it('should support moving embedded views with empty projectable slots', () => {
TestBed.overrideTemplate(
DynamicMenuItem, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);

const fixture = TestBed.createComponent(TestCmp);
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
const dynamicCmptFactory =
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);

const cmptRefWithAa = dynamicCmptFactory.create(Injector.NULL, [[]]);
const cmptRefWithBb =
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Bb')]]);
const cmptRefWithCc =
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Cc')]]);

menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));

menuItemsContainer.move(menuItemsContainer.get(0)!, 1); // [ Bb, NULL, Cc]
expect(fixture.nativeElement.textContent).toBe('BbCc');
menuItemsContainer.move(menuItemsContainer.get(2)!, 1); // [ Bb, Cc, NULL]
expect(fixture.nativeElement.textContent).toBe('BbCc');
menuItemsContainer.move(menuItemsContainer.get(0)!, 1); // [ Cc, Bb, NULL]
expect(fixture.nativeElement.textContent).toBe('CcBb');
});
});

describe('Property bindings', () => {
modifiedInIvy('Unknown property error throws an error instead of logging it')
.it('should throw on bindings to unknown properties', () => {
Expand Down

0 comments on commit 9ae21ee

Please sign in to comment.