fix(ivy): handle cases of inserting LView into LContainer

Under some circumstances inserting `LView` into `LContainer` would get
inserted in a wrong DOM location resulting in incorrect DOM node order.

These two use cases were corrected:
- Inserting in front of empty `LView`
  ```
  <ng-template></ng-template>`
  ```
- Inserting in front of nested `Lview`
  ```
  <ng-template>
    <ng-template [ngIf]="true">Nested</ng-template>
  </ng-template>`
  ```

packages/core/src/render3/node_manipulation.ts

@@ -663,27 +663,54 @@ export function appendChild(childEl: RNode | RNode[], childTNode: TNode, current
   }
 }
 
-export function getBeforeNodeForView(viewIndexInContainer: number, lContainer: LContainer): RNode|
-    null {
-  const nextViewIndex = CONTAINER_HEADER_OFFSET + viewIndexInContainer + 1;
-  if (nextViewIndex < lContainer.length) {
-    const lView = lContainer[nextViewIndex] as LView;
-    ngDevMode && assertDefined(lView[T_HOST], 'Missing Host TNode');
-    let tViewNodeChild = (lView[T_HOST] as TViewNode).child;
-    if (tViewNodeChild !== null) {
-      if (tViewNodeChild.type === TNodeType.ElementContainer ||
-          tViewNodeChild.type === TNodeType.IcuContainer) {
-        let currentChild = tViewNodeChild.child;
-        while (currentChild && (currentChild.type === TNodeType.ElementContainer ||
-                                currentChild.type === TNodeType.IcuContainer)) {
-          currentChild = currentChild.child;
-        }
-        tViewNodeChild = currentChild || tViewNodeChild;
+/**
+ * Retrieves `RNode` from `LContainer` to be used for DOM operations which is need as `before` node.
+ *
+ * When inserting `LView`s into DOM we often need to know a `before` node to insert into the DOM
+ * tree into. This function is useful for returning the `RNode` since it retrieves the first `RNode`
+ * of the `LView` at a given `index` in the `LContainer`.
+ *
+ * This method takes into account the fact that the `LView`'s first `TNode` may be another `LView`,
+ * `IcuContainer`, or `ElementContainer` which requires descending into those locations in order
+ * to retrieve the `RNode`.
+ *
+ * @param lViewIndex Index into the `LContainer` which points to the `LView` of interest
+ * @param lContainer `LContainer` to look into.
+ * @returns `RNode` of the `LView` at `index` in the `LContainer` or `LContainer`'s anchor node if
+ * no `LView` is found at `index`.
+ */
+export function getFirstRNodeFromLViewInLContainer(
+    lContainer: LContainer, lViewIndex: number): RNode {
+  ngDevMode && assertLContainer(lContainer);
+  // We read the `LView` and look for first `TNode`. However, if we have an empty `TView` (such as
+  // `<ng-template></ng-template>) we need to go to the next `LView` to and repeat the process. If
+  // we run of out of `LView`s to look at we return the `LContainer` anchor.
+  for (let i = CONTAINER_HEADER_OFFSET + lViewIndex; i < lContainer.length; i++) {
+    const lView = lContainer[i];
+    let tNode = lView[TVIEW].firstChild;
+    if (tNode !== null) {
+    let tNodeType = tNode.type;
+    if (tNodeType === TNodeType.Container) {
+      // In this case the first `TNode` is another `LContainer` this means we now need to
+      // recurse
+      // into it the `LView` and retrieve its `RNode`.
+      const childLContainer = lView[tNode.index] as LContainer;
+      ngDevMode && assertLContainer(childLContainer);
+      return getFirstRNodeFromLViewInLContainer(childLContainer, 0);
+    } else {
+      // If we have `ElementContainer` or `IcuContainer` than we need to descend into them and
+      // find the actual first `Element`
+      while (tNodeType === TNodeType.ElementContainer || tNodeType === TNodeType.IcuContainer) {
+        tNode = tNode.child !;
+        ngDevMode && assertDefined(tNode, 'ElementContainer and IcuContainer must have children');
+        tNodeType = tNode.type;
       }
-      return getNativeByTNodeOrNull(tViewNodeChild, lView);
+      return getNativeByTNode(tNode, lView);
     }
   }
-
+  }
+  // If we could not find any `RNode` in any of the `LView`s, than just use the `LContainer`'s
+  // anchor.
   return lContainer[NATIVE];
 }
 

packages/core/src/render3/view_engine_compatibility.ts

@@ -25,9 +25,9 @@ import {ACTIVE_INDEX, CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './in
 import {TContainerNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
 import {RComment, RElement, isProceduralRenderer} from './interfaces/renderer';
 import {isComponentHost, isLContainer, isLView, isRootView} from './interfaces/type_checks';
-import {CONTEXT, DECLARATION_LCONTAINER, LView, LViewFlags, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
+import {DECLARATION_LCONTAINER, LView, LViewFlags, QUERIES, RENDERER, TView, T_HOST} from './interfaces/view';
 import {assertNodeOfPossibleTypes} from './node_assert';
-import {addRemoveViewFromContainer, appendChild, detachView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
+import {addRemoveViewFromContainer, appendChild, detachView, getFirstRNodeFromLViewInLContainer, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
 import {getParentInjectorTNode} from './node_util';
 import {getLView, getPreviousOrParentTNode} from './state';
 import {getParentInjectorView, hasParentInjector} from './util/injector_utils';
@@ -254,9 +254,9 @@ export function createContainerRef(
           return this.move(viewRef, adjustedIdx);
         }
 
+        const beforeNode = getFirstRNodeFromLViewInLContainer(this._lContainer, adjustedIdx);
         insertView(lView, this._lContainer, adjustedIdx);
 
-        const beforeNode = getBeforeNodeForView(adjustedIdx, this._lContainer);
         addRemoveViewFromContainer(lView, true, beforeNode);
 
         (viewRef as ViewRef<any>).attachToViewContainerRef(this);

packages/core/test/acceptance/view_insertion_spec.ts

@@ -7,9 +7,10 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {ChangeDetectorRef, Component, EmbeddedViewRef, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
+import {ChangeDetectorRef, Component, Directive, EmbeddedViewRef, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
+import {ivyEnabled} from '@angular/private/testing';
 
 describe('view insertion', () => {
   describe('of a simple template', () => {
@@ -18,7 +19,7 @@ describe('view insertion', () => {
 
       @Component({
         selector: 'increment-comp',
-        template: `<span>created{{counter}}</span>`,
+        template: `<span>Created{{counter}}</span>`,
       })
       class IncrementComp {
         counter = _counter++;
@@ -27,6 +28,8 @@ describe('view insertion', () => {
       @Component({
         template: `
               <ng-template #simple><increment-comp></increment-comp></ng-template>
+              <ng-template #nested><ng-template [ngIf]="true">Nested</ng-template></ng-template>
+              <ng-template #empty></ng-template>
               <div #container></div>
             `
       })
@@ -37,25 +40,49 @@ describe('view insertion', () => {
         @ViewChild('simple', {read: TemplateRef, static: true})
         simple: TemplateRef<any> = null !;
 
+        // This case demonstrates when `LView` contains another `LContainer` and requires
+        // recursion when we retrieve the insert before `RNode`
+        @ViewChild('nested', {read: TemplateRef, static: true})
+        nested: TemplateRef<any> = null !;
+
+        // This case demonstrates when `LView` contains no `TNode` and we have to go to the next
+        // `LView` in order to get a real `RNode`
+        @ViewChild('empty', {read: TemplateRef, static: true})
+        empty: TemplateRef<any> = null !;
+
         view0: EmbeddedViewRef<any> = null !;
         view1: EmbeddedViewRef<any> = null !;
         view2: EmbeddedViewRef<any> = null !;
         view3: EmbeddedViewRef<any> = null !;
+        view4: EmbeddedViewRef<any> = null !;
 
         constructor(public changeDetector: ChangeDetectorRef) {}
 
         ngAfterViewInit() {
-          // insert at the front
-          this.view1 = this.container.createEmbeddedView(this.simple);  // "created0"
+          // insert at the front. Because this is empty and has no `RNode` any insertions in
+          // front of it need to fall through to the next `LView` to get insertion node.
+          this.view1 = this.container.createEmbeddedView(this.empty);
+          expect(fixture.nativeElement.textContent).toBe('');
+
+          this.view2 = this.container.createEmbeddedView(this.nested);  // "nested"
+          this.view2.detectChanges();
+          expect(fixture.nativeElement.textContent).toBe('Nested');
 
           // insert at the front again
-          this.view0 = this.container.createEmbeddedView(this.simple, {}, 0);  // "created1"
+          debugger;
+          this.view0 = this.container.createEmbeddedView(this.simple, {}, 0);  // "Created0"
+          this.view0.detectChanges();
+          expect(fixture.nativeElement.textContent).toBe('Created0Nested');
 
           // insert at the end
-          this.view3 = this.container.createEmbeddedView(this.simple);  // "created2"
+          this.view4 = this.container.createEmbeddedView(this.simple);  // "Created1"
+          this.view4.detectChanges();
+          expect(fixture.nativeElement.textContent).toBe('Created0NestedCreated1');
 
           // insert in the middle
-          this.view2 = this.container.createEmbeddedView(this.simple, {}, 2);  // "created3"
+          this.view3 = this.container.createEmbeddedView(this.simple, {}, 3);  // "Created2"
+          this.view3.detectChanges();
+          expect(fixture.nativeElement.textContent).toBe('Created0NestedCreated2Created1');
 
           // We need to run change detection here to avoid
           // ExpressionChangedAfterItHasBeenCheckedError because of the value updating in
@@ -75,8 +102,9 @@ describe('view insertion', () => {
       expect(app.container.indexOf(app.view1)).toBe(1);
       expect(app.container.indexOf(app.view2)).toBe(2);
       expect(app.container.indexOf(app.view3)).toBe(3);
+      expect(app.container.indexOf(app.view4)).toBe(4);
       // The text in each component differs based on *when* it was created.
-      expect(fixture.nativeElement.textContent).toBe('created1created0created3created2');
+      expect(fixture.nativeElement.textContent).toBe('Created0NestedCreated2Created1');
     });
   });
 
@@ -249,4 +277,82 @@ describe('view insertion', () => {
       expect(fixture.debugElement.queryAll(By.css('div.dynamic')).length).toBe(4);
     });
   });
+
+  describe('regression', () => {
+    it('FW-1559: should support inserting in front of nested `LContainers', () => {
+      @Component({
+        selector: 'repeater',
+        template: `
+          <ng-template ngFor [ngForOf]="rendered" [ngForTrackBy]="trackByFn" let-item>
+            <ng-template [ngTemplateOutlet]="outerTemplate"
+                        [ngTemplateOutletContext]="{$implicit: item}">
+            </ng-template>
+          </ng-template>
+
+          <ng-template #outerTemplate let-item>{{item}}</ng-template>
+        `
+      })
+      class Repeater {
+        rendered: string[] = [];
+
+        trackByFn(index: number, item: string) { return item; }
+      }
+
+      const fixture =
+          TestBed.configureTestingModule({declarations: [Repeater]}).createComponent(Repeater);
+      fixture.componentInstance.rendered = ['b'];
+      fixture.detectChanges();
+
+      fixture.componentInstance.rendered = ['a', 'b'];
+      (global as any).break = true;
+      fixture.detectChanges();
+      expect(fixture.nativeElement.textContent).toEqual('ab');
+    });
+
+    it('FW-1559: should properly render content if vcr.createEmbeddedView is called', () => {
+      @Directive({selector: '[dir]'})
+      class MyDir {
+        viewRef: any;
+
+        constructor(public template: TemplateRef<{}>, public viewContainerRef: ViewContainerRef) {}
+
+        show() {
+          this.viewRef = this.viewContainerRef.createEmbeddedView(this.template);
+          this.viewRef.detectChanges();
+        }
+      }
+
+      @Component({
+        selector: 'edit-form',
+        template: `
+          <ng-template dir>
+            <div *ngIf="myFlag">Text</div>
+          </ng-template>
+        `,
+      })
+      class MyComp {
+        myFlag: boolean = true;
+        @ViewChild(MyDir, {static: true}) dir !: MyDir;
+      }
+
+      TestBed.configureTestingModule({
+        declarations: [MyComp, MyDir],
+        imports: [CommonModule],
+      });
+
+      const fixture = TestBed.createComponent(MyComp);
+      fixture.detectChanges();
+
+      const dirRef = fixture.componentInstance.dir;
+      dirRef.show();
+
+      // In VE:  3 nodes - 2 comment nodes + 1 div
+      // In Ivy: 2 comment node (anchor)
+      const rootNodes = dirRef.viewRef.rootNodes;
+      expect(rootNodes.length).toBe(ivyEnabled ? 2 : 3);
+      expect((rootNodes[0] as Comment).textContent).toMatch(/"ng-reflect-ng-if": "true"/);
+      expect((rootNodes[1] as HTMLElement).outerHTML).toEqual('<div>Text</div>');
+    });
+
+  });
 });