Skip to content

Commit c539a45

Browse files
committed
fix(core): correct container anchor collection order to match DOM layout
Historically, `collectNativeNodes` collected the container's anchor comment node (`LContainer[NATIVE]`) before descending into the views contained inside the `LContainer`. While this worked logically, it did not match the actual physical layout of the DOM tree, where dynamic view content is inserted before the container anchor. This discrepancy was particularly visible in projected `@content` blocks where the anchor comment ended up rendered at the beginning instead of the end of the content block. This commit refactors `collectNativeNodes` to collect container nodes in the expected order: 1. Push the host element for dynamic containers where where `lContainer[NATIVE] !== lContainer[HOST]` (e.g., a `ViewContainerRef` injected on a `div` element). 2. Collect nodes in the container. 3. _Unconditionally_ push the container anchor comment. Associated acceptance tests in `template_ref_spec.ts` are updated to match the physically correct DOM order.
1 parent 79e5d5d commit c539a45

4 files changed

Lines changed: 28 additions & 34 deletions

File tree

packages/core/src/hydration/annotate.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -337,12 +337,11 @@ function serializeLContainer(
337337
// host element was used as a ViewContainerRef anchor (e.g. a `ViewContainerRef`
338338
// was injected within the component class). This case requires special handling.
339339
if (isLContainer(childLView)) {
340-
// Calculate the number of root nodes in all views in a given container
341-
// and increment by one to account for an anchor node itself, i.e. in this
342-
// scenario we'll have a layout that would look like this:
340+
// Calculate the number of root nodes in all attached views and add 2 to account for the
341+
// component's host node and the container's anchor node. i.e. in this scenario we'll have a
342+
// layout that would look like this:
343343
// `<app-root /><#VIEW1><#VIEW2>...<!--container-->`
344-
// The `+1` is to capture the `<app-root />` element.
345-
numRootNodes = calcNumRootNodesInLContainer(childLView) + 1;
344+
numRootNodes = calcNumRootNodesInLContainer(childLView) + 2;
346345

347346
annotateLContainerForHydration(childLView, context);
348347

packages/core/src/render3/collect_native_nodes.ts

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,26 @@ export function collectNativeNodes(
6262

6363
const lNode = lView[tNode.index];
6464
if (lNode !== null) {
65-
result.push(unwrapRNode(lNode));
66-
}
65+
if (isLContainer(lNode)) {
66+
const anchor = lNode[NATIVE];
67+
// If this is a dynamic container (ViewContainerRef on an element), the HOST element is a
68+
// distinct element node and must be pushed first (before the views are collected) to
69+
// preserve DOM order.
70+
if (anchor !== lNode[HOST]) {
71+
result.push(unwrapRNode(lNode));
72+
}
73+
74+
// Collect the root nodes from the views in this container.
75+
if (!(lNode[FLAGS] & LContainerFlags.LogicalOnly)) {
76+
collectNativeNodesInLContainer(lNode, result);
77+
}
6778

68-
// A given lNode can represent either a native node or a LContainer (when it is a host of a
69-
// ViewContainerRef). When we find a LContainer we need to descend into it to collect root nodes
70-
// from the views in this container.
71-
if (isLContainer(lNode) && !(lNode[FLAGS] & LContainerFlags.LogicalOnly)) {
72-
collectNativeNodesInLContainer(lNode, result);
79+
// The container's anchor comment node is always physically positioned after any views
80+
// rendered inside the container, so we always push it here at the end.
81+
result.push(anchor);
82+
} else {
83+
result.push(unwrapRNode(lNode));
84+
}
7385
}
7486

7587
const tNodeType = tNode.type;
@@ -108,21 +120,4 @@ export function collectNativeNodesInLContainer(lContainer: LContainer, result: a
108120
collectNativeNodes(lViewInAContainer[TVIEW], lViewInAContainer, lViewFirstChildTNode, result);
109121
}
110122
}
111-
112-
// When an LContainer is created, the anchor (comment) node is:
113-
// - (1) either reused in case of an ElementContainer (<ng-container>)
114-
// - (2) or a new comment node is created
115-
// In the first case, the anchor comment node would be added to the final
116-
// list by the code in the `collectNativeNodes` function
117-
// (see the `result.push(unwrapRNode(lNode))` line), but the second
118-
// case requires extra handling: the anchor node needs to be added to the
119-
// final list manually. See additional information in the `createAnchorNode`
120-
// function in the `view_container_ref.ts`.
121-
//
122-
// In the first case, the same reference would be stored in the `NATIVE`
123-
// and `HOST` slots in an LContainer. Otherwise, this is the second case and
124-
// we should add an element to the final list.
125-
if (lContainer[NATIVE] !== lContainer[HOST]) {
126-
result.push(lContainer[NATIVE]);
127-
}
128123
}

packages/core/test/acceptance/foreign_component/foreign_component_spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,12 +279,12 @@ describe('foreign components', () => {
279279
'<!--foreign-view-head-->' +
280280
'<div class="wrapper">' +
281281
'<!--container-->' +
282-
'<!--foreign-component-->' +
283282
'<!--foreign-view-head-->' +
284283
'<button>' +
285284
'<span id="text">Inside wrapper button</span>' +
286285
'</button>' +
287286
'<!--foreign-view-tail-->' +
287+
'<!--foreign-component-->' +
288288
'</div>' +
289289
'<!--foreign-view-tail-->' +
290290
'<!--foreign-component-->',

packages/core/test/acceptance/template_ref_spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ describe('TemplateRef', () => {
115115
</ng-template>`);
116116

117117
expect(rootNodes.length).toBe(3);
118-
expect(rootNodes[0].nodeType).toBe(Node.COMMENT_NODE);
119-
expect(rootNodes[1].nodeType).toBe(Node.TEXT_NODE);
118+
expect(rootNodes[0].nodeType).toBe(Node.TEXT_NODE);
119+
expect(rootNodes[1].nodeType).toBe(Node.COMMENT_NODE);
120120
expect(rootNodes[2].nodeType).toBe(Node.TEXT_NODE);
121121
});
122122

@@ -153,8 +153,8 @@ describe('TemplateRef', () => {
153153
`);
154154

155155
expect(rootNodes.length).toBe(3);
156-
expect(rootNodes[0].nodeType).toBe(Node.COMMENT_NODE);
157-
expect(rootNodes[1].nodeType).toBe(Node.TEXT_NODE);
156+
expect(rootNodes[0].nodeType).toBe(Node.TEXT_NODE);
157+
expect(rootNodes[1].nodeType).toBe(Node.COMMENT_NODE);
158158
expect(rootNodes[2].nodeType).toBe(Node.TEXT_NODE);
159159
});
160160

0 commit comments

Comments
 (0)