fix(ivy): move next property to TNode (#23869)

PR Close #23869

packages/core/src/render3/di.ts

@@ -20,7 +20,6 @@ import {Type} from '../type';
 
 import {assertGreaterThan, assertLessThan, assertNotNull} from './assert';
 import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, createTView, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate, resolveDirective} from './instructions';
-import {LContainer} from './interfaces/container';
 import {ComponentTemplate, DirectiveDef, DirectiveDefList, PipeDefList} from './interfaces/definition';
 import {LInjector} from './interfaces/injector';
 import {LContainerNode, LElementNode, LNode, LNodeType, LViewNode, TNodeFlags} from './interfaces/node';
@@ -573,6 +572,8 @@ export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainer
     const lContainerNode: LContainerNode = createLNodeObject(
         LNodeType.Container, vcRefHost.view, vcRefHost.parent !, undefined, lContainer, null);
 
+    // TODO(kara): Separate into own TNode when moving parent/child properties
+    lContainerNode.tNode = vcRefHost.tNode;
     vcRefHost.dynamicLContainerNode = lContainerNode;
 
     addToViewTree(vcRefHost.view, lContainer);

packages/core/src/render3/instructions.ts

@@ -17,7 +17,7 @@ import {CurrentMatchesList, LView, LViewFlags, LifecycleStage, RootContext, TDat
 
 import {LContainerNode, LElementNode, LNode, LNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue,} from './interfaces/node';
 import {assertNodeType} from './node_assert';
-import {appendChild, insertView, appendProjectedNode, removeView, canInsertNativeNode, createTextNode} from './node_manipulation';
+import {appendChild, insertView, appendProjectedNode, removeView, canInsertNativeNode, createTextNode, getNextLNode} from './node_manipulation';
 import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher';
 import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, DirectiveDefListOrFactory, PipeDefList, PipeDefListOrFactory, RenderFlags} from './interfaces/definition';
 import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer';
