refactor(core): Remove host `TNode` from `getOrCreateTNode` (#38707)

Host `TNode` was passed into `getOrCreateTNode` just so that we can
compute weather or not we are a root node. This was needed because
`previousOrParentTNode` could have `TNode` from `TView` other then
current `TView`. This is confusing mental model. Previous change
ensured that `previousOrParentTNode` must always be part of `TView`,
which enabled this change to remove the unneeded argument.

PR Close #38707

packages/core/src/render3/component.ts

@@ -174,7 +174,7 @@ export function createRootComponentView(
   const tView = rootView[TVIEW];
   ngDevMode && assertIndexInRange(rootView, 0 + HEADER_OFFSET);
   rootView[0 + HEADER_OFFSET] = rNode;
-  const tNode: TElementNode = getOrCreateTNode(tView, null, 0, TNodeType.Element, null, null);
+  const tNode: TElementNode = getOrCreateTNode(tView, 0, TNodeType.Element, null, null);
   const mergedAttrs = tNode.mergedAttrs = def.hostAttrs;
   if (mergedAttrs !== null) {
     computeStaticStyling(tNode, mergedAttrs, true);

packages/core/src/render3/di.ts

@@ -141,41 +141,41 @@ export function bloomAdd(
  * Creates (or gets an existing) injector for a given element or container.
  *
  * @param tNode for which an injector should be retrieved / created.
- * @param hostView View where the node is stored
+ * @param lView View where the node is stored
  * @returns Node injector
  */
 export function getOrCreateNodeInjectorForNode(
-    tNode: TElementNode|TContainerNode|TElementContainerNode, hostView: LView): number {
+    tNode: TElementNode|TContainerNode|TElementContainerNode, lView: LView): number {
-  const existingInjectorIndex = getInjectorIndex(tNode, hostView);
+  const existingInjectorIndex = getInjectorIndex(tNode, lView);
   if (existingInjectorIndex !== -1) {
     return existingInjectorIndex;
   }
 
-  const tView = hostView[TVIEW];
+  const tView = lView[TVIEW];
   if (tView.firstCreatePass) {
-    tNode.injectorIndex = hostView.length;
+    tNode.injectorIndex = lView.length;
     insertBloom(tView.data, tNode);  // foundation for node bloom
-    insertBloom(hostView, null);     // foundation for cumulative bloom
+    insertBloom(lView, null);        // foundation for cumulative bloom
     insertBloom(tView.blueprint, null);
   }
 
-  const parentLoc = getParentInjectorLocation(tNode, hostView);
+  const parentLoc = getParentInjectorLocation(tNode, lView);
   const injectorIndex = tNode.injectorIndex;
 
   // If a parent injector can't be found, its location is set to -1.
   // In that case, we don't need to set up a cumulative bloom
   if (hasParentInjector(parentLoc)) {
     const parentIndex = getParentInjectorIndex(parentLoc);
-    const parentLView = getParentInjectorView(parentLoc, hostView);
+    const parentLView = getParentInjectorView(parentLoc, lView);
     const parentData = parentLView[TVIEW].data as any;
     // Creates a cumulative bloom filter that merges the parent's bloom filter
     // and its own cumulative bloom (which contains tokens for all ancestors)
     for (let i = 0; i < 8; i++) {
-      hostView[injectorIndex + i] = parentLView[parentIndex + i] | parentData[parentIndex + i];
+      lView[injectorIndex + i] = parentLView[parentIndex + i] | parentData[parentIndex + i];
     }
   }
 
-  hostView[injectorIndex + PARENT_INJECTOR] = parentLoc;
+  lView[injectorIndex + PARENT_INJECTOR] = parentLoc;
   return injectorIndex;
 }
 
@@ -202,7 +202,7 @@ export function getInjectorIndex(tNode: TNode, hostView: LView): number {
  * Finds the index of the parent injector, with a view offset if applicable. Used to set the
  * parent injector initially.
  *
- * Returns a combination of number of `ViewData` we have to go up and index in that `Viewdata`
+ * Returns a combination of number of `LView` we have to go up and index in that `LView`
  */
 export function getParentInjectorLocation(tNode: TNode, view: LView): RelativeInjectorLocation {
   if (tNode.parent && tNode.parent.injectorIndex !== -1) {

packages/core/src/render3/i18n/i18n_apply.ts

@@ -463,7 +463,7 @@ function createDynamicNodeAtIndex(
   ngDevMode && assertIndexInRange(lView, index + HEADER_OFFSET);
   lView[index + HEADER_OFFSET] = native;
   // FIXME(misko): Why does this create A TNode??? I would not expect this to be here.
-  const tNode = getOrCreateTNode(tView, lView[T_HOST], index, type as any, name, null);
+  const tNode = getOrCreateTNode(tView, index, type as any, name, null);
 
   // We are creating a dynamic node, the previous tNode might not be pointing at this node.
   // We will link ourselves into the tree later with `appendI18nNode`.

packages/core/src/render3/instructions/element.ts

@@ -33,7 +33,7 @@ function elementStartFirstCreatePass(
 
   const tViewConsts = tView.consts;
   const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
-  const tNode = getOrCreateTNode(tView, lView[T_HOST], index, TNodeType.Element, name, attrs);
+  const tNode = getOrCreateTNode(tView, index, TNodeType.Element, name, attrs);
 
   const hasDirectives =
       resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));

packages/core/src/render3/instructions/element_container.ts

@@ -27,8 +27,7 @@ function elementContainerStartFirstCreatePass(
 
   const tViewConsts = tView.consts;
   const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
-  const tNode = getOrCreateTNode(
+  const tNode = getOrCreateTNode(tView, index, TNodeType.ElementContainer, 'ng-container', attrs);
-      tView, lView[T_HOST], index, TNodeType.ElementContainer, 'ng-container', attrs);
 
   // While ng-container doesn't necessarily support styling, we use the style context to identify
   // and execute directives on the ng-container.

packages/core/src/render3/instructions/projection.ts

@@ -125,7 +125,7 @@ export function ɵɵprojection(
   const lView = getLView();
   const tView = getTView();
   const tProjectionNode =
-      getOrCreateTNode(tView, lView[T_HOST], nodeIndex, TNodeType.Projection, null, attrs || null);
+      getOrCreateTNode(tView, nodeIndex, TNodeType.Projection, null, attrs || null);
 
   // We can't use viewData[HOST_NODE] because projection nodes can be nested in embedded views.
   if (tProjectionNode.projection === null) tProjectionNode.projection = selectorIndex;

packages/core/src/render3/instructions/shared.ts

@@ -202,9 +202,6 @@ export function createLView<T>(
  * Create and stores the TNode, and hooks it up to the tree.
  *
  * @param tView The current `TView`.
- * @param tHostNode This is a hack and we should not have to pass this value in. It is only used to
- * determine if the parent belongs to a different tView. Instead we should not have parentTView
- * point to TView other the current one.
  * @param index The index at which the TNode should be saved (null if view, since they are not
  * saved).
  * @param type The type of TNode to create
@@ -213,35 +210,34 @@ export function createLView<T>(
  * @param attrs Any attrs for the native element, if applicable
  */
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Element, name: string|null,
+    tView: TView, index: number, type: TNodeType.Element, name: string|null,
     attrs: TAttributes|null): TElementNode;
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Container,
+    tView: TView, index: number, type: TNodeType.Container, name: string|null,
-    name: string|null, attrs: TAttributes|null): TContainerNode;
+    attrs: TAttributes|null): TContainerNode;
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Projection, name: null,
+    tView: TView, index: number, type: TNodeType.Projection, name: null,
     attrs: TAttributes|null): TProjectionNode;
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.ElementContainer,
+    tView: TView, index: number, type: TNodeType.ElementContainer, name: string|null,
-    name: string|null, attrs: TAttributes|null): TElementContainerNode;
-export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.IcuContainer, name: null,
     attrs: TAttributes|null): TElementContainerNode;
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType, name: string|null,
+    tView: TView, index: number, type: TNodeType.IcuContainer, name: null,
-    attrs: TAttributes|null): TElementNode&TContainerNode&TElementContainerNode&TProjectionNode&
+    attrs: TAttributes|null): TElementContainerNode;
-    TIcuContainerNode {
+export function getOrCreateTNode(
+    tView: TView, index: number, type: TNodeType, name: string|null, attrs: TAttributes|null):
+    TElementNode&TContainerNode&TElementContainerNode&TProjectionNode&TIcuContainerNode {
   // Keep this function short, so that the VM will inline it.
   const adjustedIndex = index + HEADER_OFFSET;
   const tNode = tView.data[adjustedIndex] as TNode ||
-      createTNodeAtIndex(tView, tHostNode, adjustedIndex, type, name, attrs);
+      createTNodeAtIndex(tView, adjustedIndex, type, name, attrs);
   setPreviousOrParentTNode(tNode, true);
   return tNode as TElementNode & TViewNode & TContainerNode & TElementContainerNode &
       TProjectionNode & TIcuContainerNode;
 }
 
 function createTNodeAtIndex(
-    tView: TView, tHostNode: TNode|null, adjustedIndex: number, type: TNodeType, name: string|null,
+    tView: TView, adjustedIndex: number, type: TNodeType, name: string|null,
     attrs: TAttributes|null) {
   const previousOrParentTNode = getPreviousOrParentTNode();
   const isParent = getIsParent();
@@ -249,7 +245,9 @@ function createTNodeAtIndex(
       isParent ? previousOrParentTNode : previousOrParentTNode && previousOrParentTNode.parent;
   // Parents cannot cross component boundaries because components will be used in multiple places,
   // so it's only set if the view is the same.
-  const parentInSameView = parent && parent !== tHostNode;
+  // FIXME(misko): This check for `TNodeType.View` should not be needed. But removing it breaks DI,
+  // so more investigation is needed.
+  const parentInSameView = parent !== null && parent.type !== TNodeType.View;
   const tParentNode = parentInSameView ? parent as TElementNode | TContainerNode : null;
   const tNode = tView.data[adjustedIndex] =
       createTNode(tView, tParentNode, type, adjustedIndex, name, attrs);

packages/core/src/render3/instructions/template.ts

@@ -28,7 +28,7 @@ function templateFirstCreatePass(
   const tViewConsts = tView.consts;
   // TODO(pk): refactor getOrCreateTNode to have the "create" only version
   const tNode = getOrCreateTNode(
-      tView, lView[T_HOST], index, TNodeType.Container, tagName || null,
+      tView, index, TNodeType.Container, tagName || null,
       getConstant<TAttributes>(tViewConsts, attrsIndex));
 
   resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));

packages/core/src/render3/instructions/text.ts

@@ -35,7 +35,7 @@ export function ɵɵtext(index: number, value: string = ''): void {
   ngDevMode && assertIndexInRange(lView, adjustedIndex);
 
   const tNode = tView.firstCreatePass ?
-      getOrCreateTNode(tView, lView[T_HOST], index, TNodeType.Element, null, null) :
+      getOrCreateTNode(tView, index, TNodeType.Element, null, null) :
       tView.data[adjustedIndex] as TElementNode;
 
   const textNative = lView[adjustedIndex] = createTextNode(value, lView[RENDERER]);

packages/core/src/render3/node_manipulation.ts

@@ -520,7 +520,7 @@ function getRenderParent(tView: TView, tNode: TNode, currentView: LView): REleme
   // can't be used as a render parent.
   let parentTNode = tNode.parent;
   while (parentTNode != null &&
-         (parentTNode.type === TNodeType.ElementContainer ||
+         (parentTNode.type === TNodeType.ElementContainer || parentTNode.type === TNodeType.View ||
           parentTNode.type === TNodeType.IcuContainer)) {
     tNode = parentTNode;
     parentTNode = tNode.parent;

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

@@ -1355,6 +1355,9 @@
   {
     "name": "getNodeInjectable"
   },
+  {
+    "name": "getNonViewFirstChild"
+  },
   {
     "name": "getNullInjector"
   },

packages/core/test/render3/di_spec.ts

@@ -230,8 +230,7 @@ describe('di', () => {
           LViewFlags.CheckAlways, null, null, {} as any, {} as any);
       enterView(contentView);
       try {
-        const parentTNode =
+        const parentTNode = getOrCreateTNode(contentView[TVIEW], 0, TNodeType.Element, null, null);
-            getOrCreateTNode(contentView[TVIEW], null, 0, TNodeType.Element, null, null);
         // Simulate the situation where the previous parent is not initialized.
         // This happens on first bootstrap because we don't init existing values
         // so that we have smaller HelloWorld.

packages/core/test/render3/perf/setup.ts

@@ -50,7 +50,7 @@ export function setupTestHarness(
     directiveRegistry: DirectiveDefList|null = null): TestHarness {
   // Create a root view with a container
   const hostTView = createTView(TViewType.Root, -1, null, 1, 0, null, null, null, null, consts);
-  const tContainerNode = getOrCreateTNode(hostTView, null, 0, TNodeType.Container, null, null);
+  const tContainerNode = getOrCreateTNode(hostTView, 0, TNodeType.Container, null, null);
   const hostNode = renderer.createElement('div');
   const hostLView = createLView(
       null, hostTView, {}, LViewFlags.CheckAlways | LViewFlags.IsRoot, hostNode, null,

packages/core/test/render3/render_util.ts

@@ -278,7 +278,7 @@ export function renderTemplate<T>(
     def.pipeDefs = pipes || null;
 
     const componentTView = getOrCreateTComponentView(def);
-    const hostTNode = getOrCreateTNode(tView, hostLView[T_HOST], 0, TNodeType.Element, null, null);
+    const hostTNode = getOrCreateTNode(tView, 0, TNodeType.Element, null, null);
     hostLView[hostTNode.index] = hostNode;
     componentView = createLView(
         hostLView, componentTView, context, LViewFlags.CheckAlways, hostNode, hostTNode,