fix(core): fix multiple nested views removal from ViewContainerRef (angular#38317)

When removal of one view causes removal of another one from the same
ViewContainerRef it triggers an error with views length calculation. This commit
fixes this bug by removing a view from the list of available views before invoking
actual view removal (which might be recursive and relies on the length of the list
of available views).

Fixes angular#38201.

PR Close angular#38317

Author: waterplea

packages/core/src/render3/node_manipulation.ts

@@ -349,17 +349,6 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView|u
   return viewToDetach;
 }
 
-/**
- * Removes a view from a container, i.e. detaches it and then destroys the underlying LView.
- *
- * @param lContainer The container from which to remove a view
- * @param removeIndex The index of the view to remove
- */
-export function removeView(lContainer: LContainer, removeIndex: number) {
-  const detachedView = detachView(lContainer, removeIndex);
-  detachedView && destroyLView(detachedView[TVIEW], detachedView);
-}
-
 /**
  * A standalone function which destroys an LView,
  * conducting clean up (e.g. removing listeners, calling onDestroys).

packages/core/src/render3/view_engine_compatibility.ts

@@ -22,12 +22,12 @@ import {assertLContainer} from './assert';
 import {getParentInjectorLocation, NodeInjector} from './di';
 import {addToViewTree, createLContainer, createLView, renderView} from './instructions/shared';
 import {CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './interfaces/container';
-import {TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
+import {TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNode, TNodeType} from './interfaces/node';
 import {isProceduralRenderer, RComment, RElement} from './interfaces/renderer';
 import {isComponentHost, isLContainer, isLView, isRootView} from './interfaces/type_checks';
 import {DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, LView, LViewFlags, PARENT, QUERIES, RENDERER, T_HOST, TVIEW, TView} from './interfaces/view';
 import {assertNodeOfPossibleTypes} from './node_assert';
-import {addRemoveViewFromContainer, appendChild, detachView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode, removeView} from './node_manipulation';
+import {addRemoveViewFromContainer, appendChild, destroyLView, detachView, getBeforeNodeForView, insertView, nativeInsertBefore, nativeNextSibling, nativeParentNode} from './node_manipulation';
 import {getParentInjectorTNode} from './node_util';
 import {getLView, getPreviousOrParentTNode} from './state';
 import {getParentInjectorView, hasParentInjector} from './util/injector_utils';
@@ -304,8 +304,18 @@ export function createContainerRef(
       remove(index?: number): void {
         this.allocateContainerIfNeeded();
         const adjustedIdx = this._adjustIndex(index, -1);
-        removeView(this._lContainer, adjustedIdx);
-        removeFromArray(this._lContainer[VIEW_REFS]!, adjustedIdx);
+        const detachedView = detachView(this._lContainer, adjustedIdx);
+
+        if (detachedView) {
+          // Before destroying the view, remove it from the container's array of `ViewRef`s.
+          // This ensures the view container length is updated before calling
+          // `destroyLView`, which could recursively call view container methods that
+          // rely on an accurate container length.
+          // (e.g. a method on this view container being called by a child directive's OnDestroy
+          // lifecycle hook)
+          removeFromArray(this._lContainer[VIEW_REFS]!, adjustedIdx);
+          destroyLView(detachedView[TVIEW], detachedView);
+        }
       }
 
       detach(index?: number): viewEngine_ViewRef|null {

packages/core/test/acceptance/view_container_ref_spec.ts

@@ -8,7 +8,7 @@
 
 import {CommonModule, DOCUMENT} from '@angular/common';
 import {computeMsgId} from '@angular/compiler';
-import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, Injector, NgModule, NO_ERRORS_SCHEMA, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵsetDocument} from '@angular/core';
+import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, Injector, NgModule, NO_ERRORS_SCHEMA, OnDestroy, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵsetDocument} from '@angular/core';
 import {Input} from '@angular/core/src/metadata';
 import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
 import {TestBed, TestComponentRenderer} from '@angular/core/testing';
@@ -936,6 +936,62 @@ describe('ViewContainerRef', () => {
     });
   });
 
+  describe('dependant views', () => {
+    it('should not throw when view removes another view upon removal', () => {
+      @Component({
+        template: `
+          <div *ngIf="visible" [template]="parent">I host a template</div>
+          <ng-template #parent>
+              <div [template]="child">I host a child template</div>
+          </ng-template>
+          <ng-template #child>
+              I am child template
+          </ng-template>
+        `
+      })
+      class AppComponent {
+        visible = true;
+
+        constructor(private readonly vcr: ViewContainerRef) {}
+
+        add<C>(template: TemplateRef<C>): EmbeddedViewRef<C> {
+          return this.vcr.createEmbeddedView(template);
+        }
+
+        remove<C>(viewRef: EmbeddedViewRef<C>) {
+          this.vcr.remove(this.vcr.indexOf(viewRef));
+        }
+      }
+
+      @Directive({selector: '[template]'})
+      class TemplateDirective<C> implements OnInit, OnDestroy {
+        @Input() template !: TemplateRef<C>;
+        ref!: EmbeddedViewRef<C>;
+
+        constructor(private readonly host: AppComponent) {}
+
+        ngOnInit() {
+          this.ref = this.host.add(this.template);
+          this.ref.detectChanges();
+        }
+
+        ngOnDestroy() {
+          this.host.remove(this.ref);
+        }
+      }
+
+      TestBed.configureTestingModule({
+        imports: [CommonModule],
+        declarations: [AppComponent, TemplateDirective],
+      });
+
+      const fixture = TestBed.createComponent(AppComponent);
+      fixture.detectChanges();
+      fixture.componentRef.instance.visible = false;
+      fixture.detectChanges();
+    });
+  });
+
   describe('createEmbeddedView (incl. insert)', () => {
     it('should work on elements', () => {
       @Component({