refactor(ivy): merge canInsertNativeNode and getParentNative (#28011)

Previously the canInsertNativeNode and getRenderParent functions had almost
_exaclty_ the same logic. What was worse that getRenderParent was calling
canInsertNativeNode thus executing the same, non-trivial logic twice.

This commit merges canInsertNativeNode and getRenderParent into one function.
Now getRenderParent will return a native parent or null if a node can't be
inserted (content projection, root of a view that is not inserted etc.).

PR Close #28011

packages/core/src/render3/node_manipulation.ts

@@ -18,23 +18,6 @@ import {findComponentView, getNativeByTNode, isLContainer, isRootView, readEleme
 
 const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4 + unused5;
 
-/** Retrieves the native node (element or a comment) for the parent of a given node. */
-function getParentNative(tNode: TNode, currentView: LView): RElement|RComment|null {
-  return tNode.parent == null ? getHostNative(currentView) :
-                                getNativeByTNode(tNode.parent, currentView);
-}
-
-/**
- * Gets the host element given a view. Will return null if the current view is an embedded view,
- * which does not have a host element.
- */
-function getHostNative(currentView: LView): RElement|null {
-  const hostTNode = currentView[HOST_NODE] as TElementNode;
-  return hostTNode && hostTNode.type !== TNodeType.View ?
-      (getNativeByTNode(hostTNode, currentView[PARENT] !) as RElement) :
-      null;
-}
-
 export function getLContainer(tNode: TViewNode, embeddedView: LView): LContainer|null {
   if (tNode.index === -1) {
     // This is a dynamically created view inside a dynamic container.
@@ -485,51 +468,8 @@ function executeOnDestroys(view: LView): void {
   }
 }
 
-function getRenderParent(tNode: TNode, currentView: LView): RElement|null {
-  if (canInsertNativeNode(tNode, currentView)) {
-    // If we are asked for a render parent of the root component we need to do low-level DOM
-    // operation as LTree doesn't exist above the topmost host node. We might need to find a render
-    // parent of the topmost host node if the root component injects ViewContainerRef.
-    if (isRootView(currentView)) {
-      return nativeParentNode(currentView[RENDERER], getNativeByTNode(tNode, currentView));
-    }
-
-    // skip over element and ICU containers as those are represented by a comment node and
-    // can't be used as a render parent
-    tNode = getHighestElementOrICUContainer(tNode);
-
-    const hostTNode = currentView[HOST_NODE];
-    return tNode.parent == null && hostTNode !.type === TNodeType.View ?
-        getContainerRenderParent(hostTNode as TViewNode, currentView) :
-        getParentNative(tNode, currentView) as RElement;
-  }
-  return null;
-}
-
-function canInsertNativeChildOfElement(tNode: TElementNode): boolean {
-  // Component's content nodes are not inserted immediately
-  // because they will be projected, and so doing insert at this point would be wasteful.
-  // Since the projection would than move it to its final destination.
-  return !(tNode.flags & TNodeFlags.isComponent);
-}
-
 /**
- * We might delay insertion of children for a given view if it is disconnected.
+ * Returns a native element if a node can be inserted into the given parent.
- * This might happen for 2 main reasons:
- * - view is not inserted into any container (view was created but not inserted yet)
- * - view is inserted into a container but the container itself is not inserted into the DOM
- * (container might be part of projection or child of a view that is not inserted yet).
- *
- * In other words we can insert children of a given view if this view was inserted into a container
- * and
- * the container itself has its render parent determined.
- */
-function canInsertNativeChildOfView(viewTNode: TViewNode, view: LView): boolean {
-  return getContainerRenderParent(viewTNode, view) != null;
-}
-
-/**
- * Returns whether a native element can be inserted into the given parent.
  *
  * There are two reasons why we may not be able to insert a element immediately.
  * - Projection: When creating a child content element of a component, we have to skip the
@@ -539,19 +479,15 @@ function canInsertNativeChildOfView(viewTNode: TViewNode, view: LView): boolean
  *   parent container, which itself is disconnected. For example the parent container is part
  *   of a View which has not be inserted or is mare for projection but has not been inserted
  *   into destination.
- *
-
- *
- * @param tNode The tNode of the node that we want to insert.
- * @param currentView Current LView being processed.
- * @return boolean Whether the node should be inserted now (or delayed until later).
  */
-function canInsertNativeNode(tNode: TNode, currentView: LView): boolean {
+function getRenderParent(tNode: TNode, currentView: LView): RElement|null {
-  // Nodes of the top-most view can be inserted eagerly
+  // Nodes of the top-most view can be inserted eagerly.
   if (isRootView(currentView)) {
-    return true;
+    return nativeParentNode(currentView[RENDERER], getNativeByTNode(tNode, currentView));
   }
 
+  // Skip over element and ICU containers as those are represented by a comment node and
+  // can't be used as a render parent.
   const parent = getHighestElementOrICUContainer(tNode).parent;
 
   // If the parent is null, then we are inserting across views: either into an embedded view or a
@@ -559,24 +495,44 @@ function canInsertNativeNode(tNode: TNode, currentView: LView): boolean {
   if (parent == null) {
     const hostTNode = currentView[HOST_NODE] !;
     if (hostTNode.type === TNodeType.View) {
-      // We are inserting a root element of an embedded view - this should only be possible if the
+      // We are inserting a root element of an embedded view We might delay insertion of children
-      // embedded view in question is already inserted into a container and the container itself is
+      // for a given view if it is disconnected. This might happen for 2 main reasons:
-      // inserted into the DOM.
+      // - view is not inserted into any container(view was created but not inserted yet)
-      return canInsertNativeChildOfView(hostTNode as TViewNode, currentView);
+      // - view is inserted into a container but the container itself is not inserted into the DOM
+      // (container might be part of projection or child of a view that is not inserted yet).
+      // In other words we can insert children of a given view if this view was inserted into a
+      // container and the container itself has its render parent determined.
+      return getContainerRenderParent(hostTNode as TViewNode, currentView);
     } else {
       // We are inserting a root element of the component view into the component host element and
       // it should always be eager.
-      return true;
+      return getHostNative(currentView);
     }
   } else {
     ngDevMode && assertNodeType(parent, TNodeType.Element);
     // We've got a parent which is an element in the current view. We just need to verify if the
-    // parent element is not a component (this would mean that the tNode represents a root content
+    // parent element is not a component. Component's content nodes are not inserted immediately
-    // node of a component and its insertion should be delayed due to content projection).
+    // because they will be projected, and so doing insert at this point would be wasteful.
-    return canInsertNativeChildOfElement(parent as TElementNode);
+    // Since the projection would than move it to its final destination.
+    if (!(parent.flags & TNodeFlags.isComponent)) {
+      return getNativeByTNode(parent, currentView) as RElement;
+    } else {
+      return null;
+    }
   }
 }
 
+/**
+ * Gets the native host element for a given view. Will return null if the current view does not have
+ * a host element.
+ */
+function getHostNative(currentView: LView): RElement|null {
+  const hostTNode = currentView[HOST_NODE];
+  return hostTNode && hostTNode.type === TNodeType.Element ?
+      (getNativeByTNode(hostTNode, currentView[PARENT] !) as RElement) :
+      null;
+}
+
 /**
  * Inserts a native node before another native node for a given parent using {@link Renderer3}.
  * This is a utility function that can be used when native nodes were determined - it abstracts an
@@ -623,10 +579,10 @@ export function nativeNextSibling(renderer: Renderer3, node: RNode): RNode|null
  * @param currentView The current LView
  * @returns Whether or not the child was appended
  */
-export function appendChild(childEl: RNode, childTNode: TNode, currentView: LView): boolean {
+export function appendChild(childEl: RNode, childTNode: TNode, currentView: LView): void {
-  if (canInsertNativeNode(childTNode, currentView)) {
+  const renderParent = getRenderParent(childTNode, currentView);
+  if (renderParent != null) {
     const renderer = currentView[RENDERER];
-    const renderParent = getRenderParent(childTNode, currentView) !;
     const parentTNode: TNode = childTNode.parent || currentView[HOST_NODE] !;
 
     if (parentTNode.type === TNodeType.View) {
@@ -644,9 +600,7 @@ export function appendChild(childEl: RNode, childTNode: TNode, currentView: LVie
       isProceduralRenderer(renderer) ? renderer.appendChild(renderParent, childEl) :
                                        renderParent.appendChild(childEl);
     }
-    return true;
   }
-  return false;
 }
 
 /**
@@ -681,14 +635,12 @@ export function getBeforeNodeForView(index: number, views: LView[], containerNat
  * @param currentView The current LView
  * @returns Whether or not the child was removed
  */
-export function removeChild(childTNode: TNode, childEl: RNode, currentView: LView): boolean {
+export function removeChild(childTNode: TNode, childEl: RNode, currentView: LView): void {
-  // We only remove the element if not in View or not projected.
+  const parentNative = getRenderParent(childTNode, currentView);
-  if (canInsertNativeNode(childTNode, currentView)) {
+  // We only remove the element if it already has a render parent.
-    const parentNative = getRenderParent(childTNode, currentView) !;
+  if (parentNative) {
     nativeRemoveChild(currentView[RENDERER], parentNative, childEl);
-    return true;
   }
-  return false;
 }
 
 /**

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -155,15 +155,6 @@
   {
     "name": "callHooks"
   },
-  {
-    "name": "canInsertNativeChildOfElement"
-  },
-  {
-    "name": "canInsertNativeChildOfView"
-  },
-  {
-    "name": "canInsertNativeNode"
-  },
   {
     "name": "checkNoChangesMode"
   },
@@ -299,9 +290,6 @@
   {
     "name": "getParentInjectorViewOffset"
   },
-  {
-    "name": "getParentNative"
-  },
   {
     "name": "getPipeDef"
   },

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

@@ -410,15 +410,6 @@
   {
     "name": "callHooks"
   },
-  {
-    "name": "canInsertNativeChildOfElement"
-  },
-  {
-    "name": "canInsertNativeChildOfView"
-  },
-  {
-    "name": "canInsertNativeNode"
-  },
   {
     "name": "checkNoChanges"
   },
@@ -749,9 +740,6 @@
   {
     "name": "getParentInjectorViewOffset"
   },
-  {
-    "name": "getParentNative"
-  },
   {
     "name": "getParentState"
   },