angular · pkozlowski-opensource · Mar 15, 2018
diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts
@@ -19,15 +19,15 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_Vie
 import {Type} from '../type';
 
 import {assertLessThan, assertNotNull} from './assert';
-import {assertPreviousIsParent, enterView, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate} from './instructions';
+import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate} from './instructions';
 import {ComponentTemplate, DirectiveDef} from './interfaces/definition';
 import {LInjector} from './interfaces/injector';
 import {LContainerNode, LElementNode, LNode, LNodeType, LViewNode, TNodeFlags} from './interfaces/node';
 import {QueryReadType} from './interfaces/query';
 import {Renderer3} from './interfaces/renderer';
 import {LView} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
-import {insertView} from './node_manipulation';
+import {insertView, removeView} from './node_manipulation';
 import {notImplemented, stringify} from './util';
 import {EmbeddedViewRef, ViewRef, addDestroyable, createViewRef} from './view_ref';
 
@@ -551,28 +551,49 @@ class ElementRef implements viewEngine_ElementRef {
  * @returns The ViewContainerRef instance to use
  */
 export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainerRef {
-  return di.viewContainerRef ||
-      (di.viewContainerRef = new ViewContainerRef(di.node as LContainerNode));
+  if (!di.viewContainerRef) {
+    const vcRefHost = di.node;
+
+    ngDevMode && assertNodeOfPossibleTypes(vcRefHost, LNodeType.Container, LNodeType.Element);
+
+    const lContainer = createLContainer(vcRefHost.parent !, vcRefHost.view, undefined, vcRefHost);
+    const lContainerNode: LContainerNode = createLNodeObject(
+        LNodeType.Container, vcRefHost.view, vcRefHost.parent !, undefined, lContainer, null);
+
+    addToViewTree(vcRefHost.view, lContainer);
+
+    di.viewContainerRef = new ViewContainerRef(lContainerNode);
+  }
+
+  return di.viewContainerRef;
 }
 
 /**
  * A ref to a container that enables adding and removing views from that container
  * imperatively.
  */
 class ViewContainerRef implements viewEngine_ViewContainerRef {
+  private _viewRefs: viewEngine_ViewRef[] = [];
   element: viewEngine_ElementRef;
   injector: Injector;
   parentInjector: Injector;
 
-  constructor(private _node: LContainerNode) {}
+  constructor(private _lContainerNode: LContainerNode) {}
 
-  clear(): void { throw notImplemented(); }
-  get(index: number): viewEngine_ViewRef|null { throw notImplemented(); }
-  length: number;
-  createEmbeddedView<C>(
-      templateRef: viewEngine_TemplateRef<C>, context?: C|undefined,
-      index?: number|undefined): viewEngine_EmbeddedViewRef<C> {
-    const viewRef = templateRef.createEmbeddedView(context !);
+  clear(): void {
+    const lContainer = this._lContainerNode.data;
+    while (lContainer.views.length) {
+      this.remove(0);
+    }
+  }
+  get(index: number): viewEngine_ViewRef|null { return this._viewRefs[index] || null; }
+  get length(): number {
+    const lContainer = this._lContainerNode.data;
+    return lContainer.views.length;
+  }
+  createEmbeddedView<C>(templateRef: viewEngine_TemplateRef<C>, context?: C, index?: number):
+      viewEngine_EmbeddedViewRef<C> {
+    const viewRef = templateRef.createEmbeddedView(context || <any>{});
     this.insert(viewRef, index);
     return viewRef;
   }
@@ -582,36 +603,26 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
       ngModule?: viewEngine_NgModuleRef<any>|undefined): viewEngine_ComponentRef<C> {
     throw notImplemented();
   }
-  insert(viewRef: viewEngine_ViewRef, index?: number|undefined): viewEngine_ViewRef {
-    if (index == null) {
-      index = this._node.data.views.length;
-    } else {
-      // +1 because it's legal to insert at the end.
-      ngDevMode && assertLessThan(index, this._node.data.views.length + 1, 'index');
-    }
-    const lView = (viewRef as EmbeddedViewRef<any>)._lViewNode;
-    insertView(this._node, lView, index);
-
-    // TODO(pk): this is a temporary index adjustment so imperativelly inserted (through
-    // ViewContainerRef) views
-    // are not removed in the containerRefreshEnd instruction.
-    // The final fix will consist of creating a dedicated container node for views inserted through
-    // ViewContainerRef.
-    // Such container should not be trimmed as it is the case in the containerRefreshEnd
-    // instruction.
-    this._node.data.nextIndex = this._node.data.views.length;
+  insert(viewRef: viewEngine_ViewRef, index?: number): viewEngine_ViewRef {
+    const lViewNode = (viewRef as EmbeddedViewRef<any>)._lViewNode;
+    const adjustedIdx = this._adjustAndAssertIndex(index);
+
+    insertView(this._lContainerNode, lViewNode, adjustedIdx);
+    this._viewRefs.splice(adjustedIdx, 0, viewRef);
+
+    (lViewNode as{parent: LNode}).parent = this._lContainerNode;
 
     // If the view is dynamic (has a template), it needs to be counted both at the container
     // level and at the node above the container.
-    if (lView.data.template !== null) {
+    if (lViewNode.data.template !== null) {
       // Increment the container view count.
-      this._node.data.dynamicViewCount++;
+      this._lContainerNode.data.dynamicViewCount++;
 
       // Look for the parent node and increment its dynamic view count.
-      if (this._node.parent !== null && this._node.parent.data !== null) {
-        ngDevMode &&
-            assertNodeOfPossibleTypes(this._node.parent, LNodeType.View, LNodeType.Element);
-        this._node.parent.data.dynamicViewCount++;
+      if (this._lContainerNode.parent !== null && this._lContainerNode.parent.data !== null) {
+        ngDevMode && assertNodeOfPossibleTypes(
+                         this._lContainerNode.parent, LNodeType.View, LNodeType.Element);
+        this._lContainerNode.parent.data.dynamicViewCount++;
       }
     }
     return viewRef;
@@ -620,8 +631,22 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
     throw notImplemented();
   }
   indexOf(viewRef: viewEngine_ViewRef): number { throw notImplemented(); }
-  remove(index?: number|undefined): void { throw notImplemented(); }
+  remove(index?: number): void {
+    const adjustedIdx = this._adjustAndAssertIndex(index);
+    removeView(this._lContainerNode, adjustedIdx);
+    this._viewRefs.splice(adjustedIdx, 1);
+  }
   detach(index?: number|undefined): viewEngine_ViewRef|null { throw notImplemented(); }
+
+  private _adjustAndAssertIndex(index?: number|undefined) {
+    if (index == null) {
+      index = this._lContainerNode.data.views.length;
+    } else {
+      // +1 because it's legal to insert at the end.
+      ngDevMode && assertLessThan(index, this._lContainerNode.data.views.length + 1, 'index');
+    }
+    return index;
+  }
 }
 
 /**
@@ -650,7 +675,7 @@ class TemplateRef<T> implements viewEngine_TemplateRef<T> {
   }
 
   createEmbeddedView(context: T): viewEngine_EmbeddedViewRef<T> {
-    let viewNode: LViewNode = renderEmbeddedTemplate(null, this._template, context, this._renderer);
+    const viewNode = renderEmbeddedTemplate(null, this._template, context, this._renderer);
     return addDestroyable(new EmbeddedViewRef(viewNode, this._template, context));
   }
 }
diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts
@@ -298,6 +298,30 @@ export function createLView(
   return newView;
 }
 
+/**
+ * Creation of LNode object is extracted to a separate function so we always create LNode object
+ * with the same shape
+ * (same properties assigned in the same order).
+ */
+export function createLNodeObject(
+    type: LNodeType, currentView: LView, parent: LNode, native: RText | RElement | null | undefined,
+    state: any,
+    queries: LQueries | null): LElementNode&LTextNode&LViewNode&LContainerNode&LProjectionNode {
+  return {
+    type: type,
+    native: native as any,
+    view: currentView,
+    parent: parent as any,
+    child: null,
+    next: null,
+    nodeInjector: parent ? parent.nodeInjector : null,
+    data: state,
+    queries: queries,
+    tNode: null,
+    pNextOrParent: null
+  };
+}
+
 /**
  * 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.
@@ -323,19 +347,8 @@ export function createLNode(
       (isParent ? currentQueries : previousOrParentNode && previousOrParentNode.queries) ||
       parent && parent.queries && parent.queries.child();
   const isState = state != null;
-  const node: LElementNode&LTextNode&LViewNode&LContainerNode&LProjectionNode = {
-    type: type,
-    native: native as any,
-    view: currentView,
-    parent: parent as any,
-    child: null,
-    next: null,
-    nodeInjector: parent ? parent.nodeInjector : null,
-    data: isState ? state as any : null,
-    queries: queries,
-    tNode: null,
-    pNextOrParent: null
-  };
+  const node =
+      createLNodeObject(type, currentView, parent, native, isState ? state as any : null, queries);
 
   if ((type & LNodeType.ViewOrElement) === LNodeType.ViewOrElement && isState) {
     // Bit of a hack to bust through the readonly because there is a circular dep between
@@ -371,7 +384,7 @@ export function createLNode(
     } else if (previousOrParentNode) {
       ngDevMode && assertNull(
                        previousOrParentNode.next,
-                       `previousOrParentNode's next property should not have been set.`);
+                       `previousOrParentNode's next property should not have been set ${index}.`);
       previousOrParentNode.next = node;
     }
   }
@@ -446,8 +459,9 @@ export function renderEmbeddedTemplate<T>(
     enterView(viewNode.data, viewNode);
 
     template(context, cm);
-    refreshDynamicChildren();
     refreshDirectives();
+    refreshDynamicChildren();
+
   } finally {
     leaveView(currentView && currentView !.parent !);
     isParent = _isParent;
@@ -1185,9 +1199,11 @@ function addComponentLogic<T>(
 
   // Only component views should be added to the view tree directly. Embedded views are
   // accessed through their containers because they may be removed / re-added later.
-  const hostView = addToViewTree(createLView(
-      -1, rendererFactory.createRenderer(previousOrParentNode.native as RElement, def.rendererType),
-      tView, null, null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways));
+  const hostView = addToViewTree(
+      currentView, createLView(
+                       -1, rendererFactory.createRenderer(
+                               previousOrParentNode.native as RElement, def.rendererType),
+                       tView, null, null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways));
 
   (previousOrParentNode.data as any) = hostView;
   (hostView.node as any) = previousOrParentNode;
@@ -1291,6 +1307,26 @@ function generateInitialInputs(
 //// ViewContainer & View
 //////////////////////////
 
+
+export function createLContainer(
+    parentLNode: LNode, currentView: LView, template?: ComponentTemplate<any>,
+    host?: LContainerNode | LElementNode): LContainer {
+  ngDevMode && assertNotNull(parentLNode, 'containers should have a parent');
+  return <LContainer>{
+    views: [],
+    nextIndex: 0,
+    // If the direct parent of the container is a view, its views will need to be added
+    // through insertView() when its parent view is being inserted:
+    renderParent: canInsertNativeNode(parentLNode, currentView) ? parentLNode : null,
+    template: template == null ? null : template,
+    next: null,
+    parent: currentView,
+    dynamicViewCount: 0,
+    queries: null,
+    host: host == null ? null : host
+  };
+}
+
 /**
  * Creates an LContainerNode.
  *
@@ -1310,20 +1346,7 @@ export function container(
           currentView.bindingStartIndex, 'container nodes should be created before any bindings');
 
   const currentParent = isParent ? previousOrParentNode : previousOrParentNode.parent !;
-  ngDevMode && assertNotNull(currentParent, 'containers should have a parent');
-
-  const lContainer = <LContainer>{
-    views: [],
-    nextIndex: 0,
-    // If the direct parent of the container is a view, its views will need to be added
-    // through insertView() when its parent view is being inserted:
-    renderParent: canInsertNativeNode(currentParent, currentView) ? currentParent : null,
-    template: template == null ? null : template,
-    next: null,
-    parent: currentView,
-    dynamicViewCount: 0,
-    queries: null
-  };
+  const lContainer = createLContainer(currentParent, currentView, template);
 
   const node = createLNode(index, LNodeType.Container, undefined, lContainer);
 
@@ -1333,7 +1356,7 @@ export function container(
 
   // Containers are added to the current view tree instead of their embedded views
   // because views can be removed and re-inserted.
-  addToViewTree(node.data);
+  addToViewTree(currentView, node.data);
 
   if (firstTemplatePass) cacheMatchingDirectivesForNode(node.tNode);
 
@@ -1701,10 +1724,11 @@ function findComponentHost(lView: LView): LElementNode {
  * This structure will be used to traverse through nested views to remove listeners
  * and call onDestroy callbacks.
  *
+ * @param currentView The view where LView or LContainer should be added
  * @param state The LView or LContainer to add to the view tree
  * @returns The state passed in
  */
-export function addToViewTree<T extends LView|LContainer>(state: T): T {
+export function addToViewTree<T extends LView|LContainer>(currentView: LView, state: T): T {
   currentView.tail ? (currentView.tail.next = state) : (currentView.child = state);
   currentView.tail = state;
   return state;
@@ -1887,8 +1911,8 @@ export function detectChangesInternal<T>(
 
   try {
     template(component, creationMode);
-    refreshDynamicChildren();
     refreshDirectives();
+    refreshDynamicChildren();
   } finally {
     leaveView(oldView);
   }

diff --git a/packages/core/src/render3/interfaces/container.ts b/packages/core/src/render3/interfaces/container.ts
@@ -7,7 +7,7 @@
  */
 
 import {ComponentTemplate} from './definition';
-import {LElementNode, LViewNode} from './node';
+import {LContainerNode, LElementNode, LViewNode} from './node';
 import {LQueries} from './query';
 import {LView, TView} from './view';
 
@@ -80,6 +80,13 @@ export interface LContainer {
    * this container are reported to queries referenced here.
    */
   queries: LQueries|null;
+
+  /**
+   * If a LContainer is created dynamically (by a directive requesting ViewContainerRef) this fields
+   * keeps a reference to a node on which a ViewContainerRef was requested. We need to store this
+   * information to find a next render sibling node.
+   */
+  host: LContainerNode|LElementNode|null;
 }
 
 /**

diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
@@ -281,7 +281,10 @@ export function insertView(
     if (!beforeNode) {
       let containerNextNativeNode = container.native;
       if (containerNextNativeNode === undefined) {
-        containerNextNativeNode = container.native = findNextRNodeSibling(container, null);
+        // TODO(pk): this is probably too simplistic, add more tests for various host placements
+        // (dynamic view, projection, ...)
+        containerNextNativeNode = container.native =
+            findNextRNodeSibling(container.data.host ? container.data.host : container, null);
       }
       beforeNode = containerNextNativeNode;
     }

diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json
@@ -170,6 +170,9 @@
   {
     "name": "createLNode"
   },
+  {
+    "name": "createLNodeObject"
+  },
   {
     "name": "createLView"
   },
@@ -413,4 +416,4 @@
   {
     "name": "viewAttached"
   }
-]
+]