From 951b96534a4e803a879597a8084ca53ce0164b0b Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 10 Nov 2017 10:55:58 +0000 Subject: [PATCH 1/2] Removes Evented and Destroyable from WidgetBase --- src/NodeHandler.ts | 2 +- src/WidgetBase.ts | 60 +++------- src/decorators/inject.ts | 2 +- src/interfaces.d.ts | 2 +- src/mixins/I18n.ts | 7 +- src/mixins/Projector.ts | 70 ++++++----- src/util/DomWrapper.ts | 2 +- src/vdom.ts | 42 ++----- tests/unit/NodeHandler.ts | 4 +- tests/unit/WidgetBase.ts | 34 +----- tests/unit/decorators/inject.ts | 17 --- tests/unit/meta/Drag.ts | 11 -- tests/unit/meta/Matches.ts | 4 - tests/unit/mixins/I18n.ts | 1 - tests/unit/mixins/Projector.ts | 20 ---- tests/unit/util/DomWrapper.ts | 2 +- tests/unit/vdom.ts | 205 ++++++++++---------------------- 17 files changed, 153 insertions(+), 332 deletions(-) diff --git a/src/NodeHandler.ts b/src/NodeHandler.ts index b963430a..36fa5f3b 100644 --- a/src/NodeHandler.ts +++ b/src/NodeHandler.ts @@ -29,7 +29,7 @@ export class NodeHandler extends Evented implements NodeHandlerInterface { this.emit({ type: key }); } - public addRoot(element: HTMLElement, key?: string): void { + public addRoot(): void { this.emit({ type: NodeEventType.Widget }); } diff --git a/src/WidgetBase.ts b/src/WidgetBase.ts index b1d8a670..725d3bfb 100644 --- a/src/WidgetBase.ts +++ b/src/WidgetBase.ts @@ -1,5 +1,4 @@ import { EventTypedObject } from '@dojo/interfaces/core'; -import { Evented } from '@dojo/core/Evented'; import Map from '@dojo/shim/Map'; import WeakMap from '@dojo/shim/WeakMap'; import { v } from './d'; @@ -53,7 +52,7 @@ const boundAuto = auto.bind(null); /** * Main widget base for all widgets to extend */ -export class WidgetBase

extends Evented implements WidgetBaseInterface { +export class WidgetBase

implements WidgetBaseInterface { /** * static identifier @@ -87,11 +86,6 @@ export class WidgetBase

extends E private _cachedDNode: DNode | DNode[]; - /** - * map of specific property diff functions - */ - private _diffPropertyFunctionMap: Map; - /** * map of decorators that are applied to this widget */ @@ -112,42 +106,27 @@ export class WidgetBase

extends E private _boundInvalidate: () => void; - private _nodeHandler: NodeHandler; + public readonly nodeHandler: NodeHandler = new NodeHandler(); + + private _parentInvalidate: Function; /** * @constructor */ - constructor() { - super({}); + constructor(invalidate?: Function) { + if (invalidate) { + this._parentInvalidate = invalidate; + } this._children = []; this._decoratorCache = new Map(); this._properties =

{}; - this._diffPropertyFunctionMap = new Map(); + this.nodeHandler = new NodeHandler(); this._bindFunctionPropertyMap = new WeakMap<(...args: any[]) => any, { boundFunc: (...args: any[]) => any, scope: any }>(); this._registry = new RegistryHandler(); - this._nodeHandler = new NodeHandler(); - this.own(this._registry); - this.own(this._nodeHandler); this._boundRenderFunc = this.render.bind(this); this._boundInvalidate = this.invalidate.bind(this); - this.own(this.on({ - 'element-created': ({ key, element }: WidgetAndElementEvent) => { - this._nodeHandler.add(element, `${key}`); - this.onElementCreated(element, key); - }, - 'element-updated': ({ key, element }: WidgetAndElementEvent) => { - this._nodeHandler.add(element, `${key}`); - this.onElementUpdated(element, key); - }, - 'widget-created': ({ element }: WidgetAndElementEvent) => { - this._nodeHandler.addRoot(element, undefined); - }, - 'widget-updated': ({ element }: WidgetAndElementEvent) => { - this._nodeHandler.addRoot(element, undefined); - } - })); - this.own(this._registry.on('invalidate', this._boundInvalidate)); + this._registry.on('invalidate', this._boundInvalidate); } protected meta(MetaType: WidgetMetaConstructor): T { @@ -155,11 +134,11 @@ export class WidgetBase

extends E if (!cached) { cached = new MetaType({ invalidate: this._boundInvalidate, - nodeHandler: this._nodeHandler, + nodeHandler: this.nodeHandler, bind: this }); this._metaMap.set(MetaType, cached); - this.own(cached); + cached; } return cached as T; @@ -171,7 +150,7 @@ export class WidgetBase

extends E * @param element The dom node represented by the vdom node. * @param key The vdom node's key. */ - protected onElementCreated(element: Element, key: string): void { + public onElementCreated(element: Element, key: string): void { // Do nothing by default. } @@ -181,7 +160,7 @@ export class WidgetBase

extends E * @param element The dom node represented by the vdom node. * @param key The vdom node's key. */ - protected onElementUpdated(element: Element, key: string): void { + public onElementUpdated(element: Element, key: string): void { // Do nothing by default. } @@ -286,7 +265,7 @@ export class WidgetBase

extends E const render = this._runBeforeRenders(); let dNode = render(); this._cachedDNode = this.runAfterRenders(dNode); - this._nodeHandler.clear(); + this.nodeHandler.clear(); } this._renderState = WidgetRenderState.IDLE; return this._cachedDNode; @@ -295,10 +274,9 @@ export class WidgetBase

extends E public invalidate(): void { if (this._renderState === WidgetRenderState.IDLE) { this._dirty = true; - this.emit({ - type: 'invalidated', - target: this - }); + if (this._parentInvalidate) { + this._parentInvalidate(); + } } else if (this._renderState === WidgetRenderState.PROPERTIES) { this._dirty = true; @@ -476,7 +454,7 @@ export class WidgetBase

extends E }, dNode); } - this._metaMap.forEach(meta => { + this._metaMap.forEach((meta) => { meta.afterRender(); }); diff --git a/src/decorators/inject.ts b/src/decorators/inject.ts index 39207767..1e9093e2 100644 --- a/src/decorators/inject.ts +++ b/src/decorators/inject.ts @@ -53,7 +53,7 @@ export function inject({ name, getProperties }: InjectConfig) { } if (registeredInjectors.indexOf(injector) === -1) { injector.on('invalidate', () => { - this.emit({ type: 'invalidated', target: this }); + this.invalidate(); }); registeredInjectors.push(injector); } diff --git a/src/interfaces.d.ts b/src/interfaces.d.ts index e28feac4..a64b179b 100644 --- a/src/interfaces.d.ts +++ b/src/interfaces.d.ts @@ -397,7 +397,7 @@ export interface DefaultWidgetBaseInterface extends WidgetBaseInterface extends Evented { + C extends DNode = DNode> { /** * Widget properties diff --git a/src/mixins/I18n.ts b/src/mixins/I18n.ts index 878c6289..9e85cadf 100644 --- a/src/mixins/I18n.ts +++ b/src/mixins/I18n.ts @@ -79,18 +79,13 @@ export function I18nMixin>>(Base: T): T & constructor(...args: any[]) { super(...args); - const subscription = observeLocale({ + observeLocale({ next: () => { if (!this.properties.locale) { this.invalidate(); } } }); - this.own({ - destroy() { - subscription.unsubscribe(); - } - }); } public localizeBundle(bundle: Bundle): LocalizedMessages { diff --git a/src/mixins/Projector.ts b/src/mixins/Projector.ts index c2c517e4..15af0e13 100644 --- a/src/mixins/Projector.ts +++ b/src/mixins/Projector.ts @@ -2,7 +2,6 @@ import { assign } from '@dojo/core/lang'; import global from '@dojo/shim/global'; import { createHandle } from '@dojo/core/lang'; import { Handle } from '@dojo/interfaces/core'; -import { Evented } from '@dojo/core/Evented'; import 'pepjs'; import cssTransitions from '../animations/cssTransitions'; import { Constructor, DNode, Projection, ProjectionOptions } from './../interfaces'; @@ -87,7 +86,7 @@ export interface ProjectorMixin

{ * The `Projector` creates a `DocumentFragment` which replaces any other `root` that has been set. * @param doc The `Document` to use, which defaults to the global `document`. */ - sandbox(doc?: Document): Handle; + sandbox(doc?: Document): void; /** * Schedule a render. @@ -133,6 +132,11 @@ export interface ProjectorMixin

{ * Exposes invalidate for projector instances */ invalidate(): void; + + /** + * Runs registered destroy handles + */ + destroy(): void; } export function ProjectorMixin>>(Base: T): T & Constructor> { @@ -154,28 +158,27 @@ export function ProjectorMixin>>(Base: T) private _projectorProperties: this['properties'] = {} as this['properties']; private _rootTagName: string; private _attachType: AttachType; + private _handles: Function[] = []; constructor(...args: any[]) { super(...args); - const nodeEvent = new Evented(); - this.own(nodeEvent); - this._projectionOptions = { transitions: cssTransitions }; this._boundDoRender = this._doRender.bind(this); this._boundRender = this.__render__.bind(this); - this.own(this.on('invalidated', () => { - this.scheduleRender(); - })); - this.root = document.body; this.projectorState = ProjectorAttachState.Detached; } - public append(root?: Element) { + public invalidate() { + super.invalidate(); + this.scheduleRender(); + } + + public append(root?: Element): Handle { const options = { type: AttachType.Append, root @@ -184,7 +187,7 @@ export function ProjectorMixin>>(Base: T) return this._attach(options); } - public merge(root?: Element) { + public merge(root?: Element): Handle { const options = { type: AttachType.Merge, root @@ -193,7 +196,7 @@ export function ProjectorMixin>>(Base: T) return this._attach(options); } - public replace(root?: Element) { + public replace(root?: Element): Handle { const options = { type: AttachType.Replace, root @@ -252,7 +255,7 @@ export function ProjectorMixin>>(Base: T) this._async = async; } - public sandbox(doc: Document = document): Handle { + public sandbox(doc: Document = document): void { if (this.projectorState === ProjectorAttachState.Attached) { throw new Error('Projector already attached, cannot create sandbox'); } @@ -260,10 +263,11 @@ export function ProjectorMixin>>(Base: T) const previousRoot = this.root; /* free up the document fragment for GC */ - this.own(createHandle(() => { + this.own(() => { this._root = previousRoot; - })); - return this._attach({ + }); + + this._attach({ /* DocumentFragment is not assignable to Element, but provides everything needed to work */ root: doc.createDocumentFragment() as any, type: AttachType.Append @@ -290,9 +294,6 @@ export function ProjectorMixin>>(Base: T) if (this._projectorProperties.registry) { this._projectorProperties.registry.destroy(); } - if (properties.registry) { - this.own(properties.registry); - } } this._projectorProperties = assign({}, properties); super.__setCoreProperties__({ bind: this, baseRegistry: properties.registry }); @@ -324,6 +325,19 @@ export function ProjectorMixin>>(Base: T) } } + private own(handle: Function): void { + this._handles.push(handle); + } + + public destroy() { + while (this._handles.length > 0) { + const handle = this._handles.pop(); + if (handle) { + handle(); + } + } + } + private _attach({ type, root }: AttachOptions): Handle { this._attachType = type; if (root) { @@ -336,16 +350,16 @@ export function ProjectorMixin>>(Base: T) this.projectorState = ProjectorAttachState.Attached; - this._attachHandle = this.own({ - destroy: () => { - if (this.projectorState === ProjectorAttachState.Attached) { - this.pause(); - this._projection = undefined; - this.projectorState = ProjectorAttachState.Detached; - } - this._attachHandle = { destroy() { } }; + const handle = () => { + if (this.projectorState === ProjectorAttachState.Attached) { + this.pause(); + this._projection = undefined; + this.projectorState = ProjectorAttachState.Detached; } - }); + }; + + this.own(handle); + this._attachHandle = createHandle(handle); this._projectionOptions = { ...this._projectionOptions, ...{ sync: !this._async } }; diff --git a/src/util/DomWrapper.ts b/src/util/DomWrapper.ts index c955317c..a473adae 100644 --- a/src/util/DomWrapper.ts +++ b/src/util/DomWrapper.ts @@ -20,7 +20,7 @@ export function DomWrapper(domNode: Element, options: DomWrapperOptions = {}): D return hNode; } - protected onElementCreated(element: Element, key: string) { + public onElementCreated(element: Element, key: string) { options.onAttached && options.onAttached(); } diff --git a/src/vdom.ts b/src/vdom.ts index e71416f0..a672f91b 100644 --- a/src/vdom.ts +++ b/src/vdom.ts @@ -453,8 +453,7 @@ function nodeAdded(dnode: InternalDNode, transitions: TransitionStrategy) { function nodeToRemove(dnode: InternalDNode, transitions: TransitionStrategy, projectionOptions: ProjectionOptions) { if (isWNode(dnode)) { - projectionOptions.afterRenderCallbacks.push(dnode.instance.destroy.bind(dnode.instance)); - const rendered = dnode.rendered || emptyArray ; + const rendered = dnode.rendered || emptyArray; for (let i = 0; i < rendered.length; i++) { const child = rendered[i]; if (isHNode(child)) { @@ -640,8 +639,9 @@ function initPropertiesAndChildren( } setProperties(domNode, dnode.properties, projectionOptions); if (dnode.properties.key !== null && dnode.properties.key !== undefined) { + parentInstance.nodeHandler.add(domNode as HTMLElement, `${dnode.properties.key}`); projectionOptions.afterRenderCallbacks.push(() => { - parentInstance.emit({ type: 'element-created', key: dnode.properties.key, element: domNode }); + parentInstance.onElementCreated(domNode as HTMLElement, dnode.properties.key as any); }); } dnode.inserted = true; @@ -665,10 +665,7 @@ function createDom( } widgetConstructor = item; } - const instance = new widgetConstructor(); - instance.own(instance.on('invalidated', () => { - parentInstance.invalidate(); - })); + const instance = new widgetConstructor(parentInstance.invalidate.bind(parentInstance)); dnode.instance = instance; instance.__setCoreProperties__(dnode.coreProperties); instance.__setChildren__(dnode.children); @@ -679,9 +676,7 @@ function createDom( dnode.rendered = filteredRendered; addChildren(parentNode, filteredRendered, projectionOptions, instance as WidgetBase, insertBefore, childNodes); } - projectionOptions.afterRenderCallbacks.push(() => { - parentInstance.emit({ type: 'widget-created' }); - }); + instance.nodeHandler.addRoot(); } else { if (projectionOptions.merge && projectionOptions.mergeElement !== undefined) { @@ -745,9 +740,7 @@ function updateDom(previous: any, dnode: InternalDNode, projectionOptions: Proje dnode.rendered = filterAndDecorateChildren(rendered, instance); if (hasRenderChanged(previousRendered, rendered)) { updateChildren(parentNode, previousRendered, dnode.rendered, instance, projectionOptions); - projectionOptions.afterRenderCallbacks.push(() => { - parentInstance.emit({ type: 'widget-updated' }); - }); + instance.nodeHandler.addRoot(); } } else { @@ -788,8 +781,9 @@ function updateDom(previous: any, dnode: InternalDNode, projectionOptions: Proje updated = updateProperties(domNode, previous.properties, dnode.properties, projectionOptions) || updated; if (dnode.properties.key !== null && dnode.properties.key !== undefined) { + parentInstance.nodeHandler.add(domNode, `${dnode.properties.key}`); projectionOptions.afterRenderCallbacks.push(() => { - parentInstance.emit({ type: 'element-updated', key: dnode.properties.key, element: domNode }); + parentInstance.onElementUpdated(domNode as HTMLElement, dnode.properties.key as any); }); } } @@ -871,9 +865,7 @@ function createProjection(dnode: InternalDNode | InternalDNode[], parentInstance updatedDNode = filterAndDecorateChildren(updatedDNode, parentInstance); updateChildren(domNode, projectionDNode, updatedDNode as InternalDNode[], parentInstance, projectionOptions); - projectionOptions.afterRenderCallbacks.push(() => { - parentInstance.emit({ type: 'widget-created' }); - }); + parentInstance.nodeHandler.addRoot(); runDeferredRenderCallbacks(projectionOptions); runAfterRenderCallbacks(projectionOptions); projectionDNode = updatedDNode as InternalDNode[]; @@ -889,9 +881,7 @@ export const dom = { finalProjectorOptions.rootNode = rootNode; const decoratedNode = filterAndDecorateChildren(dNode, instance); addChildren(rootNode, decoratedNode, finalProjectorOptions, instance, undefined); - finalProjectorOptions.afterRenderCallbacks.push(() => { - instance.emit({ type: 'widget-created' }); - }); + instance.nodeHandler.addRoot(); runDeferredRenderCallbacks(finalProjectorOptions); runAfterRenderCallbacks(finalProjectorOptions); return createProjection(decoratedNode, instance, finalProjectorOptions); @@ -901,9 +891,7 @@ export const dom = { finalProjectorOptions.rootNode = parentNode; const decoratedNode = filterAndDecorateChildren(dNode, instance); addChildren(parentNode, decoratedNode, finalProjectorOptions, instance, undefined); - finalProjectorOptions.afterRenderCallbacks.push(() => { - instance.emit({ type: 'widget-created' }); - }); + instance.nodeHandler.addRoot(); runDeferredRenderCallbacks(finalProjectorOptions); runAfterRenderCallbacks(finalProjectorOptions); return createProjection(decoratedNode, instance, finalProjectorOptions); @@ -919,9 +907,7 @@ export const dom = { const decoratedNode = filterAndDecorateChildren(dNode, instance)[0] as InternalHNode; createDom(decoratedNode, finalProjectorOptions.rootNode, undefined, finalProjectorOptions, instance); - finalProjectorOptions.afterRenderCallbacks.push(() => { - instance.emit({ type: 'widget-created' }); - }); + instance.nodeHandler.addRoot(); runDeferredRenderCallbacks(finalProjectorOptions); runAfterRenderCallbacks(finalProjectorOptions); return createProjection(decoratedNode, instance, finalProjectorOptions); @@ -934,9 +920,7 @@ export const dom = { const decoratedNode = filterAndDecorateChildren(dNode, instance)[0] as InternalHNode; finalProjectorOptions.rootNode = element.parentNode! as Element; createDom(decoratedNode, element.parentNode!, element, finalProjectorOptions, instance); - finalProjectorOptions.afterRenderCallbacks.push(() => { - instance.emit({ type: 'widget-created' }); - }); + instance.nodeHandler.addRoot(); runDeferredRenderCallbacks(finalProjectorOptions); runAfterRenderCallbacks(finalProjectorOptions); element.parentNode!.removeChild(element); diff --git a/tests/unit/NodeHandler.ts b/tests/unit/NodeHandler.ts index 4c7bca27..356a31f6 100644 --- a/tests/unit/NodeHandler.ts +++ b/tests/unit/NodeHandler.ts @@ -53,13 +53,13 @@ registerSuite('NodeHandler', { assert.isTrue(projectorStub.notCalled); }, 'add root emits Widget'() { - nodeHandler.addRoot(element, 'foo'); + nodeHandler.addRoot(); assert.isTrue(widgetStub.calledOnce); assert.isTrue(projectorStub.notCalled); }, 'add root without a key emits Widget event only'() { - nodeHandler.addRoot(element); + nodeHandler.addRoot(); assert.isTrue(widgetStub.calledOnce); assert.isTrue(elementStub.notCalled); diff --git a/tests/unit/WidgetBase.ts b/tests/unit/WidgetBase.ts index 6b5ca46a..b8fdf9c0 100644 --- a/tests/unit/WidgetBase.ts +++ b/tests/unit/WidgetBase.ts @@ -277,21 +277,21 @@ describe('WidgetBase', () => { }); it('elements are added to node handler on create', () => { - const element = {}; + const element = {} as any; const key = '1'; const widget = new BaseTestWidget(); const meta = widget.meta(TestMeta); - widget.emit({ type: 'element-created', element, key }); + widget.nodeHandler.add(element, key); assert.isTrue(meta.has(key)); assert.strictEqual(meta.get(key), element); }); it('elements are added to node handler on update', () => { - const element = {}; + const element = {} as any; const key = '1'; const widget = new BaseTestWidget(); const meta = widget.meta(TestMeta); - widget.emit({ type: 'element-updated', element, key }); + widget.nodeHandler.add(element, key); assert.isTrue(meta.has(key)); assert.strictEqual(meta.get(key), element); }); @@ -300,7 +300,7 @@ describe('WidgetBase', () => { const widget = new BaseTestWidget(); const meta = widget.meta(TestMeta); - widget.emit({ type: 'widget-created' }); + widget.nodeHandler.addRoot(); assert.isTrue(meta.widgetEvent); }); @@ -308,33 +308,11 @@ describe('WidgetBase', () => { const widget = new BaseTestWidget(); const meta = widget.meta(TestMeta); - widget.emit({ type: 'widget-updated' }); + widget.nodeHandler.addRoot(); assert.isTrue(meta.widgetEvent); }); }); - describe('onElementCreated called on `element-created` event', () => { - class TestWidget extends BaseTestWidget { - onElementCreated(element: any, key: any) { - assert.strictEqual(element, 'element'); - assert.strictEqual(key, 'key'); - } - } - const widget = new TestWidget(); - widget.emit({ type: 'element-created', key: 'key', element: 'element' }); - }); - - describe('onElementUpdated called on `element-updated` event', () => { - class TestWidget extends BaseTestWidget { - onElementUpdated(element: any, key: any) { - assert.strictEqual(element, 'element'); - assert.strictEqual(key, 'key'); - } - } - const widget = new TestWidget(); - widget.emit({ type: 'element-updated', key: 'key', element: 'element' }); - }); - describe('decorators', () => { it('returns an empty array for decorators that do not exist', () => { diff --git a/tests/unit/decorators/inject.ts b/tests/unit/decorators/inject.ts index 4a2532e6..7502f36a 100644 --- a/tests/unit/decorators/inject.ts +++ b/tests/unit/decorators/inject.ts @@ -51,23 +51,6 @@ registerSuite('decorators/inject', { assert.strictEqual(widget.properties.foo, 'bar'); assert.strictEqual(widget.properties.bar, 'foo'); }, - 'payload are only attached once'() { - let invalidateCount = 0; - function getProperties(payload: any, properties: WidgetProperties): WidgetProperties { - return payload; - } - - @inject({ name: 'inject-one', getProperties }) - class TestWidget extends WidgetBase {} - const widget = new TestWidget(); - widget.__setCoreProperties__({ bind: widget, baseRegistry: registry }); - widget.__setProperties__({}); - widget.on('invalidated', () => { - invalidateCount++; - }); - injectorOne.set({}); - assert.strictEqual(invalidateCount, 1); - }, 'programmatic registration'() { function getPropertiesOne(payload: any, properties: WidgetProperties): WidgetProperties { return payload; diff --git a/tests/unit/meta/Drag.ts b/tests/unit/meta/Drag.ts index d289ab89..180642fb 100644 --- a/tests/unit/meta/Drag.ts +++ b/tests/unit/meta/Drag.ts @@ -54,7 +54,6 @@ registerSuite('support/meta/Drag', { assert.strictEqual((div.firstChild as HTMLElement).getAttribute('touch-action'), 'none', 'Should have set touch-action attribute to none'); assert.strictEqual((div.firstChild as HTMLElement).style.touchAction, 'none', 'Should have set touch-action type to none'); - widget.destroy(); document.body.removeChild(div); }, @@ -83,7 +82,6 @@ registerSuite('support/meta/Drag', { assert.deepEqual(dragResults, [ emptyResults, emptyResults ], 'should have been called twice, both empty results'); - widget.destroy(); document.body.removeChild(div); }, @@ -182,7 +180,6 @@ registerSuite('support/meta/Drag', { } ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); }, @@ -309,7 +306,6 @@ registerSuite('support/meta/Drag', { } ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); }, @@ -430,7 +426,6 @@ registerSuite('support/meta/Drag', { } ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); }, @@ -498,7 +493,6 @@ registerSuite('support/meta/Drag', { emptyResults ], 'the widget does not invalidate on ignored events'); - widget.destroy(); document.body.removeChild(div); }, @@ -601,7 +595,6 @@ registerSuite('support/meta/Drag', { } ], 'dragging should be attributed to parent node'); - widget.destroy(); document.body.removeChild(div); }, @@ -695,7 +688,6 @@ registerSuite('support/meta/Drag', { emptyResults ], 'there should be no drag results'); - widget.destroy(); document.body.removeChild(div); }, @@ -781,7 +773,6 @@ registerSuite('support/meta/Drag', { emptyResults ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); }, @@ -893,7 +884,6 @@ registerSuite('support/meta/Drag', { } ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); }, @@ -999,7 +989,6 @@ registerSuite('support/meta/Drag', { } ], 'the stack of should represent a drag state'); - widget.destroy(); document.body.removeChild(div); } } diff --git a/tests/unit/meta/Matches.ts b/tests/unit/meta/Matches.ts index a6cbde19..16857ac5 100644 --- a/tests/unit/meta/Matches.ts +++ b/tests/unit/meta/Matches.ts @@ -54,7 +54,6 @@ registerSuite('support/meta/Matches', { assert.deepEqual(results, [ true ], 'should have been called and the target matched'); - widget.destroy(); document.body.removeChild(div); }, @@ -89,7 +88,6 @@ registerSuite('support/meta/Matches', { assert.deepEqual(results, [ true ], 'should have been called and the target matched'); - widget.destroy(); document.body.removeChild(div); }, @@ -132,7 +130,6 @@ registerSuite('support/meta/Matches', { assert.deepEqual(results, [ false ], 'should have been called and the target not matching'); - widget.destroy(); document.body.removeChild(div); }, @@ -187,7 +184,6 @@ registerSuite('support/meta/Matches', { assert.deepEqual(results, [ true, false, false, true ], 'should have been called twice and keys changed'); - widget.destroy(); document.body.removeChild(div); } } diff --git a/tests/unit/mixins/I18n.ts b/tests/unit/mixins/I18n.ts index 176d1103..2e0b63ac 100644 --- a/tests/unit/mixins/I18n.ts +++ b/tests/unit/mixins/I18n.ts @@ -24,7 +24,6 @@ registerSuite('mixins/I18nMixin', { switchLocale(systemLocale); if (localized) { - localized.destroy(); localized = null; } }, diff --git a/tests/unit/mixins/Projector.ts b/tests/unit/mixins/Projector.ts index 6eded0d6..a14fa169 100644 --- a/tests/unit/mixins/Projector.ts +++ b/tests/unit/mixins/Projector.ts @@ -8,7 +8,6 @@ import { v } from '../../../src/d'; import { ProjectorMixin, ProjectorAttachState } from '../../../src/mixins/Projector'; import { WidgetBase } from '../../../src/WidgetBase'; import { beforeRender } from './../../../src/decorators/beforeRender'; -import { Registry } from './../../../src/Registry'; import { HNode } from './../../../src/interfaces'; const Event = global.window.Event; @@ -352,25 +351,6 @@ registerSuite('mixins/projectorMixin', { // projector.setProperties({ foo: true }); }, - 'registry destroyed when a new registry is received'() { - let registryDestroyedCount = 0; - class TestRegistry extends Registry { - destroy(): Promise { - registryDestroyedCount++; - return super.destroy(); - } - } - const projector = new BaseTestWidget(); - projector.setProperties({ registry: new TestRegistry() }); - assert.strictEqual(registryDestroyedCount, 0); - projector.setProperties({ registry: new TestRegistry() }); - assert.strictEqual(registryDestroyedCount, 1); - projector.setProperties({ registry: undefined }); - assert.strictEqual(registryDestroyedCount, 2); - projector.setProperties({ registry: new TestRegistry() }); - projector.destroy(); - assert.strictEqual(registryDestroyedCount, 3); - }, 'properties are reset to original state on render'() { const testProperties = { key: 'bar' diff --git a/tests/unit/util/DomWrapper.ts b/tests/unit/util/DomWrapper.ts index 20391774..34fd4734 100644 --- a/tests/unit/util/DomWrapper.ts +++ b/tests/unit/util/DomWrapper.ts @@ -19,7 +19,7 @@ registerSuite('DomWrapper', { afterEach() { resolvers.restore(); - projector && projector.destroy(); + projector = undefined; }, tests: { diff --git a/tests/unit/vdom.ts b/tests/unit/vdom.ts index 7c77eb82..2bdd9e6a 100644 --- a/tests/unit/vdom.ts +++ b/tests/unit/vdom.ts @@ -15,8 +15,12 @@ let consoleStub: SinonStub; const resolvers = createResolvers(); const projectorStub: any = { - on: stub(), - emit: stub() + nodeHandler: { + add: stub(), + addRoot: stub() + }, + onElementCreated: stub(), + onElementUpdated: stub() }; class MainBar extends WidgetBase { @@ -49,8 +53,8 @@ class TestWidget extends WidgetBase { describe('vdom', () => { beforeEach(() => { - projectorStub.on.reset(); - projectorStub.emit.reset(); + projectorStub.nodeHandler.add.reset(); + projectorStub.nodeHandler.addRoot.reset(); consoleStub = stub(console, 'warn'); resolvers.stub(); }); @@ -452,8 +456,8 @@ describe('vdom', () => { private myClass = false; - constructor() { - super(); + constructor(invalidate: Function) { + super(invalidate); fooInvalidate = this.invalidate.bind(this); } @@ -719,64 +723,9 @@ describe('vdom', () => { }, Error, 'Unable to replace a node with an array of nodes. (consider adding one extra level to the virtual DOM)'); }); - it('should destroy widgets when they are no longer required', () => { - let fooDestroyedCount = 0; - - class Foo extends WidgetBase { - destroy() { - fooDestroyedCount++; - return super.destroy(); - } - render() { - return null; - } - } - - class Bar extends WidgetBase { - private _count = 20; - - set count(value: number) { - this._count = value; - this.invalidate(); - - } - - render() { - const children: any[] = []; - for (let i = 0; i < this._count; i++) { - children.push(w(Foo, { key: i})); - } - - return v('div', children); - } - } - - const widget = new Bar(); - const projection = dom.create(widget.__render__() as HNode, widget); - resolvers.resolve(); - widget.count = 10; - projection.update(widget.__render__() as HNode); - resolvers.resolve(); - assert.strictEqual(fooDestroyedCount, 10); - fooDestroyedCount = 0; - widget.count = 10; - projection.update(widget.__render__() as HNode); - resolvers.resolve(); - assert.strictEqual(fooDestroyedCount, 0); - widget.count = 20; - projection.update(widget.__render__() as HNode); - resolvers.resolve(); - assert.strictEqual(fooDestroyedCount, 0); - widget.count = 0; - projection.update(widget.__render__() as HNode); - resolvers.resolve(); - assert.strictEqual(fooDestroyedCount, 20); - }); - - it('destroys existing widget and uses new widget when widget changes', () => { - let fooDestroyed = false; + it('removes existing widget and uses new widget when widget changes', () => { let fooCreated = false; - let barCreated = false; + let barCreatedCount = 0; class Foo extends WidgetBase { constructor() { @@ -784,11 +733,6 @@ describe('vdom', () => { fooCreated = true; } - destroy() { - fooDestroyed = true; - return super.destroy(); - } - render() { return v('div'); } @@ -797,7 +741,7 @@ describe('vdom', () => { class Bar extends WidgetBase { constructor() { super(); - barCreated = true; + barCreatedCount++; } render() { @@ -815,7 +759,8 @@ describe('vdom', () => { render() { return v('div', [ - this._foo ? w(Foo, {}) : w(Bar, {}) + this._foo ? w(Foo, {}) : w(Bar, {}), + this._foo ? w(Bar, { key: '1' }) : w(Bar, { key: '2' }) ]); } } @@ -827,50 +772,7 @@ describe('vdom', () => { widget.foo = false; projection.update(widget.__render__() as HNode); resolvers.resolve(); - assert.isTrue(fooDestroyed); - assert.isTrue(barCreated); - }); - - it('does not own child widgets', () => { - let fooDestroyed = false; - let barDestroyed = false; - class Foo extends WidgetBase { - destroy() { - fooDestroyed = true; - return super.destroy(); - } - } - - class Bar extends WidgetBase { - destroy() { - barDestroyed = true; - return super.destroy(); - } - - render() { - return v('div', [ w(Foo, {}) ]); - } - } - - class Baz extends WidgetBase { - private _show = false; - - render() { - this._show = !this._show; - return v('div', [ - this._show ? w(Bar, {}) : null - ]); - } - } - - const widget = new Baz(); - const projection = dom.create(widget.__render__(), widget); - resolvers.resolve(); - widget.invalidate(); - projection.update(widget.__render__()); - resolvers.resolve(); - assert.isTrue(barDestroyed); - assert.isFalse(fooDestroyed); + assert.strictEqual(barCreatedCount, 3); }); it('remove elements for embedded WNodes', () => { @@ -2297,7 +2199,7 @@ describe('vdom', () => { it('should run afterRenderCallbacks sync', () => { const projection = dom.create(v('div', { key: '1' }), projectorStub, { sync: true }); - assert.isTrue(projectorStub.emit.calledWith({ type: 'element-created', element: (projection.domNode.childNodes[0] as Element), key: '1' })); + assert.isTrue(projectorStub.nodeHandler.add.calledWith(projection.domNode.childNodes[0] as Element, '1' )); }); it('should run defferedRenderCallbacks sync', () => { @@ -2312,46 +2214,69 @@ describe('vdom', () => { describe('node callbacks', () => { - it('element-created not emitted for new nodes without a key', () => { - dom.create(v('div'), projectorStub); + it('element not added to node handler for nodes without a key', () => { + const projection = dom.create(v('div'), projectorStub); + resolvers.resolve(); + projection.update(v('div')); resolvers.resolve(); - assert.isTrue(projectorStub.emit.neverCalledWith({ type: 'element-created' })); + assert.isTrue(projectorStub.nodeHandler.add.notCalled); }); - it('element-created emitted for new nodes with a key', () => { + it('element added on create to node handler for nodes with a key', () => { const projection = dom.create(v('div', { key: '1' }), projectorStub); - resolvers.resolve(); - assert.isTrue(projectorStub.emit.calledWith({ type: 'element-created', element: (projection.domNode.childNodes[0] as Element), key: '1' })); + assert.isTrue(projectorStub.nodeHandler.add.called); + assert.isTrue(projectorStub.nodeHandler.add.calledWith(projection.domNode.childNodes[0] as Element, '1' )); + projectorStub.nodeHandler.add.reset(); + projection.update(v('div', { key: '1' })); + assert.isTrue(projectorStub.nodeHandler.add.called); + assert.isTrue(projectorStub.nodeHandler.add.calledWith(projection.domNode.childNodes[0] as Element, '1' )); }); - it('element-created emitted for new nodes with a key of 0', () => { + it('element added on update to node handler for nodes with a key of 0', () => { const projection = dom.create(v('div', { key: 0 }), projectorStub); - resolvers.resolve(); - assert.isTrue(projectorStub.emit.calledWith({ type: 'element-created', element: (projection.domNode.childNodes[0] as Element), key: 0 })); + assert.isTrue(projectorStub.nodeHandler.add.called); + assert.isTrue(projectorStub.nodeHandler.add.calledWith(projection.domNode.childNodes[0] as Element, '0' )); + projectorStub.nodeHandler.add.reset(); + projection.update(v('div', { key: 0 })); + assert.isTrue(projectorStub.nodeHandler.add.called); + assert.isTrue(projectorStub.nodeHandler.add.calledWith(projection.domNode.childNodes[0] as Element, '0' )); }); - it('element-updated not emitted for updated nodes without a key', () => { - const projection = dom.create(v('div'), projectorStub); + it('on element created and updated callbacks are called for nodes with keys', () => { + const projection = dom.create(v('div', { key: 0 }), projectorStub); resolvers.resolve(); - projection.update(v('div')); + assert.isTrue(projectorStub.onElementCreated.called); + assert.isTrue(projectorStub.onElementCreated.calledWith(projection.domNode.childNodes[0] as Element, 0 )); + projection.update(v('div', { key: 0 })); resolvers.resolve(); - assert.isTrue(projectorStub.emit.neverCalledWith({ type: 'element-updated' })); + assert.isTrue(projectorStub.onElementUpdated.called); + assert.isTrue(projectorStub.onElementUpdated.calledWith(projection.domNode.childNodes[0] as Element, 0 )); }); - it('element-updated emitted for updated nodes with a key', () => { - const projection = dom.create(v('div', { key: '1' }), projectorStub); - resolvers.resolve(); - projection.update(v('div', { key: '1' })); - resolvers.resolve(); - assert.isTrue(projectorStub.emit.calledWith({ type: 'element-updated', element: (projection.domNode.childNodes[0] as Element), key: '1' })); + it('addRoot called on node handler for created widgets with a zero key', () => { + const widget = new WidgetBase(); + widget.__setProperties__({ key: 0 }); + + const projection = dom.create(widget.__render__(), projectorStub); + assert.isTrue(projectorStub.nodeHandler.addRoot.called); + projectorStub.nodeHandler.addRoot.reset(); + widget.invalidate(); + projection.update(widget.__render__()); + assert.isTrue(projectorStub.nodeHandler.addRoot.called); + projectorStub.nodeHandler.addRoot.reset(); }); - it('element-updated emitted for updated nodes with a key of 0', () => { - const projection = dom.create(v('div', { key: 0 }), projectorStub); - resolvers.resolve(); - projection.update(v('div', { key: 0 })); - resolvers.resolve(); - assert.isTrue(projectorStub.emit.calledWith({ type: 'element-updated', element: (projection.domNode.childNodes[0] as Element), key: 0 })); + it('addRoot called on node handler for updated widgets with key', () => { + const widget = new WidgetBase(); + widget.__setProperties__({ key: '1' }); + + const projection = dom.create(widget.__render__(), projectorStub); + assert.isTrue(projectorStub.nodeHandler.addRoot.called); + projectorStub.nodeHandler.addRoot.reset(); + widget.invalidate(); + projection.update(widget.__render__()); + assert.isTrue(projectorStub.nodeHandler.addRoot.called); + projectorStub.nodeHandler.addRoot.reset(); }); }); From fb58d18291df757a20bec7f287bc0ba67803dd92 Mon Sep 17 00:00:00 2001 From: Anthony Gubler Date: Fri, 10 Nov 2017 11:53:08 +0000 Subject: [PATCH 2/2] pass schedule render as parent invalidate for top level mechanism --- src/WidgetBase.ts | 8 ++++---- src/mixins/Projector.ts | 6 +----- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/WidgetBase.ts b/src/WidgetBase.ts index 725d3bfb..2d981531 100644 --- a/src/WidgetBase.ts +++ b/src/WidgetBase.ts @@ -108,14 +108,14 @@ export class WidgetBase

implement public readonly nodeHandler: NodeHandler = new NodeHandler(); - private _parentInvalidate: Function; + protected parentInvalidate: Function; /** * @constructor */ constructor(invalidate?: Function) { if (invalidate) { - this._parentInvalidate = invalidate; + this.parentInvalidate = invalidate; } this._children = []; @@ -274,8 +274,8 @@ export class WidgetBase

implement public invalidate(): void { if (this._renderState === WidgetRenderState.IDLE) { this._dirty = true; - if (this._parentInvalidate) { - this._parentInvalidate(); + if (this.parentInvalidate) { + this.parentInvalidate(); } } else if (this._renderState === WidgetRenderState.PROPERTIES) { diff --git a/src/mixins/Projector.ts b/src/mixins/Projector.ts index 15af0e13..cf3f7063 100644 --- a/src/mixins/Projector.ts +++ b/src/mixins/Projector.ts @@ -163,6 +163,7 @@ export function ProjectorMixin>>(Base: T) constructor(...args: any[]) { super(...args); + this.parentInvalidate = this.scheduleRender.bind(this); this._projectionOptions = { transitions: cssTransitions }; @@ -173,11 +174,6 @@ export function ProjectorMixin>>(Base: T) this.projectorState = ProjectorAttachState.Detached; } - public invalidate() { - super.invalidate(); - this.scheduleRender(); - } - public append(root?: Element): Handle { const options = { type: AttachType.Append,