Skip to content

Commit 9ae21ee

Browse files
Shijiralxhub
Shijir
authored and
alxhub
committed
fix(core): properly move embedded views of dynamic component's projectable 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
1 parent 95446fb commit 9ae21ee

File tree

7 files changed

+206
-29
lines changed

7 files changed

+206
-29
lines changed

‎packages/core/src/render3/assert.ts

+16-2
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77
*/
88

99
import {assertDefined, assertEqual, assertNumber, throwError} from '../util/assert';
10+
1011
import {getComponentDef, getNgModuleDef} from './definition';
1112
import {LContainer} from './interfaces/container';
1213
import {DirectiveDef} from './interfaces/definition';
1314
import {TIcu} from './interfaces/i18n';
1415
import {NodeInjectorOffset} from './interfaces/injector';
1516
import {TNode} from './interfaces/node';
1617
import {isLContainer, isLView} from './interfaces/type_checks';
17-
import {HEADER_OFFSET, LView, TVIEW, TView} from './interfaces/view';
18-
18+
import {DECLARATION_COMPONENT_VIEW, HEADER_OFFSET, LView, T_HOST, TVIEW, TView} from './interfaces/view';
1919

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

138+
export function assertProjectionSlots(lView: LView, errMessage?: string) {
139+
assertDefined(lView[DECLARATION_COMPONENT_VIEW], 'Component views should exist.');
140+
assertDefined(
141+
lView[DECLARATION_COMPONENT_VIEW][T_HOST]!.projection,
142+
errMessage ||
143+
'Components with projection nodes (<ng-content>) must have projection slots defined.');
144+
}
145+
146+
export function assertParentView(lView: LView|null, errMessage?: string) {
147+
assertDefined(
148+
lView,
149+
errMessage || 'Component views should always have a parent view (component\'s host view)');
150+
}
151+
138152

