refactor(ivy): remove dynamicParent from LNode (#24678)

PR Close #24678

packages/core/src/render3/di.ts

@@ -23,7 +23,7 @@ import {addToViewTree, assertPreviousIsParent, createEmbeddedViewNode, createLCo
 import {VIEWS} from './interfaces/container';
 import {ComponentTemplate, DirectiveDefInternal, RenderFlags} from './interfaces/definition';
 import {LInjector} from './interfaces/injector';
-import {AttributeMarker, LContainerNode, LElementNode, LNode, LViewNode, TNodeFlags, TNodeType} from './interfaces/node';
+import {AttributeMarker, LContainerNode, LElementNode, LNode, LViewNode, TContainerNode, TElementNode, TNodeFlags, TNodeType} from './interfaces/node';
 import {LQueries, QueryReadType} from './interfaces/query';
 import {Renderer3} from './interfaces/renderer';
 import {DIRECTIVES, HOST_NODE, INJECTOR, LViewData, QUERIES, RENDERER, TVIEW, TView} from './interfaces/view';
@@ -583,14 +583,14 @@ export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainer
       lContainerNode.queries = vcRefHost.queries.container();
     }
 
-    const hostTNode = vcRefHost.tNode;
+    const hostTNode = vcRefHost.tNode as TElementNode | TContainerNode;
     if (!hostTNode.dynamicContainerNode) {
-      hostTNode.dynamicContainerNode = createTNode(TNodeType.Container, -1, null, null, null, null);
+      hostTNode.dynamicContainerNode =
+          createTNode(TNodeType.Container, -1, null, null, hostTNode, null);
     }
 
     lContainerNode.tNode = hostTNode.dynamicContainerNode;
     vcRefHost.dynamicLContainerNode = lContainerNode;
-    lContainerNode.dynamicParent = vcRefHost;
 
     addToViewTree(vcRefHost.view, hostTNode.index as number, lContainer);
 
@@ -653,7 +653,6 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
     const lViewNode = (viewRef as EmbeddedViewRef<any>)._lViewNode;
     const adjustedIdx = this._adjustIndex(index);
 
-    lViewNode.dynamicParent = this._lContainerNode;
     insertView(this._lContainerNode, lViewNode, adjustedIdx);
     const views = this._lContainerNode.data[VIEWS];
     const beforeNode = adjustedIdx + 1 < views.length ?
@@ -685,7 +684,6 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
   detach(index?: number): viewEngine_ViewRef|null {
     const adjustedIdx = this._adjustIndex(index, -1);
     const lViewNode = detachView(this._lContainerNode, adjustedIdx);
-    lViewNode.dynamicParent = null;
     return this._viewRefs.splice(adjustedIdx, 1)[0] || null;
   }
 
@@ -735,7 +733,6 @@ class TemplateRef<T> implements viewEngine_TemplateRef<T> {
       viewEngine_EmbeddedViewRef<T> {
     const viewNode = createEmbeddedViewNode(this._tView, context, this._renderer, this._queries);
     if (containerNode) {
-      viewNode.dynamicParent = containerNode;
       insertView(containerNode, viewNode, index !);
     }
     renderEmbeddedTemplate(viewNode, this._tView, context, RenderFlags.Create);

packages/core/src/render3/i18n.ts

@@ -301,7 +301,6 @@ export function i18nApply(startIndex: number, instructions: I18nInstruction[]):
         // But since this text doesn't have an index in `LViewData`, we need to create an
         // `LElementNode` with the index -1 so that it isn't saved in `LViewData`
         const textLNode = createLNode(-1, TNodeType.Element, textRNode, null, null);
-        textLNode.dynamicParent = localParentNode as LElementNode | LContainerNode;
         localPreviousNode = appendI18nNode(textLNode, localParentNode, localPreviousNode);
         break;
       case I18nInstructions.CloseNode:

packages/core/src/render3/instructions.ts

@@ -17,7 +17,7 @@ import {ACTIVE_INDEX, LContainer, RENDER_PARENT, VIEWS} from './interfaces/conta
 import {LInjector} from './interfaces/injector';
 import {CssSelectorList, LProjection, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection';
 import {LQueries} from './interfaces/query';
-import {BINDING_INDEX, CLEANUP, CONTEXT, CurrentMatchesList, DIRECTIVES, FLAGS, HEADER_OFFSET, HOST_NODE, INJECTOR, LViewData, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, RootContext, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view';
+import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTEXT, CurrentMatchesList, DIRECTIVES, FLAGS, HEADER_OFFSET, HOST_NODE, INJECTOR, LViewData, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, RootContext, SANITIZER, TAIL, TData, TVIEW, TView} from './interfaces/view';
 
 import {AttributeMarker, TAttributes, LContainerNode, LElementNode, LNode, TNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue, TElementNode,} from './interfaces/node';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
@@ -303,7 +303,8 @@ export function createLViewData<T>(
     viewData && viewData[INJECTOR],                                              // injector
     renderer,                                                                    // renderer
     sanitizer || null,                                                           // sanitizer
-    null                                                                         // tail
+    null,                                                                        // tail
+    -1                                                                           // containerIndex
   ];
 }
 
@@ -325,7 +326,6 @@ export function createLNodeObject(
     tNode: null !,
     pNextOrParent: null,
     dynamicLContainerNode: null,
-    dynamicParent: null,
     pChild: null,
   };
 }
@@ -1783,14 +1783,10 @@ export function embeddedViewStart(viewBlockId: number): RenderFlags {
     enterView(
         newView, viewNode = createLNode(viewBlockId, TNodeType.View, null, null, null, newView));
   }
-  const containerNode = getParentLNode(viewNode) as LContainerNode;
+  if (container) {
-  if (containerNode) {
-    ngDevMode && assertNodeType(viewNode, TNodeType.View);
-    ngDevMode && assertNodeType(containerNode, TNodeType.Container);
-    const lContainer = containerNode.data;
     if (creationMode) {
       // it is a new view, insert it into collection of views for a given container
-      insertView(containerNode, viewNode, lContainer[ACTIVE_INDEX] !);
+      insertView(container, viewNode, lContainer[ACTIVE_INDEX] !);
     }
     lContainer[ACTIVE_INDEX] !++;
   }

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

@@ -118,12 +118,6 @@ export interface LNode {
    */
   // TODO(kara): Remove when removing LNodes
   dynamicLContainerNode: LContainerNode|null;
-
-  /**
-   * A pointer to a parent LNode created dynamically and virtually by directives requesting
-   * ViewContainerRef. Applicable only to LContainerNode and LViewNode.
-   */
-  dynamicParent: LElementNode|LContainerNode|LViewNode|null;
 }
 
 
@@ -142,7 +136,6 @@ export interface LTextNode extends LNode {
   native: RText;
   readonly data: null;
   dynamicLContainerNode: null;
-  dynamicParent: null;
 }
 
 /** Abstract node which contains root nodes of a view. */

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

@@ -11,12 +11,12 @@ import {Sanitizer} from '../../sanitization/security';
 
 import {LContainer} from './container';
 import {ComponentQuery, ComponentTemplate, DirectiveDefInternal, DirectiveDefList, PipeDef, PipeDefList} from './definition';
-import {LElementNode, LViewNode, TNode} from './node';
+import {LContainerNode, LElementNode, LViewNode, TNode} from './node';
 import {LQueries} from './query';
 import {Renderer3} from './renderer';
 
 /** Size of LViewData's header. Necessary to adjust for it when setting slots.  */
-export const HEADER_OFFSET = 14;
+export const HEADER_OFFSET = 15;
 
 // Below are constants for LViewData indices to help us look up LViewData members
 // without having to remember the specific indices.
@@ -35,6 +35,7 @@ export const INJECTOR = 10;
 export const RENDERER = 11;
 export const SANITIZER = 12;
 export const TAIL = 13;
+export const CONTAINER_INDEX = 14;
 
 /**
  * `LViewData` stores all of the information needed to process the instructions as
@@ -122,7 +123,7 @@ export interface LViewData extends Array<any> {
    * - For embedded views, the context with which to render the template.
    * - For root view of the root component the context contains change detection data.
    * - `null` otherwise.
-  */
+   */
   [CONTEXT]: {}|RootContext|null;
 
   /** An optional Module Injector to be used as fall back after Element Injectors are consulted. */
@@ -142,6 +143,16 @@ export interface LViewData extends Array<any> {
    */
   // TODO: replace with global
   [TAIL]: LViewData|LContainer|null;
+
+  /**
+   * The index of the parent container's host node. Applicable only to embedded views that
+   * have been inserted dynamically. Will be -1 for component views and inline views.
+   *
+   * This is necessary to jump from dynamically created embedded views to their parent
+   * containers because their parent cannot be stored on the TViewNode (views may be inserted
+   * in multiple containers, so the parent cannot be shared between view instances).
+   */
+  [CONTAINER_INDEX]: number;
 }
 
 /** Flags associated with an LView (saved in LViewData[FLAGS]) */

packages/core/src/render3/node_manipulation.ts

@@ -11,7 +11,7 @@ import {LContainer, RENDER_PARENT, VIEWS, unusedValueExportToPlacateAjd as unuse
 import {LContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, TNodeType, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
 import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection';
 import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
-import {CLEANUP, DIRECTIVES, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LViewData, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
+import {CLEANUP, CONTAINER_INDEX, DIRECTIVES, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LViewData, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
 import {stringify} from './util';
 
@@ -45,9 +45,11 @@ export function getParentLNode(node: LContainerNode | LElementNode | LTextNode |
 export function getParentLNode(node: LViewNode): LContainerNode|null;
 export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null;
 export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null {
-  if (node.tNode.index === -1) {
+  if (node.tNode.index === -1 && node.tNode.type === TNodeType.View) {
-    // This is a dynamic container or an embedded view inside a dynamic container.
+    // This is a dynamically created view inside a dynamic container.
-    return node.dynamicParent;
+    // If the host index is -1, the view has not yet been inserted, so it has no parent.
+    const containerHostIndex = (node.data as LViewData)[CONTAINER_INDEX];
+    return containerHostIndex === -1 ? null : node.view[containerHostIndex].dynamicLContainerNode;
   }
   const parent = node.tNode.parent;
   return parent ? node.view[parent.index] : node.view[HOST_NODE];
@@ -293,28 +295,35 @@ export function insertView(
     container: LContainerNode, viewNode: LViewNode, index: number): LViewNode {
   const state = container.data;
   const views = state[VIEWS];
+  const lView = viewNode.data as LViewData;
 
   if (index > 0) {
     // This is a new view, we need to add it to the children.
-    views[index - 1].data[NEXT] = viewNode.data as LViewData;
+    views[index - 1].data[NEXT] = lView;
   }
 
   if (index < views.length) {
-    viewNode.data[NEXT] = views[index].data;
+    lView[NEXT] = views[index].data;
     views.splice(index, 0, viewNode);
   } else {
     views.push(viewNode);
-    viewNode.data[NEXT] = null;
+    lView[NEXT] = null;
+  }
+
+  // Dynamically inserted views need a reference to their parent container'S host so it's
+  // possible to jump from a view to its container's next when walking the node tree.
+  if (viewNode.tNode.index === -1) {
+    lView[CONTAINER_INDEX] = container.tNode.parent !.index;
+    (viewNode as{view: LViewData}).view = container.view;
   }
 
   // Notify query that a new view has been added
-  const lView = viewNode.data;
   if (lView[QUERIES]) {
     lView[QUERIES] !.insertView(index);
   }
 
   // Sets the attached flag
-  viewNode.data[FLAGS] |= LViewFlags.Attached;
+  lView[FLAGS] |= LViewFlags.Attached;
 
   return viewNode;
 }
@@ -340,10 +349,12 @@ export function detachView(container: LContainerNode, removeIndex: number): LVie
     addRemoveViewFromContainer(container, viewNode, false);
   }
   // Notify query that view has been removed
-  const removedLview = viewNode.data;
+  const removedLView = viewNode.data;
-  if (removedLview[QUERIES]) {
+  if (removedLView[QUERIES]) {
-    removedLview[QUERIES] !.removeView();
+    removedLView[QUERIES] !.removeView();
   }
+  removedLView[CONTAINER_INDEX] = -1;
+  (viewNode as{view: LViewData | null}).view = null;
   // Unsets the attached flag
   viewNode.data[FLAGS] &= ~LViewFlags.Attached;
   return viewNode;
@@ -476,14 +487,14 @@ function executePipeOnDestroys(viewData: LViewData): void {
 
 /**
  * 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 
+ * - Projection: When creating a child content element of a component, we have to skip the
  *   insertion because the content of a component will be projected.
  *   `<component><content>delayed due to projection</content></component>`
- * - Parent container is disconnected: This can happen when we are inserting a view into 
+ * - Parent container is disconnected: This can happen when we are inserting a view into
- *   parent container, which itself is disconnected. For example the parent container is part 
+ *   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 
+ *   of a View which has not be inserted or is mare for projection but has not been inserted
  *   into destination.
  *
 

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

@@ -8,6 +8,9 @@
   {
     "name": "CLEAN_PROMISE"
   },
+  {
+    "name": "CONTAINER_INDEX"
+  },
   {
     "name": "CONTEXT"
   },

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

@@ -17,6 +17,9 @@
   {
     "name": "CLEAN_PROMISE"
   },
+  {
+    "name": "CONTAINER_INDEX"
+  },
   {
     "name": "CONTEXT"
   },

packages/core/test/render3/view_container_ref_spec.ts

@@ -14,7 +14,7 @@ import {RenderFlags} from '../../src/render3/interfaces/definition';
 import {pipe, pipeBind1} from '../../src/render3/pipe';
 
 import {getRendererFactory2} from './imported_renderer2';
-import {ComponentFixture, TemplateFixture} from './render_util';
+import {ComponentFixture, TemplateFixture, createComponent} from './render_util';
 
 describe('ViewContainerRef', () => {
   let directiveInstance: DirectiveWithVCRef|null;
@@ -101,14 +101,8 @@ describe('ViewContainerRef', () => {
       });
 
       it('should work on components', () => {
-        class HeaderComponent {
+        const HeaderComponent =
-          static ngComponentDef = defineComponent({
+            createComponent('header-cmp', function(rf: RenderFlags, ctx: any) {});
-            type: HeaderComponent,
-            selectors: [['header-cmp']],
-            factory: () => new HeaderComponent(),
-            template: (rf: RenderFlags, cmp: HeaderComponent) => {}
-          });
-        }
 
         function createTemplate() {
           container(0, embeddedTemplate);
@@ -139,6 +133,38 @@ describe('ViewContainerRef', () => {
         expect(() => { createView('Z', 5); }).toThrow();
       });
 
+      it('should work with multiple instances with vcrefs', () => {
+        let firstDir: DirectiveWithVCRef;
+        let secondDir: DirectiveWithVCRef;
+
+        function createTemplate() {
+          container(0, embeddedTemplate);
+          elementStart(1, 'div', ['vcref', '']);
+          elementEnd();
+          elementStart(2, 'div', ['vcref', '']);
+          elementEnd();
+
+          // for testing only:
+          firstDir = loadDirective(0);
+          secondDir = loadDirective(1);
+        }
+
+        function update() {
+          // Hack until we can create local refs to templates
+          const tplRef = getOrCreateTemplateRef(getOrCreateNodeInjectorForNode(load(0)));
+          elementProperty(1, 'tplRef', bind(tplRef));
+          elementProperty(2, 'tplRef', bind(tplRef));
+        }
+
+        const fixture = new TemplateFixture(createTemplate, update, [DirectiveWithVCRef]);
+        expect(fixture.html).toEqual('<div vcref=""></div><div vcref=""></div>');
+
+        firstDir !.vcref.createEmbeddedView(firstDir !.tplRef, {name: 'A'});
+        secondDir !.vcref.createEmbeddedView(secondDir !.tplRef, {name: 'B'});
+        fixture.update();
+        expect(fixture.html).toEqual('<div vcref=""></div>A<div vcref=""></div>B');
+      });
+
       it('should work on containers', () => {
         function createTemplate() {
           container(0, embeddedTemplate, undefined, ['vcref', '']);
@@ -277,7 +303,7 @@ describe('ViewContainerRef', () => {
 
            /**
             * before|
-            * <ng-template directive>A<ng-template>
+            * <ng-template testDir>A<ng-template>
             * % if (condition) {
             *  B
             * % }