From 63e04c1f2aeee66d67c2bdbb4345d4a23185c7b0 Mon Sep 17 00:00:00 2001 From: telamonian Date: Sun, 26 Jan 2020 03:20:14 -0500 Subject: [PATCH 1/6] simplified/improved the implementation of "passthru" virtual nodes - build of @lumino/widgets currently broken, still need to: - add fixes to widgets (title and tabbar) - fix unittests in virtualdom pkg --- .gitignore | 8 ++ packages/virtualdom/src/index.ts | 217 +++++++++++++------------------ 2 files changed, 100 insertions(+), 125 deletions(-) diff --git a/.gitignore b/.gitignore index 71160ea30..d5df56f92 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,11 @@ node_modules build review/api/temp *.tsbuildinfo + +# jetbrains IDE stuff +*.iml +.idea/ + +# ms IDE stuff +.vscode + diff --git a/packages/virtualdom/src/index.ts b/packages/virtualdom/src/index.ts index 60bd07adb..661b03bbc 100644 --- a/packages/virtualdom/src/index.ts +++ b/packages/virtualdom/src/index.ts @@ -725,6 +725,13 @@ class VirtualElement { */ readonly children: ReadonlyArray; + /** + * An optional custom renderer for the element's children. If set, on render + * this element's DOM node and it's attrs will be created/updated as normal. + * At that point the DOM node is handed off to the renderer. + */ + readonly renderer: VirtualElement.IRenderer | undefined; + /** * The type of the node. * @@ -741,75 +748,63 @@ class VirtualElement { * @param attrs - The element attributes. * * @param children - The element children. + * + * @param renderer - An optional custom renderer for the element. */ - constructor(tag: string, attrs: ElementAttrs, children: ReadonlyArray) { + constructor(tag: string, attrs: ElementAttrs, children: ReadonlyArray, renderer?: VirtualElement.IRenderer) { this.tag = tag; this.attrs = attrs; this.children = children; + + this.renderer = renderer; } } - -/** - * A "pass thru" virtual node whose children are managed by a render and an - * unrender callback. The intent of this flavor of virtual node is to make - * it easy to blend other kinds of virtualdom (eg React) into Phosphor's - * virtualdom. - * - * #### Notes - * User code will not typically create a `VirtualElementPass` node directly. - * Instead, the `hpass()` function will be used to create an element tree. - */ -export -class VirtualElementPass{ +export namespace VirtualElement { /** - * The type of the node. - * - * This value can be used as a type guard for discriminating the - * `VirtualNode` union type. + * A type describing a custom element renderer */ - readonly type: 'passthru' = 'passthru'; - - /** - * Construct a new virtual element pass thru node. - * - * @param tag - the tag of the parent element of this node. Once the parent - * element is rendered, it will be passed as an argument to - * renderer.render - * - * @param attrs - attributes that will assigned to the - * parent element - * - * @param renderer - an object with render and unrender - * functions, each of which should take a single argument of type - * HTMLElement and return nothing. If null, the parent element - * will be rendered barren without any children. - */ - constructor(readonly tag: string, readonly attrs: ElementAttrs, readonly renderer: VirtualElementPass.IRenderer | null) {} - - render(host: HTMLElement): void { - // skip actual render if renderer is null - if (this.renderer) { - this.renderer.render(host); - } - } - - unrender(host: HTMLElement): void { - // skip actual unrender if renderer is null - if (this.renderer) { - this.renderer.unrender(host); - } - } -} - - -/** - * The namespace for the VirtualElementPass class statics. - */ -export namespace VirtualElementPass { export type IRenderer = { - render: (host: HTMLElement) => void, - unrender: (host: HTMLElement) => void + /** + * Customize how a DOM node is rendered. If .renderer is set on a given + * instance of VirtualElement, this function will be called every time + * that VirtualElement is rendered. + * + * @param host - The actual DOM node created for a VirtualElement during + * rendering. + * + * On render, host is created and its attrs are set/updated via + * the standard routines in updateContent. host is then handed off to this + * function. + * + * The render function is free to modify host. The only restriction is + * is that render should not modify any attributes set by external + * routines (ie updateContent), as this may cause thrashing when the + * virtual element is next rendered. + * + * @param options - Will be populated with the .attrs and .children fields + * set on the VirtualElement being rendered. + */ + render: (host: HTMLElement, options?: {attrs?: ElementAttrs, children?: ReadonlyArray}) => void; + + /** + * Optional cleanup function for custom renderers. If the .renderer field + * of a VirtualELement is set, and if .renderer.unrender is defined, when + * the element is changed or removed its corresponding DOM element will be + * passed to this function immediately before it is removed from the DOM. + * + * unrender is not required for for simple renderers, such as those + * implemented using `document.createElement()`. However, for certain + * rendering techniques explicit cleanup is required in order to avoid + * resource leaks. + * + * For example, if render calls `ReactDOM.render(..., host)`, then + * there has to also be a corresponding implementation of unrender that + * calls `ReactDOM.unmountComponentAtNode(host)`. + * + * @param host - the DOM element to be removed. + */ + unrender?: (host: HTMLElement) => void; }; } @@ -818,7 +813,7 @@ export namespace VirtualElementPass { * A type alias for a general virtual node. */ export -type VirtualNode = VirtualElement | VirtualElementPass | VirtualText; +type VirtualNode = VirtualElement | VirtualText; /** @@ -828,6 +823,8 @@ type VirtualNode = VirtualElement | VirtualElementPass | VirtualText; * * @param attrs - The attributes for the element, if any. * + * @param renderer - An optional custom renderer for the element. + * * @param children - The children for the element, if any. * * @returns A new virtual element node for the given parameters. @@ -845,8 +842,11 @@ type VirtualNode = VirtualElement | VirtualElementPass | VirtualText; */ export function h(tag: string, ...children: h.Child[]): VirtualElement; export function h(tag: string, attrs: ElementAttrs, ...children: h.Child[]): VirtualElement; +export function h(tag: string, renderer: VirtualElement.IRenderer, ...children: h.Child[]): VirtualElement; +export function h(tag: string, attrs: ElementAttrs, renderer: VirtualElement.IRenderer, ...children: h.Child[]): VirtualElement; export function h(tag: string): VirtualElement { let attrs: ElementAttrs = {}; + let renderer: VirtualElement.IRenderer | undefined; let children: VirtualNode[] = []; for (let i = 1, n = arguments.length; i < n; ++i) { let arg = arguments[i]; @@ -856,15 +856,18 @@ export function h(tag: string): VirtualElement { children.push(arg); } else if (arg instanceof VirtualElement) { children.push(arg); - } else if (arg instanceof VirtualElementPass) { - children.push(arg); } else if (arg instanceof Array) { extend(children, arg); - } else if (i === 1 && arg && typeof arg === 'object') { - attrs = arg; + } else if ((i === 1 || i === 2) && arg && typeof arg === 'object') { + if ("render" in arg && "unrender" in arg) { + renderer = arg; + } + else { + attrs = arg; + } } } - return new VirtualElement(tag, attrs, children); + return new VirtualElement(tag, attrs, children, renderer); function extend(array: VirtualNode[], values: h.Child[]): void { for (let child of values) { @@ -874,8 +877,6 @@ export function h(tag: string): VirtualElement { array.push(child); } else if (child instanceof VirtualElement) { array.push(child); - } else if (child instanceof VirtualElementPass) { - array.push(child); } } } @@ -1002,43 +1003,6 @@ namespace h { } -/** - * Create a new "pass thru" virtual element node. - * - * @param tag - The tag name for the parent element. - * - * @param attrs - The attributes for the parent element, if any. - * - * @param renderer - an object with render and unrender functions, if any. - * - * @returns A new "pass thru" virtual element node for the given parameters. - * - */ -export function hpass(tag: string, renderer?: VirtualElementPass.IRenderer): VirtualElementPass; -export function hpass(tag: string, attrs: ElementAttrs, renderer?: VirtualElementPass.IRenderer): VirtualElementPass; -export function hpass(tag: string): VirtualElementPass { - let attrs: ElementAttrs = {}; - let renderer: VirtualElementPass.IRenderer | null = null; - - if (arguments.length === 2) { - const arg = arguments[1]; - - if ("render" in arg && "unrender" in arg) { - renderer = arg; - } else { - attrs = arg; - } - } else if (arguments.length === 3) { - attrs = arguments[1]; - renderer = arguments[2]; - } else if (arguments.length > 3) { - throw new Error("hpass() should be called with 1, 2, or 3 arguments"); - } - - return new VirtualElementPass(tag, attrs, renderer); -} - - /** * The namespace for the virtual DOM rendering functions. */ @@ -1059,7 +1023,6 @@ namespace VirtualDOM { */ export function realize(node: VirtualText): Text; export function realize(node: VirtualElement): HTMLElement; - export function realize(node: VirtualElementPass): HTMLElement; export function realize(node: VirtualNode): HTMLElement | Text { return Private.createDOMNode(node); } @@ -1097,12 +1060,14 @@ namespace Private { /** * A weak mapping of host element to virtual DOM content. */ - export const hostMap = new WeakMap>(); + export + const hostMap = new WeakMap>(); /** * Cast a content value to a content array. */ - export function asContentArray(value: VirtualNode | ReadonlyArray | null): ReadonlyArray { + export + function asContentArray(value: VirtualNode | ReadonlyArray | null): ReadonlyArray { if (!value) { return []; } @@ -1117,7 +1082,6 @@ namespace Private { */ export function createDOMNode(node: VirtualText): Text; export function createDOMNode(node: VirtualElement): HTMLElement; - export function createDOMNode(node: VirtualElementPass): HTMLElement; export function createDOMNode(node: VirtualNode): HTMLElement | Text; export function createDOMNode(node: VirtualNode, host: HTMLElement | null): HTMLElement | Text; export function createDOMNode(node: VirtualNode, host: HTMLElement | null, before: Node | null): HTMLElement | Text; @@ -1139,8 +1103,8 @@ namespace Private { // Add the attributes for the new element. addAttrs(host, node.attrs); - if (node.type === 'passthru') { - node.render(host); + if (node.renderer) { + node.renderer.render(host, {attrs: node.attrs, children: node.children}); return host; } @@ -1159,7 +1123,8 @@ namespace Private { * This is the core "diff" algorithm. There is no explicit "patch" * phase. The host is patched at each step as the diff progresses. */ - export function updateContent(host: HTMLElement, oldContent: ReadonlyArray, newContent: ReadonlyArray): void { + export + function updateContent(host: HTMLElement, oldContent: ReadonlyArray, newContent: ReadonlyArray): void { // Bail early if the content is identical. if (oldContent === newContent) { return; @@ -1203,15 +1168,15 @@ namespace Private { continue; } - // If the types of the old and new nodes differ, - // create and insert a new node. - if (oldVNode.type !== newVNode.type || oldVNode.type === 'text' || newVNode.type === 'text') { + // If the old or new node is a text node, the other node is now + // known to be an element node, so create and insert a new node. + if (oldVNode.type === 'text' || newVNode.type === 'text') { ArrayExt.insert(oldCopy, i, newVNode); createDOMNode(newVNode, host, currElem); continue; } - // At this point, both nodes are known to be of matching non-text type. + // At this point, both nodes are known to be element nodes. // If the new elem is keyed, move an old keyed elem to the proper // location before proceeding with the diff. The search can start @@ -1257,8 +1222,8 @@ namespace Private { updateAttrs(currElem as HTMLElement, oldVNode.attrs, newVNode.attrs); // Update the element content. - if (oldVNode.type === 'passthru' || newVNode.type === 'passthru') { - (newVNode as VirtualElementPass).render(currElem as HTMLElement); + if (newVNode.renderer) { + newVNode.renderer.render(currElem as HTMLElement); } else { updateContent(currElem as HTMLElement, oldVNode.children, newVNode.children); } @@ -1272,22 +1237,24 @@ namespace Private { } /** - * Handle cleanup of stale vdom and its associated DOM. Stale nodes are - * traversed recursively and any needed explicit cleanup is carried out ( - * in particular, the unrender callback of VirtualElementPass nodes). The - * stale children of the top level node are removed using removeChild. + * Handle cleanup of stale vdom and its associated DOM. The host node is + * traversed recursively (in depth-first order), and any explicit cleanup + * required by a child node is carried out when it is visited (eg if a node + * has a custom renderer, the renderer.unrender function will be called). + * Once the subtree beneath each child of host has been completely visited, + * that child will be removed via a call to host.removeChild. */ - function removeContent(host: HTMLElement, oldContent: ReadonlyArray, newCount: number, _sentinel = false) { + function removeContent(host: HTMLElement, oldContent: ReadonlyArray, newCount: number, _sentinel: boolean) { // Dispose of the old nodes pushed to the end of the host. for (let i = oldContent.length - 1; i >= newCount; --i) { const oldNode = oldContent[i]; const child = (_sentinel ? host.lastChild : host.childNodes[i]) as HTMLElement; // recursively clean up host children - if (oldNode.type === 'text') {} else if (oldNode.type === 'passthru') { - oldNode.unrender(child!); + if (oldNode.type === 'text') {} else if (oldNode.renderer && oldNode.renderer.unrender) { + oldNode.renderer.unrender(child!); } else { - removeContent(child!, oldNode.children, 0); + removeContent(child!, oldNode.children, 0, false); } if (_sentinel) { From 5711b2f2d0619b754dbb842331042f9aa0456bac Mon Sep 17 00:00:00 2001 From: telamonian Date: Sun, 26 Jan 2020 03:52:28 -0500 Subject: [PATCH 2/6] fixed build of widgets pkg wrt to custom rendering changes in virtualdom - needed to add new `h` signature overloads to the `h.IFactory` interface --- packages/virtualdom/src/index.ts | 2 ++ packages/virtualdom/tests/src/index.spec.ts | 2 +- packages/widgets/src/tabbar.ts | 10 +++------- packages/widgets/src/title.ts | 10 +++++----- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/virtualdom/src/index.ts b/packages/virtualdom/src/index.ts index 661b03bbc..48df4f994 100644 --- a/packages/virtualdom/src/index.ts +++ b/packages/virtualdom/src/index.ts @@ -901,6 +901,8 @@ namespace h { interface IFactory { (...children: Child[]): VirtualElement; (attrs: ElementAttrs, ...children: Child[]): VirtualElement; + (renderer: VirtualElement.IRenderer, ...children: h.Child[]): VirtualElement; + (attrs: ElementAttrs, renderer: VirtualElement.IRenderer, ...children: h.Child[]): VirtualElement; } export const a: IFactory = h.bind(undefined, 'a'); diff --git a/packages/virtualdom/tests/src/index.spec.ts b/packages/virtualdom/tests/src/index.spec.ts index 15aa4c440..1a5e4115c 100644 --- a/packages/virtualdom/tests/src/index.spec.ts +++ b/packages/virtualdom/tests/src/index.spec.ts @@ -100,7 +100,7 @@ describe('@lumino/virtualdom', () => { }); - describe('VirtualElementPass', () => { + describe('VirtualElement with custom .renderer', () => { let mockRenderer = { render: (host: HTMLElement) => {}, unrender: (host: HTMLElement) =>{} diff --git a/packages/widgets/src/tabbar.ts b/packages/widgets/src/tabbar.ts index cd605f30c..137b2bff2 100644 --- a/packages/widgets/src/tabbar.ts +++ b/packages/widgets/src/tabbar.ts @@ -32,7 +32,7 @@ import { } from '@lumino/signaling'; import { - ElementDataset, ElementInlineStyle, VirtualDOM, VirtualElement, VirtualElementPass, h, hpass + ElementDataset, ElementInlineStyle, VirtualDOM, VirtualElement, h } from '@lumino/virtualdom'; import { @@ -1356,15 +1356,11 @@ namespace TabBar { * * @returns A virtual element representing the tab icon. */ - renderIcon(data: IRenderData): VirtualElement | VirtualElementPass { + renderIcon(data: IRenderData): VirtualElement { const { title } = data; let className = this.createIconClass(data); - if (title.iconRenderer) { - return hpass('div', {className, title: title.iconLabel}, title.iconRenderer); - } else { - return h.div({className}, title.iconLabel); - } + return h.div({className}, title.iconRenderer, title.iconLabel); } /** diff --git a/packages/widgets/src/title.ts b/packages/widgets/src/title.ts index d0f233c58..151b1263c 100644 --- a/packages/widgets/src/title.ts +++ b/packages/widgets/src/title.ts @@ -11,7 +11,7 @@ import { ISignal, Signal } from '@lumino/signaling'; -import { VirtualElementPass } from "@lumino/virtualdom"; +import { VirtualElement } from "@lumino/virtualdom"; /** @@ -183,7 +183,7 @@ class Title { * #### Notes * The default value is undefined. */ - get iconRenderer(): VirtualElementPass.IRenderer { + get iconRenderer(): VirtualElement.IRenderer { return this._iconRenderer; } @@ -193,7 +193,7 @@ class Title { * #### Notes * A renderer is an object that supplies a render and unrender function. */ - set iconRenderer(value: VirtualElementPass.IRenderer) { + set iconRenderer(value: VirtualElement.IRenderer) { if (this._iconRenderer === value) { return; } @@ -299,7 +299,7 @@ class Title { private _mnemonic = -1; private _iconClass = ''; private _iconLabel = ''; - private _iconRenderer: VirtualElementPass.IRenderer; + private _iconRenderer: VirtualElement.IRenderer; private _className = ''; private _closable = false; private _dataset: Title.Dataset; @@ -357,7 +357,7 @@ namespace Title { * An object that supplies render and unrender functions used * to create and cleanup the icon of the title. */ - iconRenderer?: VirtualElementPass.IRenderer; + iconRenderer?: VirtualElement.IRenderer; /** * The caption for the title. From ccd3b10aca3283bcfb35b84dce5d46662d80646f Mon Sep 17 00:00:00 2001 From: telamonian Date: Sun, 26 Jan 2020 03:59:42 -0500 Subject: [PATCH 3/6] fixed build of virtualdom tests wrt recent changes to vdom custom renderers - still need to actually try running the tests --- packages/virtualdom/tests/src/index.spec.ts | 46 ++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/virtualdom/tests/src/index.spec.ts b/packages/virtualdom/tests/src/index.spec.ts index 1a5e4115c..41093f869 100644 --- a/packages/virtualdom/tests/src/index.spec.ts +++ b/packages/virtualdom/tests/src/index.spec.ts @@ -12,7 +12,7 @@ import { } from 'chai'; import { - VirtualDOM, VirtualElement, VirtualElementPass, VirtualText, h, hpass + VirtualDOM, VirtualElement, VirtualText, h } from '@lumino/virtualdom'; @@ -109,8 +109,8 @@ describe('@lumino/virtualdom', () => { describe('#constructor()', () => { it('should create a virtual element node', () => { - let vnode = new VirtualElementPass('div', {}, mockRenderer); - expect(vnode).to.be.an.instanceof(VirtualElementPass); + let vnode = new VirtualElement('div', {}, [], mockRenderer); + expect(vnode).to.be.an.instanceof(VirtualElement); }); }); @@ -118,7 +118,7 @@ describe('@lumino/virtualdom', () => { describe('#type', () => { it('should be `element`', () => { - let vnode = new VirtualElementPass('div', {}, mockRenderer); + let vnode = new VirtualElement('div', {}, [], mockRenderer); expect(vnode.type).to.equal('passthru'); }); @@ -127,7 +127,7 @@ describe('@lumino/virtualdom', () => { describe('#tag', () => { it('should be the element tag name', () => { - let vnode = new VirtualElementPass('img', {}, mockRenderer); + let vnode = new VirtualElement('img', {}, [], mockRenderer); expect(vnode.tag).to.equal('img'); }); @@ -137,7 +137,7 @@ describe('@lumino/virtualdom', () => { it('should be the element attrs', () => { let attrs = { className: 'baz' }; - let vnode = new VirtualElementPass('img', attrs, mockRenderer); + let vnode = new VirtualElement('img', attrs, [], mockRenderer); expect(vnode.attrs).to.deep.equal(attrs); }); @@ -146,7 +146,7 @@ describe('@lumino/virtualdom', () => { describe('#renderer', () => { it('should be the element children renderer', () => { - let vnode = new VirtualElementPass('div', {}, mockRenderer); + let vnode = new VirtualElement('div', {}, [], mockRenderer); expect(vnode.renderer!.render).to.equal(mockRenderer.render); expect(vnode.renderer!.unrender).to.equal(mockRenderer.unrender); }); @@ -353,47 +353,47 @@ describe('@lumino/virtualdom', () => { unrender: (host: HTMLElement) =>{} }; - it('should create a new virtual element passthru node', () => { - let vnode = hpass( + it('should create a new virtual element with custom renderer', () => { + let vnode = h( tag, attrs, mockRenderer ); - expect(vnode).to.be.an.instanceof(VirtualElementPass); + expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal(tag); expect(vnode.attrs).to.deep.equal(attrs); expect(vnode.renderer!.render).to.equal(mockRenderer.render); expect(vnode.renderer!.unrender).to.equal(mockRenderer.unrender); }); - it('should create a passthru vnode without attrs', () => { - let vnode = hpass( + it('should create a virtual element with custom renderer and without attrs', () => { + let vnode = h( 'div', mockRenderer ); - expect(vnode).to.be.an.instanceof(VirtualElementPass); + expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal('div'); expect(vnode.attrs).to.deep.equal({}); expect(vnode.renderer!.render).to.equal(mockRenderer.render); expect(vnode.renderer!.unrender).to.equal(mockRenderer.unrender); }); - it('should create a passthru vnode without renderer', () => { - let vnode = hpass( + it('should create a virtual element without custom renderer and with attrs', () => { + let vnode = h( 'div', attrs ); - expect(vnode).to.be.an.instanceof(VirtualElementPass); + expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal(tag); expect(vnode.attrs).to.deep.equal(attrs); expect(vnode.renderer).to.equal(null); }); - it('should create a passthru vnode without attrs or renderer', () => { - let vnode = hpass( + it('should create a virtual element without custom renderer or attrs', () => { + let vnode = h( 'div' ); - expect(vnode).to.be.an.instanceof(VirtualElementPass); + expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal('div'); expect(vnode.attrs).to.deep.equal({}); expect(vnode.renderer).to.equal(null); @@ -551,7 +551,7 @@ describe('@lumino/virtualdom', () => { describe('realize()', () => { it('should realize successfully', () => { - let node = VirtualDOM.realize(hpass('span', rendererClosure())); + let node = VirtualDOM.realize(h('span', rendererClosure())); expect(node.tagName.toLowerCase()).to.equal('span'); expect(node.children[0].tagName.toLowerCase()).to.equal('div'); expect(node.children[0].className).to.equal('p-render'); @@ -563,7 +563,7 @@ describe('@lumino/virtualdom', () => { it('should render successfully at top of tree', () => { let host = document.createElement('div'); - VirtualDOM.render(hpass('span', rendererClosure()), host); + VirtualDOM.render(h('span', rendererClosure()), host); expect(host.children[0].tagName.toLowerCase()).to.equal('span'); expect(host.children[0].children[0].tagName.toLowerCase()).to.equal('div'); expect(host.children[0].children[0].className).to.equal('p-render'); @@ -573,7 +573,7 @@ describe('@lumino/virtualdom', () => { let host = document.createElement('div'); let record: any = {child: undefined, cleanedUp: false}; - let children = [h.a(), h.span(), h.div(h.div(), hpass('span', rendererClosure(record)), h.div())]; + let children = [h.a(), h.span(), h.div(h.div(), h('span', rendererClosure(record)), h.div())]; VirtualDOM.render(children, host); expect(host.children[2].children[1].children[0]).to.equal(record.child); expect(host.children[2].children[1].children[0].className).to.equal('p-render'); @@ -584,7 +584,7 @@ describe('@lumino/virtualdom', () => { let record: any = {child: undefined, cleanedUp: false}; // first pass, render the hpass children - let children0 = [h.a(), h.span(), h.div(h.div(), hpass('span', rendererClosure(record)), h.div())]; + let children0 = [h.a(), h.span(), h.div(h.div(), h('span', rendererClosure(record)), h.div())]; VirtualDOM.render(children0, host); // second pass, explicitly unrender the hpass children From 628280502ceb5ebe0729bd07f5a2c05888d100ea Mon Sep 17 00:00:00 2001 From: telamonian Date: Sun, 26 Jan 2020 04:04:50 -0500 Subject: [PATCH 4/6] finished fixing unittests for virtualdom pkg --- packages/virtualdom/tests/src/index.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/virtualdom/tests/src/index.spec.ts b/packages/virtualdom/tests/src/index.spec.ts index 41093f869..b64e2ae35 100644 --- a/packages/virtualdom/tests/src/index.spec.ts +++ b/packages/virtualdom/tests/src/index.spec.ts @@ -119,7 +119,7 @@ describe('@lumino/virtualdom', () => { it('should be `element`', () => { let vnode = new VirtualElement('div', {}, [], mockRenderer); - expect(vnode.type).to.equal('passthru'); + expect(vnode.type).to.equal('element'); }); }); @@ -345,7 +345,7 @@ describe('@lumino/virtualdom', () => { }); - describe('hpass()', () => { + describe('h() with IRenderer param', () => { let tag = 'div'; let attrs = { className: 'baz' }; let mockRenderer = { @@ -386,7 +386,7 @@ describe('@lumino/virtualdom', () => { expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal(tag); expect(vnode.attrs).to.deep.equal(attrs); - expect(vnode.renderer).to.equal(null); + expect(vnode.renderer).to.equal(undefined); }); it('should create a virtual element without custom renderer or attrs', () => { @@ -396,7 +396,7 @@ describe('@lumino/virtualdom', () => { expect(vnode).to.be.an.instanceof(VirtualElement); expect(vnode.tag).to.equal('div'); expect(vnode.attrs).to.deep.equal({}); - expect(vnode.renderer).to.equal(null); + expect(vnode.renderer).to.equal(undefined); }); }); @@ -533,7 +533,7 @@ describe('@lumino/virtualdom', () => { }); - describe('VirtualDOM passthru', () => { + describe('VirtualDOM with custom renderer', () => { const rendererClosure = (record: any = {}) => { return { render: (host: HTMLElement) => { @@ -583,11 +583,11 @@ describe('@lumino/virtualdom', () => { let host = document.createElement('div'); let record: any = {child: undefined, cleanedUp: false}; - // first pass, render the hpass children + // first pass, render the custom children let children0 = [h.a(), h.span(), h.div(h.div(), h('span', rendererClosure(record)), h.div())]; VirtualDOM.render(children0, host); - // second pass, explicitly unrender the hpass children + // second pass, explicitly unrender the custom children let children1 = [h.a(), h.span(), h.label()]; VirtualDOM.render(children1, host); expect(record.cleanedUp).to.equal(true); From 792cd0b37a18bf8d85cd32a86bb5c69696c09e60 Mon Sep 17 00:00:00 2001 From: telamonian Date: Sun, 26 Jan 2020 18:39:13 -0500 Subject: [PATCH 5/6] added backwards compatibility shim for `hpass` and `VirtualElementPass` --- packages/virtualdom/src/index.ts | 89 +++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/virtualdom/src/index.ts b/packages/virtualdom/src/index.ts index 48df4f994..95a5608df 100644 --- a/packages/virtualdom/src/index.ts +++ b/packages/virtualdom/src/index.ts @@ -760,7 +760,8 @@ class VirtualElement { } } -export namespace VirtualElement { +export +namespace VirtualElement { /** * A type describing a custom element renderer */ @@ -808,6 +809,53 @@ export namespace VirtualElement { }; } +/** + * DEPRECATED - use VirtualElement with a defined renderer param instead. + * This class is provided as a backwards compatibility shim + * + * A "pass thru" virtual node whose children are managed by a render and an + * unrender callback. The intent of this flavor of virtual node is to make + * it easy to blend other kinds of virtualdom (eg React) into Phosphor's + * virtualdom. + * + * #### Notes + * User code will not typically create a `VirtualElementPass` node directly. + * Instead, the `hpass()` function will be used to create an element tree. + */ +export +class VirtualElementPass extends VirtualElement { + /** + * DEPRECATED - use VirtualElement with a defined renderer param instead + * + * Construct a new virtual element pass thru node. + * + * @param tag - the tag of the parent element of this node. Once the parent + * element is rendered, it will be passed as an argument to + * renderer.render + * + * @param attrs - attributes that will assigned to the + * parent element + * + * @param renderer - an object with render and unrender + * functions, each of which should take a single argument of type + * HTMLElement and return nothing. If null, the parent element + * will be rendered barren without any children. + */ + constructor(tag: string, attrs: ElementAttrs, renderer: VirtualElementPass.IRenderer | null) { + super(tag, attrs, [], renderer || undefined); + } +} + +export +namespace VirtualElementPass { + /** + * DEPRECATED - use VirtualElement.IRenderer instead + * + * A type describing a custom element renderer + */ + export type IRenderer = VirtualElement.IRenderer; +} + /** * A type alias for a general virtual node. @@ -1005,6 +1053,45 @@ namespace h { } +/** + * DEPRECATED - pass the renderer arg to the h function instead + * + * Create a new "pass thru" virtual element node. + * + * @param tag - The tag name for the parent element. + * + * @param attrs - The attributes for the parent element, if any. + * + * @param renderer - an object with render and unrender functions, if any. + * + * @returns A new "pass thru" virtual element node for the given parameters. + * + */ +export function hpass(tag: string, renderer?: VirtualElementPass.IRenderer): VirtualElementPass; +export function hpass(tag: string, attrs: ElementAttrs, renderer?: VirtualElementPass.IRenderer): VirtualElementPass; +export function hpass(tag: string): VirtualElementPass { + let attrs: ElementAttrs = {}; + let renderer: VirtualElementPass.IRenderer | null = null; + + if (arguments.length === 2) { + const arg = arguments[1]; + + if ("render" in arg && "unrender" in arg) { + renderer = arg; + } else { + attrs = arg; + } + } else if (arguments.length === 3) { + attrs = arguments[1]; + renderer = arguments[2]; + } else if (arguments.length > 3) { + throw new Error("hpass() should be called with 1, 2, or 3 arguments"); + } + + return new VirtualElementPass(tag, attrs, renderer); +} + + /** * The namespace for the virtual DOM rendering functions. */ From 3f8359c28f5b6d43be10946f89553e946ae45577 Mon Sep 17 00:00:00 2001 From: telamonian Date: Mon, 27 Jan 2020 15:57:37 -0500 Subject: [PATCH 6/6] removed unrender from runtime typecheck of renderer, as it's now optional --- packages/virtualdom/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/virtualdom/src/index.ts b/packages/virtualdom/src/index.ts index 95a5608df..087d15852 100644 --- a/packages/virtualdom/src/index.ts +++ b/packages/virtualdom/src/index.ts @@ -907,7 +907,7 @@ export function h(tag: string): VirtualElement { } else if (arg instanceof Array) { extend(children, arg); } else if ((i === 1 || i === 2) && arg && typeof arg === 'object') { - if ("render" in arg && "unrender" in arg) { + if ("render" in arg) { renderer = arg; } else { @@ -1076,7 +1076,7 @@ export function hpass(tag: string): VirtualElementPass { if (arguments.length === 2) { const arg = arguments[1]; - if ("render" in arg && "unrender" in arg) { + if ("render" in arg) { renderer = arg; } else { attrs = arg;