139153
/**
140154
* This is a basic sanity check that the `injectorIndex` seems to point to what looks like a

‎packages/core/src/render3/collect_native_nodes.ts

+6-17
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {assertDefined} from '../util/assert';
10-
9+
import {assertParentView} from './assert';
1110
import {icuContainerIterate} from './i18n/i18n_tree_shaking';
1211
import {CONTAINER_HEADER_OFFSET} from './interfaces/container';
13-
import {TElementNode, TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
12+
import {TIcuContainerNode, TNode, TNodeType} from './interfaces/node';
1413
import {RNode} from './interfaces/renderer_dom';
1514
import {isLContainer} from './interfaces/type_checks';
1615
import {DECLARATION_COMPONENT_VIEW, LView, T_HOST, TVIEW, TView} from './interfaces/view';
1716
import {assertTNodeType} from './node_assert';
17+
import {getProjectionNodes} from './node_manipulation';
1818
import {getLViewParent} from './util/view_traversal_utils';
1919
import {unwrapRNode} from './util/view_utils';
2020

@@ -58,23 +58,12 @@ export function collectNativeNodes(
5858
result.push(rNode);
5959
}
6060
} else if (tNodeType & TNodeType.Projection) {
61-
const componentView = lView[DECLARATION_COMPONENT_VIEW];
62-
const componentHost = componentView[T_HOST] as TElementNode;
63-
const slotIdx = tNode.projection as number;
64-
ngDevMode &&
65-
assertDefined(
66-
componentHost.projection,
67-
'Components with projection nodes (<ng-content>) must have projection slots defined.');
68-
69-
const nodesInSlot = componentHost.projection![slotIdx];
61+
const nodesInSlot = getProjectionNodes(lView, tNode);
7062
if (Array.isArray(nodesInSlot)) {
7163
result.push(...nodesInSlot);
7264
} else {
73-
const parentView = getLViewParent(componentView)!;
74-
ngDevMode &&
75-
assertDefined(
76-
parentView,
77-
'Component views should always have a parent view (component\'s host view)');
65+
const parentView = getLViewParent(lView[DECLARATION_COMPONENT_VIEW])!;
66+
ngDevMode && assertParentView(parentView);
7867
collectNativeNodes(parentView[TVIEW], parentView, nodesInSlot, result, true);
7968
}
8069
}

‎packages/core/src/render3/node_manipulation.ts

+21-9
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import {RendererStyleFlags2} from '../render/api_flags';
1212
import {addToArray, removeFromArray} from '../util/array_utils';
1313
import {assertDefined, assertDomNode, assertEqual, assertFunction, assertString} from '../util/assert';
1414
import {escapeCommentText} from '../util/dom';
15-
import {assertLContainer, assertLView, assertTNodeForLView} from './assert';
15+
16+
import {assertLContainer, assertLView, assertParentView, assertProjectionSlots, assertTNodeForLView} from './assert';
1617
import {attachPatchData} from './context_discovery';
1718
import {icuContainerIterate} from './i18n/i18n_tree_shaking';
1819
import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
@@ -772,14 +773,14 @@ function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
772773
// If the ICU container has no nodes, than we use the ICU anchor as the node.
773774
return rNode || unwrapRNode(lView[tNode.index]);
774775
} else {
775-
const componentView = lView[DECLARATION_COMPONENT_VIEW];
776-
const componentHost = componentView[T_HOST] as TElementNode;
777-
const parentView = getLViewParent(componentView);
778-
const firstProjectedTNode: TNode|null =
779-
(componentHost.projection as (TNode | null)[])[tNode.projection as number];
780-
781-
if (firstProjectedTNode != null) {
782-
return getFirstNativeNode(parentView!, firstProjectedTNode);
776+
const projectionNodes = getProjectionNodes(lView, tNode);
777+
if (projectionNodes !== null) {
778+
if (Array.isArray(projectionNodes)) {
779+
return projectionNodes[0];
780+
}
781+
const parentView = getLViewParent(lView[DECLARATION_COMPONENT_VIEW]);
782+
ngDevMode && assertParentView(parentView);
783+
return getFirstNativeNode(parentView!, projectionNodes);
783784
} else {
784785
return getFirstNativeNode(lView, tNode.next);
785786
}
@@ -789,6 +790,17 @@ function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
789790
return null;
790791
}
791792

793+
export function getProjectionNodes(lView: LView, tNode: TNode|null): TNode|RNode[]|null {
794+
if (tNode !== null) {
795+
const componentView = lView[DECLARATION_COMPONENT_VIEW];
796+
const componentHost = componentView[T_HOST] as TElementNode;
797+
const slotIdx = tNode.projection as number;
798+
ngDevMode && assertProjectionSlots(lView);
799+
return componentHost.projection![slotIdx];
800+
}
801+
return null;
802+
}
803+
792804
export function getBeforeNodeForView(viewIndexInContainer: number, lContainer: LContainer): RNode|
793805
null {
794806
const nextViewIndex = CONTAINER_HEADER_OFFSET + viewIndexInContainer + 1;

‎packages/core/test/bundling/forms/bundle.golden_symbols.json

+3
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,9 @@
10881088
{
10891089
"name": "getPreviousIndex"
10901090
},
1091+
{
1092+
"name": "getProjectionNodes"
1093+
},
10911094
{
10921095
"name": "getPromiseCtor"
10931096
},

‎packages/core/test/bundling/router/bundle.golden_symbols.json

+3
Original file line numberDiff line numberDiff line change
@@ -1403,6 +1403,9 @@
14031403
{
14041404
"name": "getPreviousIndex"
14051405
},
1406+
{
1407+
"name": "getProjectionNodes"
1408+
},
14061409
{
14071410
"name": "getPromiseCtor"
14081411
},

‎packages/core/test/bundling/todo/bundle.golden_symbols.json

+3
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@
437437
{
438438
"name": "getPreviousIndex"
439439
},
440+
{
441+
"name": "getProjectionNodes"
442+
},
440443
{
441444
"name": "getSelectedIndex"
442445
},

‎packages/core/test/linker/integration_spec.ts

+154-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {CommonModule, DOCUMENT, ɵgetDOM as getDOM} from '@angular/common';
10-
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';
10+
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';
1111
import {ChangeDetectionStrategy, ChangeDetectorRef, PipeTransform} from '@angular/core/src/change_detection/change_detection';
1212
import {getDebugContext} from '@angular/core/src/errors';
1313
import {ComponentFactoryResolver} from '@angular/core/src/linker/component_factory_resolver';
@@ -1641,6 +1641,159 @@ function declareTests(config?: {useJit: boolean}) {
16411641
expect(fixture.nativeElement).toHaveText('');
16421642
});
16431643

1644+
1645+
describe('moving embedded views of projectable nodes in a dynamic component', () => {
1646+
@Component({selector: 'menu-item', template: ''})
1647+
class DynamicMenuItem {
1648+
@ViewChild('templateRef', {static: true}) templateRef!: TemplateRef<any>;
1649+
itemContent!: string;
1650+
}
1651+
1652+
@NgModule({
1653+
declarations: [DynamicMenuItem],
1654+
entryComponents: [DynamicMenuItem],
1655+
})
1656+
class DynamicMenuItemModule {
1657+
}
1658+
1659+
@Component({selector: 'test', template: `<ng-container #menuItemsContainer></ng-container>`})
1660+
class TestCmp {
1661+
constructor(public cfr: ComponentFactoryResolver) {}
1662+
@ViewChild('menuItemsContainer', {static: true, read: ViewContainerRef})
1663+
menuItemsContainer!: ViewContainerRef;
1664+
}
1665+
1666+
beforeEach(() => {
1667+
TestBed.configureTestingModule({
1668+
declarations: [TestCmp],
1669+
imports: [DynamicMenuItemModule],
1670+
});
1671+
});
1672+
1673+
const createElWithContent = (content: string, tagName = 'span') => {
1674+
const element = document.createElement(tagName);
1675+
element.textContent = content;
1676+
return element;
1677+
};
1678+
1679+
it('should support moving embedded views of projectable nodes', () => {
1680+
TestBed.overrideTemplate(
1681+
DynamicMenuItem, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);
1682+
1683+
const fixture = TestBed.createComponent(TestCmp);
1684+
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
1685+
const dynamicCmptFactory =
1686+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);
1687+
1688+
const cmptRefWithAa =
1689+
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Aa')]]);
1690+
const cmptRefWithBb =
1691+
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Bb')]]);
1692+
const cmptRefWithCc =
1693+
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Cc')]]);
1694+
1695+
menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
1696+
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
1697+
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));
1698+
1699+
menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
1700+
expect(fixture.nativeElement.textContent).toBe('BbAaCc');
1701+
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
1702+
expect(fixture.nativeElement.textContent).toBe('BbCcAa');
1703+
});
1704+
1705+
it('should support moving embedded views of projectable nodes in multiple slots', () => {
1706+
TestBed.overrideTemplate(
1707+
DynamicMenuItem,
1708+
`<ng-template #templateRef><ng-content select="span"></ng-content><ng-content select="button"></ng-content></ng-template>`);
1709+
1710+
const fixture = TestBed.createComponent(TestCmp);
1711+
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
1712+
const dynamicCmptFactory =
1713+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);
1714+
1715+
const cmptRefWithAa = dynamicCmptFactory.create(
1716+
Injector.NULL, [[createElWithContent('A')], [createElWithContent('a', 'button')]]);
1717+
const cmptRefWithBb = dynamicCmptFactory.create(
1718+
Injector.NULL, [[createElWithContent('B')], [createElWithContent('b', 'button')]]);
1719+
const cmptRefWithCc = dynamicCmptFactory.create(
1720+
Injector.NULL, [[createElWithContent('C')], [createElWithContent('c', 'button')]]);
1721+
1722+
menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
1723+
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
1724+
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));
1725+
1726+
menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
1727+
expect(fixture.nativeElement.textContent).toBe('BbAaCc');
1728+
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
1729+
expect(fixture.nativeElement.textContent).toBe('BbCcAa');
1730+
});
1731+
1732+
it('should support moving embedded views of projectable nodes in multiple slots and interpolations',
1733+
() => {
1734+
TestBed.overrideTemplate(
1735+
DynamicMenuItem,
1736+
`<ng-template #templateRef><ng-content select="span"></ng-content>{{itemContent}}<ng-content select="button"></ng-content></ng-template>`);
1737+
1738+
TestBed.configureTestingModule(
1739+
{declarations: [TestCmp], imports: [DynamicMenuItemModule]});
1740+
1741+
const fixture = TestBed.createComponent(TestCmp);
1742+
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
1743+
const dynamicCmptFactory =
1744+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);
1745+
1746+
const cmptRefWithAa = dynamicCmptFactory.create(
1747+
Injector.NULL, [[createElWithContent('A')], [createElWithContent('a', 'button')]]);
1748+
const cmptRefWithBb = dynamicCmptFactory.create(
1749+
Injector.NULL, [[createElWithContent('B')], [createElWithContent('b', 'button')]]);
1750+
const cmptRefWithCc = dynamicCmptFactory.create(
1751+
Injector.NULL, [[createElWithContent('C')], [createElWithContent('c', 'button')]]);
1752+
1753+
menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
1754+
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
1755+
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));
1756+
1757+
cmptRefWithAa.instance.itemContent = '0';
1758+
cmptRefWithBb.instance.itemContent = '1';
1759+
cmptRefWithCc.instance.itemContent = '2';
1760+
1761+
fixture.detectChanges();
1762+
1763+
menuItemsContainer.move(menuItemsContainer.get(0)!, 1);
1764+
expect(fixture.nativeElement.textContent).toBe('B1bA0aC2c');
1765+
menuItemsContainer.move(menuItemsContainer.get(2)!, 1);
1766+
expect(fixture.nativeElement.textContent).toBe('B1bC2cA0a');
1767+
});
1768+
1769+
it('should support moving embedded views with empty projectable slots', () => {
1770+
TestBed.overrideTemplate(
1771+
DynamicMenuItem, `<ng-template #templateRef><ng-content></ng-content></ng-template>`);
1772+
1773+
const fixture = TestBed.createComponent(TestCmp);
1774+
const menuItemsContainer = fixture.componentInstance.menuItemsContainer;
1775+
const dynamicCmptFactory =
1776+
fixture.componentInstance.cfr.resolveComponentFactory(DynamicMenuItem);
1777+
1778+
const cmptRefWithAa = dynamicCmptFactory.create(Injector.NULL, [[]]);
1779+
const cmptRefWithBb =
1780+
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Bb')]]);
1781+
const cmptRefWithCc =
1782+
dynamicCmptFactory.create(Injector.NULL, [[createElWithContent('Cc')]]);
1783+
1784+
menuItemsContainer.insert(cmptRefWithAa.instance.templateRef.createEmbeddedView({}));
1785+
menuItemsContainer.insert(cmptRefWithBb.instance.templateRef.createEmbeddedView({}));
1786+
menuItemsContainer.insert(cmptRefWithCc.instance.templateRef.createEmbeddedView({}));
1787+
1788+
menuItemsContainer.move(menuItemsContainer.get(0)!, 1); // [ Bb, NULL, Cc]
1789+
expect(fixture.nativeElement.textContent).toBe('BbCc');
1790+
menuItemsContainer.move(menuItemsContainer.get(2)!, 1); // [ Bb, Cc, NULL]
1791+
expect(fixture.nativeElement.textContent).toBe('BbCc');
1792+
menuItemsContainer.move(menuItemsContainer.get(0)!, 1); // [ Cc, Bb, NULL]
1793+
expect(fixture.nativeElement.textContent).toBe('CcBb');
1794+
});
1795+
});
1796+
16441797
describe('Property bindings', () => {
16451798
modifiedInIvy('Unknown property error throws an error instead of logging it')
16461799
.it('should throw on bindings to unknown properties', () => {

0 commit comments

Comments
 (0)