Skip to content

Commit 7217733

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 e695379 commit 7217733

3 files changed

Lines changed: 25 additions & 33 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: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,24 @@ export function collectNativeNodes(
5454

5555
const lNode = lView[tNode.index];
5656
if (lNode !== null) {
57-
result.push(unwrapRNode(lNode));
58-
}
57+
if (isLContainer(lNode)) {
58+
const anchor = lNode[NATIVE];
59+
// If this is a dynamic container (ViewContainerRef on an element), the HOST element is a
60+
// distinct element node and must be pushed first (before the views are collected) to
61+
// preserve DOM order.
62+
if (anchor !== lNode[HOST]) {
63+
result.push(unwrapRNode(lNode));
64+
}
65+
66+
// Collect the root nodes from the views in this container.
67+
collectNativeNodesInLContainer(lNode, result);
5968

60-
// A given lNode can represent either a native node or a LContainer (when it is a host of a
61-
// ViewContainerRef). When we find a LContainer we need to descend into it to collect root nodes
62-
// from the views in this container.
63-
if (isLContainer(lNode)) {
64-
collectNativeNodesInLContainer(lNode, result);
69+
// The container's anchor comment node is always physically positioned after any views
70+
// rendered inside the container, so we always push it here at the end.
71+
result.push(anchor);
72+
} else {
73+
result.push(unwrapRNode(lNode));
74+
}
6575
}
6676

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

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)