@@ -346,7 +346,6 @@ export function createLNodeObject(
     view: currentView,
     parent: parent as any,
     child: null,
-    next: null,
     nodeInjector: parent ? parent.nodeInjector : null,
     data: state,
     queries: queries,
@@ -359,22 +358,31 @@ export function createLNodeObject(
 /**
  * A common way of creating the LNode to make sure that all of them have same shape to
  * keep the execution code monomorphic and fast.
+ *
+ * @param index The index at which the LNode should be saved (null if view, since they are not
+ * saved)
+ * @param type The type of LNode to create
+ * @param native The native element for this LNode, if applicable
+ * @param name The tag name of the associated native element, if applicable
+ * @param attrs Any attrs for the native element, if applicable
+ * @param data Any data that should be saved on the LNode
  */
 export function createLNode(
     index: number | null, type: LNodeType.Element, native: RElement | RText | null,
-    lView?: LView | null): LElementNode;
+    name: string | null, attrs: string[] | null, lView?: LView | null): LElementNode;
 export function createLNode(
-    index: null, type: LNodeType.View, native: null, lView: LView): LViewNode;
+    index: null, type: LNodeType.View, native: null, name: null, attrs: null,
+    lView: LView): LViewNode;
 export function createLNode(
-    index: number, type: LNodeType.Container, native: undefined,
+    index: number, type: LNodeType.Container, native: undefined, name: string | null,
-    lContainer: LContainer): LContainerNode;
+    attrs: string[] | null, lContainer: LContainer): LContainerNode;
 export function createLNode(
-    index: number, type: LNodeType.Projection, native: null,
+    index: number, type: LNodeType.Projection, native: null, name: null, attrs: string[] | null,
     lProjection: LProjection): LProjectionNode;
 export function createLNode(
     index: number | null, type: LNodeType, native: RText | RElement | null | undefined,
-    state?: null | LView | LContainer | LProjection): LElementNode&LTextNode&LViewNode&
+    name: string | null, attrs: string[] | null, state?: null | LView | LContainer |
-    LContainerNode&LProjectionNode {
+        LProjection): LElementNode&LTextNode&LViewNode&LContainerNode&LProjectionNode {
   const parent = isParent ? previousOrParentNode :
                             previousOrParentNode && previousOrParentNode.parent as LNode;
   let queries =
@@ -397,10 +405,12 @@ export function createLNode(
 
     // Every node adds a value to the static data array to avoid a sparse array
     if (index >= tData.length) {
-      tData[index] = null;
+      const tNode = tData[index] = createTNode(index, name, attrs, null);
-    } else {
+      if (!isParent && previousOrParentNode) {
-      node.tNode = tData[index] as TNode;
+        previousOrParentNode.tNode !.next = tNode;
+      }
     }
+    node.tNode = tData[index] as TNode;
 
     // Now link ourselves into the tree.
     if (isParent) {
@@ -415,14 +425,6 @@ export function createLNode(
       } else {
         // We are adding component view, so we don't link parent node child to this node.
       }
-    } else if (previousOrParentNode) {
-      ngDevMode && assertNull(
-                       previousOrParentNode.next,
-                       `previousOrParentNode's next property should not have been set ${index}.`);
-      previousOrParentNode.next = node;
-      if (previousOrParentNode.dynamicLContainerNode) {
-        previousOrParentNode.dynamicLContainerNode.next = node;
-      }
     }
   }
   previousOrParentNode = node;
@@ -463,7 +465,7 @@ export function renderTemplate<T>(
     rendererFactory = providedRendererFactory;
     const tView = getOrCreateTView(template, directives || null, pipes || null);
     host = createLNode(
-        null, LNodeType.Element, hostNode,
+        null, LNodeType.Element, hostNode, null, null,
         createLView(
             -1, providedRendererFactory.createRenderer(null, null), tView, null, {},
             LViewFlags.CheckAlways, sanitizer));
@@ -500,7 +502,7 @@ export function renderEmbeddedTemplate<T>(
       const lView = createLView(
           -1, renderer, tView, template, context, LViewFlags.CheckAlways, getCurrentSanitizer());
 
-      viewNode = createLNode(null, LNodeType.View, null, lView);
+      viewNode = createLNode(null, LNodeType.View, null, null, null, lView);
       rf = RenderFlags.Create;
     }
     oldView = enterView(viewNode.data, viewNode);
@@ -585,7 +587,10 @@ export function elementStart(
 
   ngDevMode && ngDevMode.rendererCreateElement++;
   const native: RElement = renderer.createElement(name);
-  const node: LElementNode = createLNode(index, LNodeType.Element, native !, null);
+  ngDevMode && assertDataInRange(index - 1);
+
+  const node: LElementNode =
+      createLNode(index, LNodeType.Element, native !, name, attrs || null, null);
 
   if (attrs) setUpAttributes(native, attrs);
   appendChild(node.parent !, native, currentView);
@@ -608,9 +613,7 @@ function createDirectivesAndLocals(
   const node = previousOrParentNode;
   if (firstTemplatePass) {
     ngDevMode && ngDevMode.firstTemplatePass++;
-    ngDevMode && assertDataInRange(index - 1);
+    cacheMatchingDirectivesForNode(node.tNode !, currentView.tView, localRefs || null);
-    node.tNode = tData[index] = createTNode(name, attrs || null, inlineViews ? [] : null);
-    cacheMatchingDirectivesForNode(node.tNode, currentView.tView, localRefs || null);
   } else {
     instantiateDirectivesDirectly();
   }
@@ -870,14 +873,13 @@ export function hostElement(
     sanitizer?: Sanitizer | null): LElementNode {
   resetApplicationState();
   const node = createLNode(
-      0, LNodeType.Element, rNode,
+      0, LNodeType.Element, rNode, null, null,
       createLView(
           -1, renderer, getOrCreateTView(def.template, def.directiveDefs, def.pipeDefs), null, null,
           def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, sanitizer));
 
   if (firstTemplatePass) {
-    node.tNode = createTNode(tag as string, null, null);
+    node.tNode !.flags = TNodeFlags.isComponent;
-    node.tNode.flags = TNodeFlags.isComponent;
     if (def.diPublic) def.diPublic(def);
     currentView.tView.directives = [def];
   }
@@ -1028,16 +1030,17 @@ export function elementProperty<T>(
 /**
  * Constructs a TNode object from the arguments.
  *
+ * @param index The index of the TNode in TView.data
  * @param tagName The tag name of the node
- * @param attrs The attributes defined on this ndoe
+ * @param attrs The attributes defined on this node
  * @param tViews Any TViews attached to this node
- * @param localNames A list of local names and their matching indices
  * @returns the TNode object
  */
 function createTNode(
-    tagName: string | null, attrs: string[] | null, tViews: TView[] | null): TNode {
+    index: number, tagName: string | null, attrs: string[] | null, tViews: TView[] | null): TNode {
   ngDevMode && ngDevMode.tNode++;
   return {
+    index: index,
     flags: 0,
     tagName: tagName,
     attrs: attrs,
@@ -1045,7 +1048,8 @@ function createTNode(
     initialInputs: undefined,
     inputs: undefined,
     outputs: undefined,
-    tViews: tViews
+    tViews: tViews,
+    next: null
   };
 }
 
@@ -1240,7 +1244,8 @@ export function text(index: number, value?: any): void {
           currentView.bindingStartIndex, -1, 'text nodes should be created before bindings');
   ngDevMode && ngDevMode.rendererCreateTextNode++;
   const textNode = createTextNode(value, renderer);
-  const node = createLNode(index, LNodeType.Element, textNode);
+  const node = createLNode(index, LNodeType.Element, textNode, null, null);
+
   // Text nodes are self closing.
   isParent = false;
   appendChild(node.parent !, textNode, currentView);
@@ -1473,7 +1478,10 @@ export function container(
   const currentParent = isParent ? previousOrParentNode : previousOrParentNode.parent !;
   const lContainer = createLContainer(currentParent, currentView, template);
 
-  const node = createLNode(index, LNodeType.Container, undefined, lContainer);
+  const node = createLNode(
+      index, LNodeType.Container, undefined, tagName || null, attrs || null, lContainer);
+
+  if (firstTemplatePass && template == null) node.tNode !.tViews = [];
 
   // Containers are added to the current view tree instead of their embedded views
   // because views can be removed and re-inserted.
@@ -1610,7 +1618,7 @@ export function embeddedViewStart(viewBlockId: number): RenderFlags {
       newView.queries = lContainer.queries.enterView(lContainer.nextIndex);
     }
 
-    enterView(newView, viewNode = createLNode(null, LNodeType.View, null, newView));
+    enterView(newView, viewNode = createLNode(null, LNodeType.View, null, null, null, newView));
   }
   return getRenderFlags(viewNode.data);
 }
@@ -1673,7 +1681,7 @@ export function embeddedViewEnd(): void {
 function setRenderParentInProjectedNodes(
     renderParent: LElementNode | null, viewNode: LViewNode): void {
   if (renderParent != null) {
-    let node = viewNode.child;
+    let node: LNode|null = viewNode.child;
     while (node) {
       if (node.type === LNodeType.Projection) {
         let nodeToProject: LNode|null = (node as LProjectionNode).data.head;
@@ -1685,7 +1693,7 @@ function setRenderParentInProjectedNodes(
           nodeToProject = nodeToProject === lastNodeToProject ? null : nodeToProject.pNextOrParent;
         }
       }
-      node = node.next;
+      node = getNextLNode(node);
     }
   }
 }
@@ -1749,8 +1757,8 @@ export function projectionDef(
     distributedNodes[i] = [];
   }
 
-  const componentNode = findComponentHost(currentView);
+  const componentNode: LElementNode = findComponentHost(currentView);
-  let componentChild = componentNode.child;
+  let componentChild: LNode|null = componentNode.child;
 
   while (componentChild !== null) {
     // execute selector matching logic if and only if:
@@ -1763,7 +1771,7 @@ export function projectionDef(
       distributedNodes[0].push(componentChild);
     }
 
-    componentChild = componentChild.next;
+    componentChild = getNextLNode(componentChild);
   }
 
   ngDevMode && assertDataNext(index);
@@ -1810,11 +1818,8 @@ function appendToProjectionNode(
  */
 export function projection(
     nodeIndex: number, localIndex: number, selectorIndex: number = 0, attrs?: string[]): void {
-  const node = createLNode(nodeIndex, LNodeType.Projection, null, {head: null, tail: null});
+  const node = createLNode(
-
+      nodeIndex, LNodeType.Projection, null, null, attrs || null, {head: null, tail: null});
-  if (node.tNode == null) {
-    node.tNode = createTNode(null, attrs || null, null);
-  }
 
   // `<ng-content>` has no content
   isParent = false;

packages/core/src/render3/interfaces/node.ts

@@ -79,12 +79,6 @@ export interface LNode {
    */
   child: LNode|null;
 
-  /**
-   * The next sibling node. Necessary so we can propagate through the root nodes of a view
-   * to insert them or remove them from the DOM.
-   */
-  next: LNode|null;
-
   /**
    * If regular LElementNode, then `data` will be null.
    * If LElementNode with component, then `data` contains LView.
@@ -139,7 +133,6 @@ export interface LElementNode extends LNode {
   readonly native: RElement;
 
   child: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
-  next: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
 
   /** If Component then data has LView (light DOM) */
   readonly data: LView|null;
@@ -153,7 +146,6 @@ export interface LTextNode extends LNode {
   /** The text node associated with this node. */
   native: RText;
   child: null;
-  next: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
 
   /** LTextNodes can be inside LElementNodes or inside LViewNodes. */
   readonly parent: LElementNode|LViewNode;
@@ -165,7 +157,6 @@ export interface LTextNode extends LNode {
 export interface LViewNode extends LNode {
   readonly native: null;
   child: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
-  next: LViewNode|null;
 
   /**  LViewNodes can only be added to LContainerNodes. */
   readonly parent: LContainerNode|null;
@@ -185,7 +176,6 @@ export interface LContainerNode extends LNode {
   native: RElement|RText|null|undefined;
   readonly data: LContainer;
   child: null;
-  next: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
 
   /** Containers can be added to elements or views. */
   readonly parent: LElementNode|LViewNode|null;
@@ -195,7 +185,6 @@ export interface LContainerNode extends LNode {
 export interface LProjectionNode extends LNode {
   readonly native: null;
   child: null;
-  next: LContainerNode|LElementNode|LTextNode|LProjectionNode|null;
 
   readonly data: LProjection;
 
@@ -216,6 +205,14 @@ export interface LProjectionNode extends LNode {
  * see: https://en.wikipedia.org/wiki/Flyweight_pattern for more on the Flyweight pattern
  */
 export interface TNode {
+  /**
+   * Index of the TNode in TView.data and corresponding LNode in LView.data.
+   *
+   * This is necessary to get from any TNode to its corresponding LNode when
+   * traversing the node tree.
+   */
+  index: number;
+
   /**
    * This number stores two values using its bits:
    *
@@ -303,6 +300,12 @@ export interface TNode {
    * If this TNode corresponds to an LElementNode, tViews will be null .
    */
   tViews: TView|TView[]|null;
+
+  /**
+   * The next sibling node. Necessary so we can propagate through the root nodes of a view
+   * to insert them or remove them from the DOM.
+   */
+  next: TNode|null;
 }
 
 /** Static data for an LElementNode  */

packages/core/src/render3/node_manipulation.ts

@@ -44,13 +44,13 @@ function findNextRNodeSibling(node: LNode | null, stopNode: LNode | null): RElem
       }
       currentNode = pNextOrParent;
     } else {
-      let currentSibling = currentNode.next;
+      let currentSibling = getNextLNode(currentNode);
       while (currentSibling) {
         const nativeNode = findFirstRNode(currentSibling);
         if (nativeNode) {
           return nativeNode;
         }
-        currentSibling = currentSibling.next;
+        currentSibling = getNextLNode(currentSibling);
       }
       const parentNode = currentNode.parent;
       currentNode = null;
@@ -65,6 +65,16 @@ function findNextRNodeSibling(node: LNode | null, stopNode: LNode | null): RElem
   return null;
 }
 
+/** Retrieves the sibling node for the given node. */
+export function getNextLNode(node: LNode): LNode|null {
+  // View nodes don't have TNodes, so their next must be retrieved through their LView.
+  if (node.type === LNodeType.View) {
+    const lView = node.data as LView;
+    return lView.next ? (lView.next as LView).node : null;
+  }
+  return node.tNode !.next ? node.view.data[node.tNode !.next !.index] : null;
+}
+
 /**
  * Get the next node in the LNode tree, taking into account the place where a node is
  * projected (in the shadow DOM) rather than where it comes from (in the light DOM).
@@ -83,7 +93,7 @@ function getNextLNodeWithProjection(node: LNode): LNode|null {
   }
 
   // returns node.next because the the node is not projected
-  return node.next;
+  return getNextLNode(node);
 }
 
 /**
@@ -187,7 +197,7 @@ export function addRemoveViewFromContainer(
           isProceduralRenderer(renderer) ? renderer.removeChild(parent as RElement, node.native !) :
                                            parent.removeChild(node.native !);
         }
-        nextNode = node.next;
+        nextNode = getNextLNode(node);
       } else if (node.type === LNodeType.Container) {
         // if we get to a container, it must be a root node of a view because we are only
         // propagating down into child views / containers and not child elements
@@ -267,32 +277,34 @@ export function destroyViewTree(rootView: LView): void {
  * the container's parent view is added later).
  *
  * @param container The container into which the view should be inserted
- * @param newView The view to insert
+ * @param viewNode The view to insert
  * @param index The index at which to insert the view
  * @returns The inserted view
  */
 export function insertView(
-    container: LContainerNode, newView: LViewNode, index: number): LViewNode {
+    container: LContainerNode, viewNode: LViewNode, index: number): LViewNode {
   const state = container.data;
   const views = state.views;
 
   if (index > 0) {
     // This is a new view, we need to add it to the children.
-    setViewNext(views[index - 1], newView);
+    views[index - 1].data.next = viewNode.data as LView;
   }
 
   if (index < views.length) {
-    setViewNext(newView, views[index]);
+    viewNode.data.next = views[index].data;
-    views.splice(index, 0, newView);
+    views.splice(index, 0, viewNode);
   } else {
-    views.push(newView);
+    views.push(viewNode);
+    viewNode.data.next = null;
   }
 
   // If the container's renderParent is null, we know that it is a root node of its own parent view
   // and we should wait until that parent processes its nodes (otherwise, we will insert this view's
   // nodes twice - once now and once when its parent inserts its views).
   if (container.data.renderParent !== null) {
-    let beforeNode = findNextRNodeSibling(newView, container);
+    let beforeNode = findNextRNodeSibling(viewNode, container);
+
     if (!beforeNode) {
       let containerNextNativeNode = container.native;
       if (containerNextNativeNode === undefined) {
@@ -300,10 +312,10 @@ export function insertView(
       }
       beforeNode = containerNextNativeNode;
     }
-    addRemoveViewFromContainer(container, newView, true, beforeNode);
+    addRemoveViewFromContainer(container, viewNode, true, beforeNode);
   }
 
-  return newView;
+  return viewNode;
 }
 
 /**
@@ -321,10 +333,9 @@ export function removeView(container: LContainerNode, removeIndex: number): LVie
   const views = container.data.views;
   const viewNode = views[removeIndex];
   if (removeIndex > 0) {
-    setViewNext(views[removeIndex - 1], viewNode.next);
+    views[removeIndex - 1].data.next = viewNode.data.next as LView;
   }
   views.splice(removeIndex, 1);
-  viewNode.next = null;
   destroyViewTree(viewNode.data);
   addRemoveViewFromContainer(container, viewNode, false);
   // Notify query that view has been removed
@@ -332,19 +343,6 @@ export function removeView(container: LContainerNode, removeIndex: number): LVie
   return viewNode;
 }
 
-/**
- * Sets a next on the view node, so views in for loops can easily jump from
- * one view to the next to add/remove elements. Also adds the LView (view.data)
- * to the view tree for easy traversal when cleaning up the view.
- *
- * @param view The view to set up
- * @param next The view's new next
- */
-export function setViewNext(view: LViewNode, next: LViewNode | null): void {
-  view.next = next;
-  view.data.next = next ? next.data : null;
-}
-
 /**
  * Determines which LViewOrLContainer to jump to when traversing back up the
  * tree in destroyViewTree.

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

@@ -389,6 +389,9 @@
   {
     "name": "getDirectiveInstance"
   },
+  {
+    "name": "getNextLNode"
+  },
   {
     "name": "getNextLNodeWithProjection"
   },
@@ -617,9 +620,6 @@
   {
     "name": "setUpAttributes"
   },
-  {
-    "name": "setViewNext"
-  },
   {
     "name": "stringify"
   },

packages/core/test/render3/di_spec.ts

@@ -1367,7 +1367,7 @@ describe('di', () => {
           createLView(-1, null !, createTView(null, null), null, null, LViewFlags.CheckAlways);
       const oldView = enterView(contentView, null !);
       try {
-        const parent = createLNode(0, LNodeType.Element, null, null);
+        const parent = createLNode(0, LNodeType.Element, null, null, null, null);
 
         // Simulate the situation where the previous parent is not initialized.
         // This happens on first bootstrap because we don't init existing values

packages/core/test/render3/integration_spec.ts

@@ -32,7 +32,7 @@ describe('render3 integration test', () => {
       }
       expect(ngDevMode).toHaveProperties({
         firstTemplatePass: 1,
-        tNode: 1,
+        tNode: 2,
         tView: 1,
         rendererCreateElement: 1,
       });

packages/core/test/render3/node_selector_matcher_spec.ts

@@ -12,14 +12,14 @@ import {getProjectAsAttrValue, isNodeMatchingSelectorList, isNodeMatchingSelecto
 
 function testLStaticData(tagName: string, attrs: string[] | null): TNode {
   return {
-    flags: 0,
+    index: 0,
-    tagName,
+    flags: 0, tagName, attrs,
-    attrs,
     localNames: null,
     initialInputs: undefined,
     inputs: undefined,
     outputs: undefined,
     tViews: null,
+    next: null
   };
 }
 

packages/core/test/render3/view_container_ref_spec.ts

@@ -271,7 +271,7 @@ describe('ViewContainerRef', () => {
             * <ng-template directive>A<ng-template>
             * % if (condition) {
             *  B
-            * }
+            * % }
             * |after
             */
            class TestComponent {