From 6ae0084255eefd801708c6d89782faa31c02a341 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Fri, 19 Apr 2019 19:46:52 +0200
Subject: [PATCH] fix(ivy): error when calling remove() or detach() on an empty
 view container (#29986)

Fixes Ivy throwing an error if `remove()` or `detach()` are called on an empty `ViewContainerRef`.

This PR resolves FW-1270.

PR Close #29986
---
 .../core/src/render3/node_manipulation.ts     | 34 +++++++++++--------
 .../src/render3/view_engine_compatibility.ts  |  4 +--
 .../acceptance/view_container_ref_spec.ts     | 32 +++++++++++++----
 3 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
index cfc42d8b43..5fa4eacdf0 100644
--- a/packages/core/src/render3/node_manipulation.ts
+++ b/packages/core/src/render3/node_manipulation.ts
@@ -371,23 +371,25 @@ export function insertView(lView: LView, lContainer: LContainer, index: number)
  * @param removeIndex The index of the view to detach
  * @returns Detached LView instance.
  */
-export function detachView(lContainer: LContainer, removeIndex: number): LView {
+export function detachView(lContainer: LContainer, removeIndex: number): LView|undefined {
   const views = lContainer[VIEWS];
   const viewToDetach = views[removeIndex];
-  if (removeIndex > 0) {
-    views[removeIndex - 1][NEXT] = viewToDetach[NEXT] as LView;
-  }
-  views.splice(removeIndex, 1);
-  addRemoveViewFromContainer(viewToDetach, false);
+  if (viewToDetach) {
+    if (removeIndex > 0) {
+      views[removeIndex - 1][NEXT] = viewToDetach[NEXT] as LView;
+    }
+    views.splice(removeIndex, 1);
+    addRemoveViewFromContainer(viewToDetach, false);
 
-  if ((viewToDetach[FLAGS] & LViewFlags.Attached) &&
-      !(viewToDetach[FLAGS] & LViewFlags.Destroyed) && viewToDetach[QUERIES]) {
-    viewToDetach[QUERIES] !.removeView();
+    if ((viewToDetach[FLAGS] & LViewFlags.Attached) &&
+        !(viewToDetach[FLAGS] & LViewFlags.Destroyed) && viewToDetach[QUERIES]) {
+      viewToDetach[QUERIES] !.removeView();
+    }
+    viewToDetach[PARENT] = null;
+    viewToDetach[NEXT] = null;
+    // Unsets the attached flag
+    viewToDetach[FLAGS] &= ~LViewFlags.Attached;
   }
-  viewToDetach[PARENT] = null;
-  viewToDetach[NEXT] = null;
-  // Unsets the attached flag
-  viewToDetach[FLAGS] &= ~LViewFlags.Attached;
   return viewToDetach;
 }
 
@@ -399,8 +401,10 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView {
  */
 export function removeView(lContainer: LContainer, removeIndex: number) {
   const view = lContainer[VIEWS][removeIndex];
-  detachView(lContainer, removeIndex);
-  destroyLView(view);
+  if (view) {
+    detachView(lContainer, removeIndex);
+    destroyLView(view);
+  }
 }
 
 /**
diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts
index 2b70809100..7a45a13adf 100644
--- a/packages/core/src/render3/view_engine_compatibility.ts
+++ b/packages/core/src/render3/view_engine_compatibility.ts
@@ -281,8 +281,8 @@ export function createContainerRef(
       detach(index?: number): viewEngine_ViewRef|null {
         const adjustedIdx = this._adjustIndex(index, -1);
         const view = detachView(this._lContainer, adjustedIdx);
-        const wasDetached = this._viewRefs.splice(adjustedIdx, 1)[0] != null;
-        return wasDetached ? new ViewRef(view, view[CONTEXT], -1) : null;
+        const wasDetached = view && this._viewRefs.splice(adjustedIdx, 1)[0] != null;
+        return wasDetached ? new ViewRef(view !, view ![CONTEXT], -1) : null;
       }
 
       private _adjustIndex(index?: number, shift: number = 0) {
diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts
index 50a79dccbc..82d08ed6e2 100644
--- a/packages/core/test/acceptance/view_container_ref_spec.ts
+++ b/packages/core/test/acceptance/view_container_ref_spec.ts
@@ -113,6 +113,26 @@ describe('ViewContainerRef', () => {
         });
   });
 
+  it('should not throw when calling remove() on an empty container', () => {
+    const fixture = TestBed.createComponent(ViewContainerRefApp);
+    fixture.detectChanges();
+
+    const viewContainerRef = fixture.componentInstance.vcrComp.vcr;
+
+    expect(viewContainerRef.length).toBe(0);
+    expect(() => viewContainerRef.remove()).not.toThrow();
+  });
+
+  it('should not throw when calling detach() on an empty container', () => {
+    const fixture = TestBed.createComponent(ViewContainerRefApp);
+    fixture.detectChanges();
+
+    const viewContainerRef = fixture.componentInstance.vcrComp.vcr;
+
+    expect(viewContainerRef.length).toBe(0);
+    expect(() => viewContainerRef.detach()).not.toThrow();
+  });
+
   describe('destroy should clean the DOM in all cases:', () => {
     function executeTest(template: string) {
       TestBed.overrideTemplate(DestroyCasesComp, template).configureTestingModule({
@@ -154,7 +174,7 @@ describe('ViewContainerRef', () => {
         <ng-template #foo>
           <span>Foo</span>
         </ng-template>
-        
+
         <ng-template structDir>
           <before></before>
           <ng-container [ngTemplateOutlet]="foo">
@@ -169,7 +189,7 @@ describe('ViewContainerRef', () => {
       <ng-template #foo>
         <span>Foo</span>
       </ng-template>
-      
+
       <ng-template structDir>
         <before></before>
         <div [ngTemplateOutlet]="foo">
@@ -184,7 +204,7 @@ describe('ViewContainerRef', () => {
       <ng-template #foo>
         <span>Foo</span>
       </ng-template>
-      
+
       <ng-template structDir>
         <before></before>
         <ng-template [ngTemplateOutlet]="foo"></ng-template>
@@ -197,7 +217,7 @@ describe('ViewContainerRef', () => {
       <ng-template #foo>
         <span>Foo</span>
       </ng-template>
-      
+
       <ng-template structDir>
         <before></before>
         <ng-container>
@@ -217,7 +237,7 @@ describe('ViewContainerRef', () => {
       <ng-template #foo>
         <span i18n>Bar</span>
       </ng-template>
-      
+
       <ng-template structDir>
         <before></before>
         <ng-container i18n>
@@ -238,7 +258,7 @@ describe('ViewContainerRef', () => {
       <ng-template #foo>
         <span>Foo</span>
       </ng-template>
-      
+
       <ng-template structDir i18n>
         <before></before>
         <div [ngTemplateOutlet]="foo">