From f0f426b2d06929e3675a5beacf75295f4bed1d4b Mon Sep 17 00:00:00 2001
From: Pete Bacon Darwin <pete@bacondarwin.com>
Date: Tue, 26 Nov 2019 12:09:28 +0000
Subject: [PATCH] fix(ivy): support inserting a `viewRef` that is already
 present (#34052)

When inserting a `viewRef` it is possible to not provide
an `index`, which is regarded as appending to the end of
the container.

If the `viewRef` already exists in the container, then
this results in a move. But there was a fault in the logic
that computed where to insert the `viewRef` that did not
account for the fact that the `viewRef` was already in
the container, so the insertion `index` was outside the
bounds of the array.

Fixes #33924

PR Close #34052
---
 .../src/render3/view_engine_compatibility.ts     |  6 ++++--
 .../test/acceptance/view_container_ref_spec.ts   | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts
index a1228e0df4..623d2f663f 100644
--- a/packages/core/src/render3/view_engine_compatibility.ts
+++ b/packages/core/src/render3/view_engine_compatibility.ts
@@ -245,14 +245,16 @@ export function createContainerRef(
         }
         this.allocateContainerIfNeeded();
         const lView = (viewRef as ViewRef<any>)._lView !;
-        const adjustedIdx = this._adjustIndex(index);
 
         if (viewAttachedToContainer(lView)) {
           // If view is already attached, fall back to move() so we clean up
           // references appropriately.
-          return this.move(viewRef, adjustedIdx);
+          // Note that we "shift" -1 because the move will involve inserting
+          // one view but also removing one view.
+          return this.move(viewRef, this._adjustIndex(index, -1));
         }
 
+        const adjustedIdx = this._adjustIndex(index);
         insertView(lView, this._lContainer, adjustedIdx);
 
         const beforeNode = getBeforeNodeForView(adjustedIdx, this._lContainer);
diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts
index e393c932f4..b023d21a83 100644
--- a/packages/core/test/acceptance/view_container_ref_spec.ts
+++ b/packages/core/test/acceptance/view_container_ref_spec.ts
@@ -204,6 +204,22 @@ describe('ViewContainerRef', () => {
         expect(fixture.nativeElement.textContent).toEqual('102');
       }
     });
+
+    it('should do nothing when a view is re-inserted / moved at the same index', () => {
+      const fixture = TestBed.createComponent(ViewContainerRefApp);
+      fixture.detectChanges();
+
+      const templates = fixture.componentInstance.vcrComp.templates.toArray();
+      const viewContainerRef = fixture.componentInstance.vcrComp.vcr;
+
+      const ref0 = viewContainerRef.createEmbeddedView(templates[0]);
+
+      expect(fixture.nativeElement.textContent).toEqual('0');
+
+      // insert again at the same place but without specifying any index
+      viewContainerRef.insert(ref0);
+      expect(fixture.nativeElement.textContent).toEqual('0');
+    });
   });
 
   describe('move', () => {