diff --git a/packages/core/src/render3/STATUS.md b/packages/core/src/render3/STATUS.md index 2468ba56be..944c91574f 100644 --- a/packages/core/src/render3/STATUS.md +++ b/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 | -| `get()` | ❌ | 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 | | `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 | -| `indexOf()` | ❌ | n/a | n/a | n/a | n/a | n/a | -| `destroy()` | n/a | n/a | ❌ | ❌ | n/a | n/a | -| `destroyed` | n/a | n/a | ❌ | ❌ | n/a | n/a | -| `onDestroy()` | n/a | n/a | ❌ | ❌ | n/a | n/a | -| `markForCheck()` | n/a | n/a | ❌ | n/a | n/a | ✅ | -| `detach()` | ❌ | n/a | ❌ | n/a | n/a | ✅ | -| `detachChanges()` | n/a | n/a | ❌ | n/a | n/a | ✅ | -| `checkNoChanges()` | n/a | n/a | ❌ | n/a | n/a | ✅ | -| `reattach()` | n/a | n/a | ❌ | n/a | n/a | ✅ | -| `nativeElement()` | 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 | +| `length()` | ✅ | n/a | n/a | n/a | n/a | n/a | +| `remove()` | ✅ | n/a | n/a | n/a | n/a | n/a | +| `destroy()` | n/a | n/a | ✅ | ✅ | n/a | n/a | +| `destroyed` | n/a | n/a | ✅ | ✅ | n/a | n/a | +| `onDestroy()` | n/a | n/a | ✅ | ✅ | n/a | n/a | +| `markForCheck()` | n/a | n/a | ✅ | ✅ | n/a | ✅ | +| `detach()` | ✅ | n/a | ✅ | ✅ | n/a | ✅ | +| `detachChanges()` | 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()` | ❌ | -| `nextSibling()` | ❌ | +| `parentNode()` | n/a | +| `nextSibling()` | n/a | | `setAttribute()` | ✅ | | `removeAttribute()` | ✅ | | `addClass()` | ✅ | diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 7ce6ef86bb..3049cd8de2 100644 --- a/packages/core/src/render3/component.ts +++ b/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( componentType: ComponentType, opts: CreateComponentOptions): viewEngine_ComponentRef { 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, diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 9f7a5702cc..ba5a71bb76 100644 --- a/packages/core/src/render3/di.ts +++ b/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)._lViewNode; const adjustedIdx = this._adjustIndex(index); + (viewRef as EmbeddedViewRef).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); - // TODO(ml): proper destroy of the ViewRef, i.e. recursively destroy the LviewNode and its - // children, delete DOM nodes and QueryList, trigger hooks (onDestroy), destroy the renderer, - // detach projected nodes + const adjustedIdx = this._adjustIndex(index, -1); + removeView(this._lContainerNode, adjustedIdx); + this._viewRefs.splice(adjustedIdx, 1); } 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 implements viewEngine_TemplateRef { createEmbeddedView(context: T): viewEngine_EmbeddedViewRef { 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); } } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 0533fa31ce..5c1fdcad56 100644 --- a/packages/core/src/render3/instructions.ts +++ b/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); } } diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 942c1e0551..4d702169c8 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/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 */ diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index a22c3cc088..257c6e97b8 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/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 - return (node as LElementNode).native; + // Execute the action + 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 nextNode: LNode|null = null; - const renderer = container.view.renderer; - if (node.tNode.type === TNodeType.Element) { - if (insertMode) { - 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; - } - } + let node: LNode|null = getChildLNode(rootNode); + const renderer = container.view.renderer; + walkLNodeTree( + node, rootNode, insertMode ? WalkLNodeTreeAction.Insert : WalkLNodeTreeAction.Detach, + renderer, parentNode, beforeNode); } } @@ -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 - * listeners, calling onDestroys). + * removes the view's elements from the DOM. * - * @param container The container from which to remove a view - * @param removeIndex The index of the view to remove - * @returns The removed view + * @param container The container from which to detach a view + * @param removeIndex The index of the view to detach + * @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. diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 21a69f8ff3..7e3f029848 100644 --- a/packages/core/src/render3/query.ts +++ b/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( const queryList = new QueryList(); const queries = getCurrentQueries(LQueries_); queries.track(queryList, predicate, descend, read); - + getCleanup().push(queryList.destroy, queryList); if (memoryIndex != null) { store(memoryIndex, queryList); } diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 44b7682927..4d912ffc55 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/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 implements viewEngine_EmbeddedViewRef { 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 implements viewEngine_EmbeddedViewRef { this.context = context; } - destroy(): void { notImplemented(); } - destroyed: boolean; - onDestroy(callback: Function) { notImplemented(); } + get destroyed(): boolean { + return (this._view.flags & LViewFlags.Destroyed) === LViewFlags.Destroyed; + } + + 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 extends ViewRef { * @internal */ _lViewNode: LViewNode; + private _viewContainerRef: viewEngine_ViewContainerRef|null = null; constructor(viewNode: LViewNode, template: ComponentTemplate, context: T) { super(viewNode.data, context); this._lViewNode = viewNode; } -} -/** - * Creates a ViewRef bundled with destroy functionality. - * - * @param context The context for this view - * @returns The ViewRef - */ -export function createViewRef(view: LView | null, context: T): ViewRef { - // TODO: add detectChanges back in when implementing ChangeDetectorRef.detectChanges - return addDestroyable(new ViewRef(view !, context)); -} + destroy(): void { + if (this._viewContainerRef && + (this._view.flags & LViewFlags.Attached) === LViewFlags.Attached) { + this._viewContainerRef.detach(this._viewContainerRef.indexOf(this)); + this._viewContainerRef = null; + } + super.destroy(); + } -/** Interface for destroy logic. Implemented by addDestroyable. */ -export interface DestroyRef { - /** 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(obj: any): T&DestroyRef { - 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; + attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) { this._viewContainerRef = vcRef; } } diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 6571af19de..0d19433be3 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/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" }, diff --git a/packages/core/test/render3/imported_renderer2.ts b/packages/core/test/render3/imported_renderer2.ts index 30d25b5789..49ec367b85 100644 --- a/packages/core/test/render3/imported_renderer2.ts +++ b/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 { diff --git a/packages/core/test/render3/query_list_spec.ts b/packages/core/test/render3/query_list_spec.ts index 648d27eadc..59e3973047 100644 --- a/packages/core/test/render3/query_list_spec.ts +++ b/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(); + }); + }); }); diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 133c24fdb1..7382fabfa3 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/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; + + 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>(0)) && + (ctx.query = queryInstance = tmp as QueryList); + } + }); + + 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) { + * + * %} + */ + let condition = true; + const t = new TemplateFixture(createTemplate, updateTemplate, [SimpleComponentWithQuery]); + expect(t.html).toEqual('
'); + expect((queryInstance !.changes as EventEmitter).closed).toBeFalsy(); + + condition = false; + t.update(); + expect(t.html).toEqual(''); + expect((queryInstance !.changes as EventEmitter).closed).toBeTruthy(); + }); + }); }); diff --git a/packages/core/test/render3/renderer_factory_spec.ts b/packages/core/test/render3/renderer_factory_spec.ts index c2d8200a15..3cf26ecaf3 100644 --- a/packages/core/test/render3/renderer_factory_spec.ts +++ b/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; diff --git a/packages/core/test/render3/view_container_ref_spec.ts b/packages/core/test/render3/view_container_ref_spec.ts index 8dc809fa6e..94ec687f7b 100644 --- a/packages/core/test/render3/view_container_ref_spec.ts +++ b/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) { - directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, {name: s}, index); + function createView(s: string, index?: number): EmbeddedViewRef { + 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]); - createView('A'); + const fixture = new TemplateFixture( + 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('

ABCDE'); @@ -433,29 +438,97 @@ describe('ViewContainerRef', () => { directiveInstance !.vcref.detach(3); fixture.update(); expect(fixture.html).toEqual('

ABCE'); + expect(viewD.destroyed).toBeFalsy(); directiveInstance !.vcref.detach(0); fixture.update(); expect(fixture.html).toEqual('

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('

ABCDE'); directiveInstance !.vcref.detach(); fixture.update(); expect(fixture.html).toEqual('

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('

ABCDE'); + + directiveInstance !.vcref.remove(3); + fixture.update(); + expect(fixture.html).toEqual('

ABCE'); + expect(viewD.destroyed).toBeTruthy(); + + directiveInstance !.vcref.remove(0); + fixture.update(); + expect(fixture.html).toEqual('

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('

ABCDE'); + + directiveInstance !.vcref.remove(); + fixture.update(); + expect(fixture.html).toEqual('

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' + ]); }); }); });