feat(ivy): implement ViewContainerRef.remove (#24221)

PR Close #24221

packages/core/src/render3/STATUS.md

@@ -238,22 +238,25 @@ The goal is for the `@Component` (and friends) to be the compiler of template. S
 ### `______Ref`s
 | Method                 | View Container Ref | Template Ref | Embeded View Ref | View Ref | Element Ref | Change Detection Ref |
 | ---------------------- | ------------------ | ------------ | ---------------- | -------- | ----------- | -------------------- |
-| `clear()`              |  ❌                | n/a          | n/a              | n/a      | n/a         | n/a                  |
+| `clear()`              |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `get()`                |  ❌                | n/a          | n/a              | n/a      | n/a         | n/a                  |
+| `get()`                |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
 | `createEmbededView()`  |  ✅                | ✅           | n/a              | n/a      | n/a         | n/a                  |
-| `createComponent()`    |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
+| `createComponent()`    |  ❌                | n/a          | n/a              | n/a      | n/a         | n/a                  |
 | `insert()`             |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `move()`               |  ❌                | n/a          | n/a              | n/a      | n/a         | n/a                  |
+| `move()`               |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `indexOf()`            |  ❌                | n/a          | n/a              | n/a      | n/a         | n/a                  |
+| `indexOf()`            |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `destroy()`            | n/a                | n/a          |  ❌              | ❌       | n/a         | n/a                  |
+| `length()`             |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `destroyed`            | n/a                | n/a          |  ❌              | ❌       | n/a         | n/a                  |
+| `remove()`             |  ✅                | n/a          | n/a              | n/a      | n/a         | n/a                  |
-| `onDestroy()`          | n/a                | n/a          |  ❌              | ❌       | n/a         | n/a                  |
+| `destroy()`            | n/a                | n/a          |  ✅              | ✅       | n/a         | n/a                  |
-| `markForCheck()`       | n/a                | n/a          |  ❌              | n/a      | n/a         | ✅                   |
+| `destroyed`            | n/a                | n/a          |  ✅              | ✅       | n/a         | n/a                  |
-| `detach()`             |  ❌                | n/a          |  ❌              | n/a      | n/a         | ✅                   |
+| `onDestroy()`          | n/a                | n/a          |  ✅              | ✅       | n/a         | n/a                  |
-| `detachChanges()`      | n/a                | n/a          |  ❌              | n/a      | n/a         | ✅                   |
+| `markForCheck()`       | n/a                | n/a          |  ✅              | ✅       | n/a         | ✅                   |
-| `checkNoChanges()`     | n/a                | n/a          |  ❌              | n/a      | n/a         | ✅                   |
+| `detach()`             |  ✅                | n/a          |  ✅              | ✅       | n/a         | ✅                   |
-| `reattach()`           | n/a                | n/a          |  ❌              | n/a      | n/a         | ✅                   |
+| `detachChanges()`      | n/a                | n/a          |  ✅              | ✅       | n/a         | ✅                   |
-| `nativeElement()`      | n/a                | n/a          | n/a              | n/a      |  ✅         | n/a                  |
+| `checkNoChanges()`     | n/a                | n/a          |  ✅              | ✅       | n/a         | ✅                   |
+| `reattach()`           | n/a                | n/a          |  ✅              | ✅       | n/a         | ✅                   |
+| `nativeElement()`      | n/a                | n/a          | n/a              | n/a     |  ✅         | n/a                  |
+| `elementRef`           | n/a                | ✅           | n/a              | n/a      |  n/a        | n/a                  |
 
 ### Renderer2
 | Method                              | Runtime |
@@ -268,8 +271,8 @@ The goal is for the `@Component` (and friends) to be the compiler of template. S
 | `insertBefore()`                    |  ✅     |
 | `removeChild()`                     |  ✅     |
 | `selectRootElement()`               |  ✅     |
-| `parentNode()`                      |  ❌     |
+| `parentNode()`                      |  n/a    |
-| `nextSibling()`                     |  ❌     |
+| `nextSibling()`                     |  n/a    |
 | `setAttribute()`                    |  ✅     |
 | `removeAttribute()`                 |  ✅     |
 | `addClass()`                        |  ✅     |

packages/core/src/render3/component.ts

@@ -21,7 +21,7 @@ import {LElementNode, TNodeFlags} from './interfaces/node';
 import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
 import {LView, LViewFlags, RootContext} from './interfaces/view';
 import {stringify} from './util';
-import {createViewRef} from './view_ref';
+import {ViewRef} from './view_ref';
 
 
 
@@ -82,7 +82,7 @@ export function createComponentRef<T>(
     componentType: ComponentType<T>, opts: CreateComponentOptions): viewEngine_ComponentRef<T> {
   const component = renderComponent(componentType, opts);
   const hostView = _getComponentHostLElementNode(component).data as LView;
-  const hostViewRef = createViewRef(hostView, component);
+  const hostViewRef = new ViewRef(hostView, component);
   return {
     location: {nativeElement: getHostElement(component)},
     injector: opts.injector || NULL_INJECTOR,

packages/core/src/render3/di.ts

@@ -27,9 +27,9 @@ import {LQueries, QueryReadType} from './interfaces/query';
 import {Renderer3} from './interfaces/renderer';
 import {LView, TView} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
-import {getParentLNode, insertView, removeView} from './node_manipulation';
+import {detachView, getParentLNode, insertView, removeView} from './node_manipulation';
 import {notImplemented, stringify} from './util';
-import {EmbeddedViewRef, ViewRef, addDestroyable, createViewRef} from './view_ref';
+import {EmbeddedViewRef, ViewRef} from './view_ref';
 
 
 
@@ -282,7 +282,7 @@ export function getOrCreateChangeDetectorRef(
 
   const currentNode = di.node;
   if (isComponent(currentNode.tNode)) {
-    return di.changeDetectorRef = createViewRef(currentNode.data as LView, context);
+    return di.changeDetectorRef = new ViewRef(currentNode.data as LView, context);
   } else if (currentNode.tNode.type === TNodeType.Element) {
     return di.changeDetectorRef = getOrCreateHostChangeDetector(currentNode.view.node);
   }
@@ -298,7 +298,7 @@ function getOrCreateHostChangeDetector(currentNode: LViewNode | LElementNode):
 
   return existingRef ?
       existingRef :
-      createViewRef(
+      new ViewRef(
           hostNode.data as LView,
           hostNode.view
               .directives ![hostNode.tNode.flags >> TNodeFlags.DirectiveStartingIndexShift]);
@@ -638,9 +638,14 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
   }
 
   insert(viewRef: viewEngine_ViewRef, index?: number): viewEngine_ViewRef {
+    if (viewRef.destroyed) {
+      throw new Error('Cannot insert a destroyed View in a ViewContainer!');
+    }
     const lViewNode = (viewRef as EmbeddedViewRef<any>)._lViewNode;
     const adjustedIdx = this._adjustIndex(index);
 
+    (viewRef as EmbeddedViewRef<any>).attachToViewContainerRef(this);
+
     insertView(this._lContainerNode, lViewNode, adjustedIdx);
     // invalidate cache of next sibling RNode (we do similar operation in the containerRefreshEnd
     // instruction)
@@ -661,15 +666,14 @@ class ViewContainerRef implements viewEngine_ViewContainerRef {
   indexOf(viewRef: viewEngine_ViewRef): number { return this._viewRefs.indexOf(viewRef); }
 
   remove(index?: number): void {
-    this.detach(index);
+    const adjustedIdx = this._adjustIndex(index, -1);
-    // TODO(ml): proper destroy of the ViewRef, i.e. recursively destroy the LviewNode and its
+    removeView(this._lContainerNode, adjustedIdx);
-    // children, delete DOM nodes and QueryList, trigger hooks (onDestroy), destroy the renderer,
+    this._viewRefs.splice(adjustedIdx, 1);
-    // detach projected nodes
   }
 
   detach(index?: number): viewEngine_ViewRef|null {
     const adjustedIdx = this._adjustIndex(index, -1);
-    removeView(this._lContainerNode, adjustedIdx);
+    detachView(this._lContainerNode, adjustedIdx);
     return this._viewRefs.splice(adjustedIdx, 1)[0] || null;
   }
 
@@ -723,6 +727,6 @@ class TemplateRef<T> implements viewEngine_TemplateRef<T> {
   createEmbeddedView(context: T): viewEngine_EmbeddedViewRef<T> {
     const viewNode = renderEmbeddedTemplate(
         null, this._tView, this._template, context, this._renderer, this._queries);
-    return addDestroyable(new EmbeddedViewRef(viewNode, this._template, context));
+    return new EmbeddedViewRef(viewNode, this._template, context);
   }
 }

packages/core/src/render3/instructions.ts

@@ -81,7 +81,7 @@ let renderer: Renderer3;
 let rendererFactory: RendererFactory3;
 
 export function getRenderer(): Renderer3 {
-  // top level variables should not be exported for performance reason (PERF_NOTES.md)
+  // top level variables should not be exported for performance reasons (PERF_NOTES.md)
   return renderer;
 }
 
@@ -93,7 +93,7 @@ export function getCurrentSanitizer(): Sanitizer|null {
 let previousOrParentNode: LNode;
 
 export function getPreviousOrParentNode(): LNode {
-  // top level variables should not be exported for performance reason (PERF_NOTES.md)
+  // top level variables should not be exported for performance reasons (PERF_NOTES.md)
   return previousOrParentNode;
 }
 
@@ -126,7 +126,7 @@ let currentView: LView = null !;
 let currentQueries: LQueries|null;
 
 export function getCurrentQueries(QueryType: {new (): LQueries}): LQueries {
-  // top level variables should not be exported for performance reason (PERF_NOTES.md)
+  // top level variables should not be exported for performance reasons (PERF_NOTES.md)
   return currentQueries || (currentQueries = new QueryType());
 }
 
@@ -136,7 +136,7 @@ export function getCurrentQueries(QueryType: {new (): LQueries}): LQueries {
 let creationMode: boolean;
 
 export function getCreationMode(): boolean {
-  // top level variables should not be exported for performance reason (PERF_NOTES.md)
+  // top level variables should not be exported for performance reasons (PERF_NOTES.md)
   return creationMode;
 }
 
@@ -173,6 +173,11 @@ let directives: any[]|null;
  */
 let cleanup: any[]|null;
 
+export function getCleanup(): any[] {
+  // top level variables should not be exported for performance reasons (PERF_NOTES.md)
+  return cleanup || (cleanup = currentView.cleanup = []);
+}
+
 /**
  * In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
  *
@@ -913,7 +918,7 @@ export function listener(
 
   // In order to match current behavior, native DOM event listeners must be added for all
   // events (including outputs).
-  const cleanupFns = cleanup || (cleanup = currentView.cleanup = []);
+  const cleanupFns = getCleanup();
   ngDevMode && ngDevMode.rendererAddEventListener++;
   if (isProceduralRenderer(renderer)) {
     const wrappedListener = wrapListenerWithDirtyLogic(currentView, listenerFn);
@@ -947,7 +952,7 @@ function createOutput(outputs: PropertyAliasValue, listener: Function): void {
   for (let i = 0; i < outputs.length; i += 2) {
     ngDevMode && assertDataInRange(outputs[i] as number, directives !);
     const subscription = directives ![outputs[i] as number][outputs[i + 1]].subscribe(listener);
-    cleanup !.push(subscription.unsubscribe, subscription);
+    getCleanup().push(subscription.unsubscribe, subscription);
   }
 }
 

packages/core/src/render3/interfaces/view.ts

@@ -164,16 +164,16 @@ export const enum LViewFlags {
    * back into the parent view, `data` will be defined and `creationMode` will be
    * improperly reported as false.
    */
-  CreationMode = 0b00001,
+  CreationMode = 0b000001,
 
   /** Whether this view has default change detection strategy (checks always) or onPush */
-  CheckAlways = 0b00010,
+  CheckAlways = 0b000010,
 
   /** Whether or not this view is currently dirty (needing check) */
-  Dirty = 0b00100,
+  Dirty = 0b000100,
 
   /** Whether or not this view is currently attached to change detection tree. */
-  Attached = 0b01000,
+  Attached = 0b001000,
 
   /**
    *  Whether or not the init hooks have run.
@@ -182,7 +182,10 @@ export const enum LViewFlags {
    * runs OR the first cR() instruction that runs (so inits are run for the top level view before
    * any embedded views).
    */
-  RunInit = 0b10000,
+  RunInit = 0b010000,
+
+  /** Whether or not this view is destroyed. */
+  Destroyed = 0b100000,
 }
 
 /** Interface necessary to work with view tree traversal */

packages/core/src/render3/node_manipulation.ts

@@ -12,7 +12,7 @@ import {LContainer, unusedValueExportToPlacateAjd as unused1} from './interfaces
 import {LContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, TNodeFlags, TNodeType, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
 import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection';
 import {ProceduralRenderer3, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
-import {HookData, LView, LViewOrLContainer, TView, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
+import {HookData, LView, LViewFlags, LViewOrLContainer, TView, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
 import {assertNodeType} from './node_assert';
 import {stringify} from './util';
 
@@ -149,17 +149,72 @@ function getNextOrParentSiblingNode(initialNode: LNode, rootNode: LNode): LNode|
  * @returns RNode The first RNode of the given LNode or null if there is none.
  */
 function findFirstRNode(rootNode: LNode): RElement|RText|null {
-  let node: LNode|null = rootNode;
+  return walkLNodeTree(rootNode, rootNode, WalkLNodeTreeAction.Find) || null;
+}
+
+const enum WalkLNodeTreeAction {
+  /** returns the first available native node */
+  Find = 0,
+
+  /** node insert in the native environment */
+  Insert = 1,
+
+  /** node detach from the native environment */
+  Detach = 2,
+
+  /** node destruction using the renderer's API */
+  Destroy = 3,
+}
+
+/**
+ * Walks a tree of LNodes, applying a transformation on the LElement nodes, either only on the first
+ * one found, or on all of them.
+ * NOTE: for performance reasons, the possible actions are inlined within the function instead of
+ * being passed as an argument.
+ *
+ * @param startingNode the node from which the walk is started.
+ * @param rootNode the root node considered.
+ * @param action Identifies the action to be performed on the LElement nodes.
+ * @param renderer Optional the current renderer, required for action modes 1, 2 and 3.
+ * @param renderParentNode Optionnal the render parent node to be set in all LContainerNodes found,
+ * required for action modes 1 and 2.
+ * @param beforeNode Optionnal the node before which elements should be added, required for action
+ * modes 1.
+ */
+function walkLNodeTree(
+    startingNode: LNode | null, rootNode: LNode, action: WalkLNodeTreeAction, renderer?: Renderer3,
+    renderParentNode?: LElementNode | null, beforeNode?: RNode | null) {
+  let node: LNode|null = startingNode;
   while (node) {
     let nextNode: LNode|null = null;
     if (node.tNode.type === TNodeType.Element) {
-      // A LElementNode has a matching RNode in LElementNode.native
+      // Execute the action
-      return (node as LElementNode).native;
+      if (action === WalkLNodeTreeAction.Find) {
+        return node.native;
+      } else if (action === WalkLNodeTreeAction.Insert) {
+        const parent = renderParentNode !.native;
+        isProceduralRenderer(renderer !) ?
+            (renderer as ProceduralRenderer3)
+                .insertBefore(parent !, node.native !, beforeNode as RNode | null) :
+            parent !.insertBefore(node.native !, beforeNode as RNode | null, true);
+      } else if (action === WalkLNodeTreeAction.Detach) {
+        const parent = renderParentNode !.native;
+        isProceduralRenderer(renderer !) ?
+            (renderer as ProceduralRenderer3).removeChild(parent as RElement, node.native !) :
+            parent !.removeChild(node.native !);
+      } else if (action === WalkLNodeTreeAction.Destroy) {
+        ngDevMode && ngDevMode.rendererDestroyNode++;
+        (renderer as ProceduralRenderer3).destroyNode !(node.native !);
+      }
+      nextNode = getNextLNode(node);
     } else if (node.tNode.type === TNodeType.Container) {
       const lContainerNode: LContainerNode = (node as LContainerNode);
       const childContainerData: LContainer = lContainerNode.dynamicLContainerNode ?
           lContainerNode.dynamicLContainerNode.data :
           lContainerNode.data;
+      if (renderParentNode) {
+        childContainerData.renderParent = renderParentNode;
+      }
       nextNode =
           childContainerData.views.length ? getChildLNode(childContainerData.views[0]) : null;
     } else if (node.tNode.type === TNodeType.Projection) {
@@ -172,7 +227,6 @@ function findFirstRNode(rootNode: LNode): RElement|RText|null {
 
     node = nextNode === null ? getNextOrParentSiblingNode(node, rootNode) : nextNode;
   }
-  return null;
 }
 
 export function createTextNode(value: any, renderer: Renderer3): RText {
@@ -204,46 +258,12 @@ export function addRemoveViewFromContainer(
   ngDevMode && assertNodeType(rootNode, TNodeType.View);
   const parentNode = container.data.renderParent;
   const parent = parentNode ? parentNode.native : null;
-  let node: LNode|null = getChildLNode(rootNode);
   if (parent) {
-    while (node) {
+    let node: LNode|null = getChildLNode(rootNode);
-      let nextNode: LNode|null = null;
+    const renderer = container.view.renderer;
-      const renderer = container.view.renderer;
+    walkLNodeTree(
-      if (node.tNode.type === TNodeType.Element) {
+        node, rootNode, insertMode ? WalkLNodeTreeAction.Insert : WalkLNodeTreeAction.Detach,
-        if (insertMode) {
+        renderer, parentNode, beforeNode);
-          isProceduralRenderer(renderer) ?
-              renderer.insertBefore(parent, node.native !, beforeNode as RNode | null) :
-              parent.insertBefore(node.native !, beforeNode as RNode | null, true);
-        } else {
-          if (isProceduralRenderer(renderer)) {
-            renderer.removeChild(parent as RElement, node.native !);
-            if (renderer.destroyNode) {
-              ngDevMode && ngDevMode.rendererDestroyNode++;
-              renderer.destroyNode(node.native !);
-            }
-          } else {
-            parent.removeChild(node.native !);
-          }
-        }
-        nextNode = getNextLNode(node);
-      } else if (node.tNode.type === TNodeType.Container) {
-        // if we get to a container, it must be a root node of a view because we are only
-        // propagating down into child views / containers and not child elements
-        const childContainerData: LContainer = (node as LContainerNode).data;
-        childContainerData.renderParent = parentNode;
-        nextNode =
-            childContainerData.views.length ? getChildLNode(childContainerData.views[0]) : null;
-      } else if (node.tNode.type === TNodeType.Projection) {
-        nextNode = (node as LProjectionNode).data.head;
-      } else {
-        nextNode = getChildLNode(node as LViewNode);
-      }
-      if (nextNode === null) {
-        node = getNextOrParentSiblingNode(node, rootNode);
-      } else {
-        node = nextNode;
-      }
-    }
   }
 }
 
@@ -350,36 +370,51 @@ export function insertView(
     addRemoveViewFromContainer(container, viewNode, true, beforeNode);
   }
 
+  // Sets the attached flag
+  viewNode.data.flags |= LViewFlags.Attached;
+
   return viewNode;
 }
 
 /**
- * Removes a view from a container.
+ * Detaches a view from a container.
  *
  * This method splices the view from the container's array of active views. It also
- * removes the view's elements from the DOM and conducts cleanup (e.g. removing
+ * removes the view's elements from the DOM.
- * listeners, calling onDestroys).
  *
- * @param container The container from which to remove a view
+ * @param container The container from which to detach a view
- * @param removeIndex The index of the view to remove
+ * @param removeIndex The index of the view to detach
- * @returns The removed view
+ * @returns The detached view
  */
-export function removeView(container: LContainerNode, removeIndex: number): LViewNode {
+export function detachView(container: LContainerNode, removeIndex: number): LViewNode {
   const views = container.data.views;
   const viewNode = views[removeIndex];
   if (removeIndex > 0) {
     views[removeIndex - 1].data.next = viewNode.data.next as LView;
   }
   views.splice(removeIndex, 1);
-  destroyViewTree(viewNode.data);
   addRemoveViewFromContainer(container, viewNode, false);
-
   // Notify query that view has been removed
   const removedLview = viewNode.data;
   if (removedLview.queries) {
     removedLview.queries.removeView(removeIndex);
   }
+  // Unsets the attached flag
+  viewNode.data.flags &= ~LViewFlags.Attached;
+  return viewNode;
+}
 
+/**
+ * Removes a view from a container, i.e. detaches it and then destroys the underlying LView.
+ *
+ * @param container The container from which to remove a view
+ * @param removeIndex The index of the view to remove
+ * @returns The removed view
+ */
+export function removeView(container: LContainerNode, removeIndex: number): LViewNode {
+  const viewNode = container.data.views[removeIndex];
+  detachView(container, removeIndex);
+  destroyLView(viewNode.data);
   return viewNode;
 }
 
@@ -392,6 +427,22 @@ export function getLViewChild(view: LView): LView|LContainer|null {
   return hostNode.data ? hostNode.data : (hostNode.dynamicLContainerNode as LContainerNode).data;
 }
 
+/**
+ * A standalone function which destroys an LView,
+ * conducting cleanup (e.g. removing listeners, calling onDestroys).
+ *
+ * @param view The view to be destroyed.
+ */
+export function destroyLView(view: LView) {
+  const renderer = view.renderer;
+  if (isProceduralRenderer(renderer) && renderer.destroyNode) {
+    walkLNodeTree(view.node, view.node, WalkLNodeTreeAction.Destroy, renderer);
+  }
+  destroyViewTree(view);
+  // Sets the destroyed flag
+  view.flags |= LViewFlags.Destroyed;
+}
+
 /**
  * Determines which LViewOrLContainer to jump to when traversing back up the
  * tree in destroyViewTree.

packages/core/src/render3/query.ts

@@ -17,7 +17,7 @@ import {getSymbolIterator} from '../util';
 
 import {assertEqual, assertNotNull} from './assert';
 import {ReadFromInjectorFn, getOrCreateNodeInjectorForNode} from './di';
-import {assertPreviousIsParent, getCurrentQueries, store} from './instructions';
+import {assertPreviousIsParent, getCleanup, getCurrentQueries, store} from './instructions';
 import {DirectiveDef, unusedValueExportToPlacateAjd as unused1} from './interfaces/definition';
 import {LInjector, unusedValueExportToPlacateAjd as unused2} from './interfaces/injector';
 import {LContainerNode, LElementNode, LNode, TNode, TNodeFlags, unusedValueExportToPlacateAjd as unused3} from './interfaces/node';
@@ -416,7 +416,7 @@ export function query<T>(
   const queryList = new QueryList<T>();
   const queries = getCurrentQueries(LQueries_);
   queries.track(queryList, predicate, descend, read);
-
+  getCleanup().push(queryList.destroy, queryList);
   if (memoryIndex != null) {
     store(memoryIndex, queryList);
   }

packages/core/src/render3/view_ref.ts

@@ -6,19 +6,20 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref';
 import {EmbeddedViewRef as viewEngine_EmbeddedViewRef} from '../linker/view_ref';
 
 import {checkNoChanges, detectChanges, markViewDirty} from './instructions';
 import {ComponentTemplate} from './interfaces/definition';
 import {LViewNode} from './interfaces/node';
 import {LView, LViewFlags} from './interfaces/view';
-import {notImplemented} from './util';
+import {destroyLView} from './node_manipulation';
 
 export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T> {
   context: T;
   rootNodes: any[];
 
-  constructor(private _view: LView, context: T|null, ) { this.context = context !; }
+  constructor(protected _view: LView, context: T|null) { this.context = context !; }
 
   /** @internal */
   _setComponentContext(view: LView, context: T) {
@@ -26,9 +27,15 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T> {
     this.context = context;
   }
 
-  destroy(): void { notImplemented(); }
+  get destroyed(): boolean {
-  destroyed: boolean;
+    return (this._view.flags & LViewFlags.Destroyed) === LViewFlags.Destroyed;
-  onDestroy(callback: Function) { notImplemented(); }
+  }
+
+  destroy(): void { destroyLView(this._view); }
+
+  onDestroy(callback: Function) {
+    (this._view.cleanup || (this._view.cleanup = [])).push(callback, null);
+  }
 
   /**
    * Marks a view and all of its ancestors dirty.
@@ -213,48 +220,21 @@ export class EmbeddedViewRef<T> extends ViewRef<T> {
    * @internal
    */
   _lViewNode: LViewNode;
+  private _viewContainerRef: viewEngine_ViewContainerRef|null = null;
 
   constructor(viewNode: LViewNode, template: ComponentTemplate<T>, context: T) {
     super(viewNode.data, context);
     this._lViewNode = viewNode;
   }
-}
 
-/**
+  destroy(): void {
- * Creates a ViewRef bundled with destroy functionality.
+    if (this._viewContainerRef &&
- *
+        (this._view.flags & LViewFlags.Attached) === LViewFlags.Attached) {
- * @param context The context for this view
+      this._viewContainerRef.detach(this._viewContainerRef.indexOf(this));
- * @returns The ViewRef
+      this._viewContainerRef = null;
- */
+    }
-export function createViewRef<T>(view: LView | null, context: T): ViewRef<T> {
+    super.destroy();
-  // TODO: add detectChanges back in when implementing ChangeDetectorRef.detectChanges
+  }
-  return addDestroyable(new ViewRef(view !, context));
-}
 
-/** Interface for destroy logic. Implemented by addDestroyable. */
+  attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) { this._viewContainerRef = vcRef; }
-export interface DestroyRef<T> {
-  /** Whether or not this object has been destroyed */
-  destroyed: boolean;
-  /** Destroy the instance and call all onDestroy callbacks. */
-  destroy(): void;
-  /** Register callbacks that should be called onDestroy */
-  onDestroy(cb: Function): void;
-}
-
-/**
- * Decorates an object with destroy logic (implementing the DestroyRef interface)
- * and returns the enhanced object.
- *
- * @param obj The object to decorate
- * @returns The object with destroy logic
- */
-export function addDestroyable<T, C>(obj: any): T&DestroyRef<C> {
-  let destroyFn: Function[]|null = null;
-  obj.destroyed = false;
-  obj.destroy = function() {
-    destroyFn && destroyFn.forEach((fn) => fn());
-    this.destroyed = true;
-  };
-  obj.onDestroy = (fn: Function) => (destroyFn || (destroyFn = [])).push(fn);
-  return obj;
 }

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -194,9 +194,6 @@
   {
     "name": "addComponentLogic"
   },
-  {
-    "name": "addDestroyable"
-  },
   {
     "name": "addRemoveViewFromContainer"
   },
@@ -293,9 +290,15 @@
   {
     "name": "defineInjector"
   },
+  {
+    "name": "destroyLView"
+  },
   {
     "name": "destroyViewTree"
   },
+  {
+    "name": "detachView"
+  },
   {
     "name": "detectChanges"
   },
@@ -374,6 +377,9 @@
   {
     "name": "getChildLNode"
   },
+  {
+    "name": "getCleanup"
+  },
   {
     "name": "getCurrentSanitizer"
   },
@@ -650,6 +656,9 @@
   {
     "name": "viewAttached"
   },
+  {
+    "name": "walkLNodeTree"
+  },
   {
     "name": "wrapListenerWithDirtyAndDefault"
   },

packages/core/test/render3/imported_renderer2.ts

@@ -35,7 +35,15 @@ export class SimpleDomEventsPlugin extends EventManagerPlugin {
 export function getRendererFactory2(document: any): RendererFactory2 {
   const fakeNgZone: NgZone = new NoopNgZone();
   const eventManager = new EventManager([new SimpleDomEventsPlugin(document)], fakeNgZone);
-  return new ɵDomRendererFactory2(eventManager, new ɵDomSharedStylesHost(document));
+  const rendererFactory =
+      new ɵDomRendererFactory2(eventManager, new ɵDomSharedStylesHost(document));
+  const origCreateRenderer = rendererFactory.createRenderer;
+  rendererFactory.createRenderer = function() {
+    const renderer = origCreateRenderer.apply(this, arguments);
+    renderer.destroyNode = () => {};
+    return renderer;
+  };
+  return rendererFactory;
 }
 
 export function getAnimationRendererFactory2(document: any): RendererFactory2 {

packages/core/test/render3/query_list_spec.ts

@@ -91,4 +91,12 @@ describe('QueryList', () => {
 
   });
 
+  describe('destroy', () => {
+    it('should close all subscriptions', () => {
+      let completed = false;
+      q.changes.subscribe(() => {}, () => {}, () => { completed = true; });
+      q.destroy();
+      expect(completed).toBeTruthy();
+    });
+  });
 });

packages/core/test/render3/query_spec.ts

@@ -9,6 +9,7 @@
 import {NgForOfContext} from '@angular/common';
 import {TemplateRef, ViewContainerRef} from '@angular/core';
 
+import {EventEmitter} from '../..';
 import {QUERY_READ_CONTAINER_REF, QUERY_READ_ELEMENT_REF, QUERY_READ_FROM_NODE, QUERY_READ_TEMPLATE_REF, getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di';
 import {AttributeMarker, QueryList, defineComponent, defineDirective, detectChanges, injectViewContainerRef} from '../../src/render3/index';
 import {bind, container, containerRefreshEnd, containerRefreshStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, load, loadDirective} from '../../src/render3/instructions';
@@ -16,7 +17,7 @@ import {RenderFlags} from '../../src/render3/interfaces/definition';
 import {query, queryRefresh} from '../../src/render3/query';
 
 import {NgForOf, NgIf} from './common_with_def';
-import {ComponentFixture, createComponent, createDirective, renderComponent} from './render_util';
+import {ComponentFixture, TemplateFixture, createComponent, createDirective, renderComponent} from './render_util';
 
 
 
@@ -1237,4 +1238,58 @@ describe('query', () => {
     });
 
   });
+
+  describe('queryList', () => {
+    it('should be destroyed when the containing view is destroyed', () => {
+      let queryInstance: QueryList<any>;
+
+      const SimpleComponentWithQuery =
+          createComponent('some-component-with-query', function(rf: RenderFlags, ctx: any) {
+            let tmp: any;
+            if (rf & RenderFlags.Create) {
+              query(0, ['foo'], false, QUERY_READ_FROM_NODE);
+              elementStart(1, 'div', null, ['foo', '']);
+              elementEnd();
+            }
+            if (rf & RenderFlags.Update) {
+              queryRefresh(tmp = load<QueryList<any>>(0)) &&
+                  (ctx.query = queryInstance = tmp as QueryList<any>);
+            }
+          });
+
+      function createTemplate() { container(0); }
+
+      function updateTemplate() {
+        containerRefreshStart(0);
+        {
+          if (condition) {
+            let rf1 = embeddedViewStart(1);
+            {
+              if (rf1 & RenderFlags.Create) {
+                elementStart(0, 'some-component-with-query');
+                elementEnd();
+              }
+            }
+            embeddedViewEnd();
+          }
+        }
+        containerRefreshEnd();
+      }
+
+      /**
+       * % if (condition) {
+       *   <some-component-with-query></some-component-with-query>
+       * %}
+       */
+      let condition = true;
+      const t = new TemplateFixture(createTemplate, updateTemplate, [SimpleComponentWithQuery]);
+      expect(t.html).toEqual('<some-component-with-query><div></div></some-component-with-query>');
+      expect((queryInstance !.changes as EventEmitter<any>).closed).toBeFalsy();
+
+      condition = false;
+      t.update();
+      expect(t.html).toEqual('');
+      expect((queryInstance !.changes as EventEmitter<any>).closed).toBeTruthy();
+    });
+  });
 });

packages/core/test/render3/renderer_factory_spec.ts

@@ -209,12 +209,6 @@ describe('animation renderer factory', () => {
 
 describe('Renderer2 destruction hooks', () => {
   const rendererFactory = getRendererFactory2(document);
-  const origCreateRenderer = rendererFactory.createRenderer;
-  rendererFactory.createRenderer = function() {
-    const renderer = origCreateRenderer.apply(this, arguments);
-    renderer.destroyNode = () => {};
-    return renderer;
-  };
 
   it('should call renderer.destroyNode for each node destroyed', () => {
     let condition = true;

packages/core/test/render3/view_container_ref_spec.ts

@@ -6,13 +6,14 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '../../src/core';
+import {Component, Directive, EmbeddedViewRef, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '../../src/core';
 import {getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di';
 import {NgOnChangesFeature, defineComponent, defineDirective, definePipe, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index';
 import {bind, container, containerRefreshEnd, containerRefreshStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, load, loadDirective, projection, projectionDef, reserveSlots, text, textBinding} from '../../src/render3/instructions';
 import {RenderFlags} from '../../src/render3/interfaces/definition';
 import {pipe, pipeBind1} from '../../src/render3/pipe';
 
+import {getRendererFactory2} from './imported_renderer2';
 import {ComponentFixture, TemplateFixture} from './render_util';
 
 describe('ViewContainerRef', () => {
@@ -45,8 +46,9 @@ describe('ViewContainerRef', () => {
       }
     }
 
-    function createView(s: string, index?: number) {
+    function createView(s: string, index?: number): EmbeddedViewRef<any> {
-      directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, {name: s}, index);
+      return directiveInstance !.vcref.createEmbeddedView(
+          directiveInstance !.tplRef, {name: s}, index);
     }
 
     /**
@@ -419,13 +421,16 @@ describe('ViewContainerRef', () => {
       });
     });
 
+    const rendererFactory = getRendererFactory2(document);
+
     describe('detach', () => {
       it('should detach the right embedded view when an index is specified', () => {
-        const fixture = new TemplateFixture(createTemplate, updateTemplate, [DirectiveWithVCRef]);
+        const fixture = new TemplateFixture(
-        createView('A');
+            createTemplate, updateTemplate, [DirectiveWithVCRef], null, null, rendererFactory);
+        const viewA = createView('A');
         createView('B');
         createView('C');
-        createView('D');
+        const viewD = createView('D');
         createView('E');
         fixture.update();
         expect(fixture.html).toEqual('<p vcref=""></p>ABCDE');
@@ -433,29 +438,97 @@ describe('ViewContainerRef', () => {
         directiveInstance !.vcref.detach(3);
         fixture.update();
         expect(fixture.html).toEqual('<p vcref=""></p>ABCE');
+        expect(viewD.destroyed).toBeFalsy();
 
         directiveInstance !.vcref.detach(0);
         fixture.update();
         expect(fixture.html).toEqual('<p vcref=""></p>BCE');
+        expect(viewA.destroyed).toBeFalsy();
 
         expect(() => { directiveInstance !.vcref.detach(-1); }).toThrow();
         expect(() => { directiveInstance !.vcref.detach(42); }).toThrow();
+        expect(ngDevMode).toHaveProperties({rendererDestroyNode: 0});
       });
 
 
       it('should detach the last embedded view when no index is specified', () => {
-        const fixture = new TemplateFixture(createTemplate, updateTemplate, [DirectiveWithVCRef]);
+        const fixture = new TemplateFixture(
+            createTemplate, updateTemplate, [DirectiveWithVCRef], null, null, rendererFactory);
         createView('A');
         createView('B');
         createView('C');
         createView('D');
-        createView('E');
+        const viewE = createView('E');
         fixture.update();
         expect(fixture.html).toEqual('<p vcref=""></p>ABCDE');
 
         directiveInstance !.vcref.detach();
         fixture.update();
         expect(fixture.html).toEqual('<p vcref=""></p>ABCD');
+        expect(viewE.destroyed).toBeFalsy();
+        expect(ngDevMode).toHaveProperties({rendererDestroyNode: 0});
+      });
+    });
+
+    describe('remove', () => {
+      it('should remove the right embedded view when an index is specified', () => {
+        const fixture = new TemplateFixture(
+            createTemplate, updateTemplate, [DirectiveWithVCRef], null, null, rendererFactory);
+        const viewA = createView('A');
+        createView('B');
+        createView('C');
+        const viewD = createView('D');
+        createView('E');
+        fixture.update();
+        expect(fixture.html).toEqual('<p vcref=""></p>ABCDE');
+
+        directiveInstance !.vcref.remove(3);
+        fixture.update();
+        expect(fixture.html).toEqual('<p vcref=""></p>ABCE');
+        expect(viewD.destroyed).toBeTruthy();
+
+        directiveInstance !.vcref.remove(0);
+        fixture.update();
+        expect(fixture.html).toEqual('<p vcref=""></p>BCE');
+        expect(viewA.destroyed).toBeTruthy();
+
+        expect(() => { directiveInstance !.vcref.remove(-1); }).toThrow();
+        expect(() => { directiveInstance !.vcref.remove(42); }).toThrow();
+        expect(ngDevMode).toHaveProperties({rendererDestroyNode: 2});
+      });
+
+      it('should remove the last embedded view when no index is specified', () => {
+        const fixture = new TemplateFixture(
+            createTemplate, updateTemplate, [DirectiveWithVCRef], null, null, rendererFactory);
+        createView('A');
+        createView('B');
+        createView('C');
+        createView('D');
+        const viewE = createView('E');
+        fixture.update();
+        expect(fixture.html).toEqual('<p vcref=""></p>ABCDE');
+
+        directiveInstance !.vcref.remove();
+        fixture.update();
+        expect(fixture.html).toEqual('<p vcref=""></p>ABCD');
+        expect(viewE.destroyed).toBeTruthy();
+        expect(ngDevMode).toHaveProperties({rendererDestroyNode: 1});
+      });
+
+      it('should throw when trying to insert a removed or destroyed view', () => {
+        const fixture = new TemplateFixture(
+            createTemplate, updateTemplate, [DirectiveWithVCRef], null, null, rendererFactory);
+        const viewA = createView('A');
+        const viewB = createView('B');
+        fixture.update();
+
+        directiveInstance !.vcref.remove();
+        fixture.update();
+        expect(() => directiveInstance !.vcref.insert(viewB)).toThrow();
+
+        viewA.destroy();
+        fixture.update();
+        expect(() => directiveInstance !.vcref.insert(viewA)).toThrow();
       });
     });
 
@@ -843,6 +916,8 @@ describe('ViewContainerRef', () => {
         ngAfterViewInit() { this.log('afterViewInit-' + this.name); }
         ngAfterViewChecked() { this.log('afterViewChecked-' + this.name); }
 
+        ngOnDestroy() { this.log('onDestroy-' + this.name); }
+
         static ngComponentDef = defineComponent({
           type: ComponentWithHooks,
           selectors: [['hooks']],
@@ -936,6 +1011,30 @@ describe('ViewContainerRef', () => {
         'doCheck-A', 'doCheck-B', 'doCheck-C', 'afterContentChecked-C', 'afterViewChecked-C',
         'afterContentChecked-A', 'afterContentChecked-B', 'afterViewChecked-A', 'afterViewChecked-B'
       ]);
+
+      log.length = 0;
+      const viewRef = directiveInstance !.vcref.detach(0);
+      fixture.update();
+      expect(log).toEqual([
+        'doCheck-A', 'doCheck-B', 'afterContentChecked-A', 'afterContentChecked-B',
+        'afterViewChecked-A', 'afterViewChecked-B'
+      ]);
+
+      log.length = 0;
+      directiveInstance !.vcref.insert(viewRef !);
+      fixture.update();
+      expect(log).toEqual([
+        'doCheck-A', 'doCheck-B', 'doCheck-C', 'afterContentChecked-C', 'afterViewChecked-C',
+        'afterContentChecked-A', 'afterContentChecked-B', 'afterViewChecked-A', 'afterViewChecked-B'
+      ]);
+
+      log.length = 0;
+      directiveInstance !.vcref.remove(0);
+      fixture.update();
+      expect(log).toEqual([
+        'onDestroy-C', 'doCheck-A', 'doCheck-B', 'afterContentChecked-A', 'afterContentChecked-B',
+        'afterViewChecked-A', 'afterViewChecked-B'
+      ]);
     });
   });
 });