From b26a90567c4e95e833e8ee00882fc28910def20e Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 8 Mar 2018 16:55:47 -0800 Subject: [PATCH] feat(ivy): support attaching and detaching views from change detection (#22670) PR Close #22670 --- packages/core/src/render3/component.ts | 9 +- packages/core/src/render3/di.ts | 9 +- packages/core/src/render3/instructions.ts | 11 +- packages/core/src/render3/interfaces/view.ts | 9 +- packages/core/src/render3/view_ref.ts | 29 ++- .../test/render3/change_detection_spec.ts | 199 +++++++++++++++++- 6 files changed, 237 insertions(+), 29 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 0fbaac464b..96010c8d3d 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -16,7 +16,7 @@ import {queueLifecycleHooks} from './hooks'; import {CLEAN_PROMISE, _getComponentHostLElementNode, createLView, createTView, directiveCreate, enterView, getDirectiveInstance, getRootView, hostElement, initChangeDetectorIfExisting, leaveView, locateHostElement, scheduleTick, tick} from './instructions'; import {ComponentDef, ComponentType} from './interfaces/definition'; import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; -import {LViewFlags, RootContext} from './interfaces/view'; +import {LView, LViewFlags, RootContext} from './interfaces/view'; import {stringify} from './util'; import {createViewRef} from './view_ref'; @@ -74,13 +74,14 @@ export interface CreateComponentOptions { export function createComponentRef( componentType: ComponentType, opts: CreateComponentOptions): viewEngine_ComponentRef { const component = renderComponent(componentType, opts); - const hostView = createViewRef(component); + const hostView = _getComponentHostLElementNode(component).data as LView; + const hostViewRef = createViewRef(hostView, component); return { location: {nativeElement: getHostElement(component)}, injector: opts.injector || NULL_INJECTOR, instance: component, - hostView: hostView, - changeDetectorRef: hostView, + hostView: hostViewRef, + changeDetectorRef: hostViewRef, componentType: componentType, // TODO: implement destroy and onDestroy destroy: () => {}, diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index abf5394acb..a53438d1c7 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -25,6 +25,7 @@ import {LInjector} from './interfaces/injector'; import {LContainerNode, LElementNode, LNode, LNodeFlags, LViewNode} from './interfaces/node'; import {QueryReadType} from './interfaces/query'; import {Renderer3} from './interfaces/renderer'; +import {LView} from './interfaces/view'; import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; import {insertView} from './node_manipulation'; import {notImplemented, stringify} from './util'; @@ -301,7 +302,7 @@ export function getOrCreateChangeDetectorRef( return di.changeDetectorRef = getOrCreateHostChangeDetector(currentNode.view.node); } else if ((currentNode.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element) { // if it's an element node with data, it's a component and context will be set later - return di.changeDetectorRef = createViewRef(context); + return di.changeDetectorRef = createViewRef(currentNode.data as LView, context); } return null !; } @@ -313,8 +314,10 @@ function getOrCreateHostChangeDetector(currentNode: LViewNode | LElementNode): const hostInjector = hostNode.nodeInjector; const existingRef = hostInjector && hostInjector.changeDetectorRef; - return existingRef ? existingRef : - createViewRef(hostNode.view.data[hostNode.flags >> LNodeFlags.INDX_SHIFT]); + return existingRef ? + existingRef : + createViewRef( + hostNode.data as LView, hostNode.view.data[hostNode.flags >> LNodeFlags.INDX_SHIFT]); } /** diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 78ed8128f0..10ef97b088 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -210,7 +210,7 @@ export function createLView( const newView = { parent: currentView, id: viewId, // -1 for component views - flags: flags | LViewFlags.CreationMode, + flags: flags | LViewFlags.CreationMode | LViewFlags.Attached, node: null !, // until we initialize it in createNode. data: [], tView: tView, @@ -1281,14 +1281,19 @@ export function directiveRefresh(directiveIndex: number, elementIndex: number assertNotNull(element.data, `Component's host node should have an LView attached.`); const hostView = element.data !; - // Only CheckAlways components or dirty OnPush components should be checked - if (hostView.flags & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { + // Only attached CheckAlways components or attached, dirty OnPush components should be checked + if (viewAttached(hostView) && hostView.flags & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { ngDevMode && assertDataInRange(directiveIndex); detectChangesInternal(hostView, element, getDirectiveInstance(data[directiveIndex])); } } } +/** Returns a boolean for whether the view is attached */ +function viewAttached(view: LView): boolean { + return (view.flags & LViewFlags.Attached) === LViewFlags.Attached; +} + /** * Instruction to distribute projectable nodes among occurrences in a given template. * It takes all the selectors from the entire component's template and decides where diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 0bd4d67992..ac1ff88608 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -183,13 +183,16 @@ export const enum LViewFlags { * back into the parent view, `data` will be defined and `creationMode` will be * improperly reported as false. */ - CreationMode = 0b001, + CreationMode = 0b0001, /** Whether this view has default change detection strategy (checks always) or onPush */ - CheckAlways = 0b010, + CheckAlways = 0b0010, /** Whether or not this view is currently dirty (needing check) */ - Dirty = 0b100 + Dirty = 0b0100, + + /** Whether or not this view is currently attached to change detection tree. */ + Attached = 0b1000, } /** Interface necessary to work with view tree traversal */ diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index 92be0d9124..422299fefa 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -7,16 +7,18 @@ */ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef} from '../linker/view_ref'; + import {detectChanges} from './instructions'; import {ComponentTemplate} from './interfaces/definition'; import {LViewNode} from './interfaces/node'; +import {LView, LViewFlags} from './interfaces/view'; import {notImplemented} from './util'; export class ViewRef implements viewEngine_EmbeddedViewRef { context: T; rootNodes: any[]; - constructor(context: T|null) { this.context = context !; } + constructor(private _view: LView, context: T|null, ) { this.context = context !; } /** @internal */ _setComponentContext(context: T) { this.context = context; } @@ -25,12 +27,27 @@ export class ViewRef implements viewEngine_EmbeddedViewRef { destroyed: boolean; onDestroy(callback: Function) { notImplemented(); } markForCheck(): void { notImplemented(); } - detach(): void { notImplemented(); } + + /** + * Detaches a view from the change detection tree. + * + * Detached views will not be checked during change detection runs, even if the view + * is dirty. This can be used in combination with detectChanges to implement local + * change detection checks. + */ + detach(): void { this._view.flags &= ~LViewFlags.Attached; } + + /** + * Re-attaches a view to the change detection tree. + * + * This can be used to re-attach views that were previously detached from the tree + * using detach(). Views are attached to the tree by default. + */ + reattach(): void { this._view.flags |= LViewFlags.Attached; } detectChanges(): void { detectChanges(this.context); } checkNoChanges(): void { notImplemented(); } - reattach(): void { notImplemented(); } } @@ -41,7 +58,7 @@ export class EmbeddedViewRef extends ViewRef { _lViewNode: LViewNode; constructor(viewNode: LViewNode, template: ComponentTemplate, context: T) { - super(context); + super(viewNode.data, context); this._lViewNode = viewNode; } } @@ -52,9 +69,9 @@ export class EmbeddedViewRef extends ViewRef { * @param context The context for this view * @returns The ViewRef */ -export function createViewRef(context: T): ViewRef { +export function createViewRef(view: LView, context: T): ViewRef { // TODO: add detectChanges back in when implementing ChangeDetectorRef.detectChanges - return addDestroyable(new ViewRef(context)); + return addDestroyable(new ViewRef(view, context)); } /** Interface for destroy logic. Implemented by addDestroyable. */ diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts index 259ccf6603..4dd97fbaa1 100644 --- a/packages/core/test/render3/change_detection_spec.ts +++ b/packages/core/test/render3/change_detection_spec.ts @@ -11,7 +11,7 @@ import {withBody} from '@angular/core/testing'; import {ChangeDetectionStrategy, ChangeDetectorRef, DoCheck, EmbeddedViewRef, TemplateRef, ViewContainerRef} from '../../src/core'; import {getRenderedText, whenRendered} from '../../src/render3/component'; import {NgOnChangesFeature, PublicFeature, defineComponent, defineDirective, injectChangeDetectorRef, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; -import {bind, container, containerRefreshEnd, containerRefreshStart, detectChanges, directiveRefresh, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, listener, markDirty, text, textBinding} from '../../src/render3/instructions'; +import {bind, container, containerRefreshEnd, containerRefreshStart, detectChanges, directiveRefresh, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, listener, markDirty, text, textBinding, tick} from '../../src/render3/instructions'; import {containerEl, renderComponent, requestAnimationFrame, toHtml} from './render_util'; @@ -146,21 +146,21 @@ describe('change detection', () => { it('should call doCheck even when OnPush components are not dirty', () => { const myApp = renderComponent(MyApp); - detectChanges(myApp); + tick(myApp); expect(comp.doCheckCount).toEqual(2); - detectChanges(myApp); + tick(myApp); expect(comp.doCheckCount).toEqual(3); }); it('should skip OnPush components in update mode when they are not dirty', () => { const myApp = renderComponent(MyApp); - detectChanges(myApp); + tick(myApp); // doCheckCount is 2, but 1 should be rendered since it has not been marked dirty. expect(getRenderedText(myApp)).toEqual('1 - Nancy'); - detectChanges(myApp); + tick(myApp); // doCheckCount is 3, but 1 should be rendered since it has not been marked dirty. expect(getRenderedText(myApp)).toEqual('1 - Nancy'); }); @@ -169,14 +169,14 @@ describe('change detection', () => { const myApp = renderComponent(MyApp); myApp.name = 'Bess'; - detectChanges(myApp); + tick(myApp); expect(getRenderedText(myApp)).toEqual('2 - Bess'); myApp.name = 'George'; - detectChanges(myApp); + tick(myApp); expect(getRenderedText(myApp)).toEqual('3 - George'); - detectChanges(myApp); + tick(myApp); expect(getRenderedText(myApp)).toEqual('3 - George'); }); @@ -189,7 +189,7 @@ describe('change detection', () => { requestAnimationFrame.flush(); expect(getRenderedText(myApp)).toEqual('2 - Nancy'); - detectChanges(myApp); + tick(myApp); expect(getRenderedText(myApp)).toEqual('2 - Nancy'); }); @@ -275,7 +275,7 @@ describe('change detection', () => { expect(comp !.doCheckCount).toEqual(1); expect(getRenderedText(myButtonApp)).toEqual('1 - 1 - Nancy'); - detectChanges(myButtonApp); + tick(myButtonApp); expect(parent !.doCheckCount).toEqual(2); // parent isn't checked, so child doCheck won't run expect(comp !.doCheckCount).toEqual(1); @@ -577,6 +577,185 @@ describe('change detection', () => { }); + describe('attach/detach', () => { + let comp: DetachedComp; + + class MyApp { + constructor(public cdr: ChangeDetectorRef) {} + + static ngComponentDef = defineComponent({ + type: MyApp, + tag: 'my-app', + factory: () => new MyApp(injectChangeDetectorRef()), + /** */ + template: (ctx: MyApp, cm: boolean) => { + if (cm) { + elementStart(0, DetachedComp); + elementEnd(); + } + DetachedComp.ngComponentDef.h(1, 0); + directiveRefresh(1, 0); + } + }); + } + + class DetachedComp { + value = 'one'; + doCheckCount = 0; + + constructor(public cdr: ChangeDetectorRef) {} + + ngDoCheck() { this.doCheckCount++; } + + static ngComponentDef = defineComponent({ + type: DetachedComp, + tag: 'detached-comp', + factory: () => comp = new DetachedComp(injectChangeDetectorRef()), + /** {{ value }} */ + template: (ctx: DetachedComp, cm: boolean) => { + if (cm) { + text(0); + } + textBinding(0, bind(ctx.value)); + } + }); + } + + it('should not check detached components', () => { + const app = renderComponent(MyApp); + expect(getRenderedText(app)).toEqual('one'); + + comp.cdr.detach(); + + comp.value = 'two'; + tick(app); + expect(getRenderedText(app)).toEqual('one'); + }); + + it('should check re-attached components', () => { + const app = renderComponent(MyApp); + expect(getRenderedText(app)).toEqual('one'); + + comp.cdr.detach(); + comp.value = 'two'; + + comp.cdr.reattach(); + tick(app); + expect(getRenderedText(app)).toEqual('two'); + }); + + it('should call lifecycle hooks on detached components', () => { + const app = renderComponent(MyApp); + expect(comp.doCheckCount).toEqual(1); + + comp.cdr.detach(); + + tick(app); + expect(comp.doCheckCount).toEqual(2); + }); + + it('should check detached component when detectChanges is called', () => { + const app = renderComponent(MyApp); + expect(getRenderedText(app)).toEqual('one'); + + comp.cdr.detach(); + + comp.value = 'two'; + detectChanges(comp); + expect(getRenderedText(app)).toEqual('two'); + }); + + it('should not check detached component when markDirty is called', () => { + const app = renderComponent(MyApp); + expect(getRenderedText(app)).toEqual('one'); + + comp.cdr.detach(); + + comp.value = 'two'; + markDirty(comp); + requestAnimationFrame.flush(); + + expect(getRenderedText(app)).toEqual('one'); + }); + + it('should detach any child components when parent is detached', () => { + const app = renderComponent(MyApp); + expect(getRenderedText(app)).toEqual('one'); + + app.cdr.detach(); + + comp.value = 'two'; + tick(app); + expect(getRenderedText(app)).toEqual('one'); + + app.cdr.reattach(); + + tick(app); + expect(getRenderedText(app)).toEqual('two'); + }); + + it('should detach OnPush components properly', () => { + let onPushComp: OnPushComp; + + class OnPushComp { + /** @Input() */ + value: string; + + constructor(public cdr: ChangeDetectorRef) {} + + static ngComponentDef = defineComponent({ + type: OnPushComp, + tag: 'on-push-comp', + factory: () => onPushComp = new OnPushComp(injectChangeDetectorRef()), + /** {{ value }} */ + template: (ctx: OnPushComp, cm: boolean) => { + if (cm) { + text(0); + } + textBinding(0, bind(ctx.value)); + }, + changeDetection: ChangeDetectionStrategy.OnPush, + inputs: {value: 'value'} + }); + } + + class OnPushApp { + value = 'one'; + + static ngComponentDef = defineComponent({ + type: OnPushApp, + tag: 'on-push-app', + factory: () => new OnPushApp(), + /** */ + template: (ctx: OnPushApp, cm: boolean) => { + if (cm) { + elementStart(0, OnPushComp); + elementEnd(); + } + elementProperty(0, 'value', bind(ctx.value)); + OnPushComp.ngComponentDef.h(1, 0); + directiveRefresh(1, 0); + } + }); + } + + const app = renderComponent(OnPushApp); + expect(getRenderedText(app)).toEqual('one'); + + onPushComp !.cdr.detach(); + + app.value = 'two'; + tick(app); + expect(getRenderedText(app)).toEqual('one'); + + onPushComp !.cdr.reattach(); + + tick(app); + expect(getRenderedText(app)).toEqual('two'); + }); + + }); + }); });