From 9263b17eee9f1b26fb9b0ceddf335829294d7eaa Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Fri, 25 Nov 2022 18:31:08 -0800 Subject: [PATCH 1/4] Create reduced test case --- packages/diffhtml-components/lib/component.js | 3 ++ .../diffhtml-components/lib/create-state.js | 2 ++ .../test/integration/hooks.js | 32 +++++++++++++++---- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/diffhtml-components/lib/component.js b/packages/diffhtml-components/lib/component.js index 628210e3..1329fe8b 100644 --- a/packages/diffhtml-components/lib/component.js +++ b/packages/diffhtml-components/lib/component.js @@ -289,6 +289,7 @@ export default class Component { } let renderTree = this.render(this.props, this.state); + console.log('Clearing render state', renderTree, this); ActiveRenderState.length = 0; // Do not render. @@ -353,6 +354,8 @@ export default class Component { return transaction; }); + console.log('diff into fragment', renderTree); + // Compare the existing component node(s) to the new node(s). const transaction = /** @type {Transaction} */(outerHTML(fragment, renderTree, { tasks })); diff --git a/packages/diffhtml-components/lib/create-state.js b/packages/diffhtml-components/lib/create-state.js index 555b62e7..058a84de 100644 --- a/packages/diffhtml-components/lib/create-state.js +++ b/packages/diffhtml-components/lib/create-state.js @@ -25,6 +25,8 @@ export function createState(defaultValue = {}) { const currentValue = activeHook ? activeHook[0] : defaultValue; const retVal = activeHook || [currentValue]; + console.log(activeComponent, currentValue); + /** * @param {any} newValue */ diff --git a/packages/diffhtml-components/test/integration/hooks.js b/packages/diffhtml-components/test/integration/hooks.js index 1e1f01b9..0d3b1492 100644 --- a/packages/diffhtml-components/test/integration/hooks.js +++ b/packages/diffhtml-components/test/integration/hooks.js @@ -416,8 +416,7 @@ describe('Hooks', function() { strictEqual(this.fixture.outerHTML, `
true
`); }); - it('will support nested setState with top-level re-rendering', async () => { - let setComponentValue = null; + it('will support nested createState with top-level re-rendering', async () => { let setNestedValue = null; function Nested() { @@ -429,10 +428,32 @@ describe('Hooks', function() { } function Component() { + return html`<${Nested} />`; + } + + this.fixture = document.createElement('div'); + + await innerHTML(this.fixture, html`<${Component} />`); + await setNestedValue(123); + strictEqual(this.fixture.outerHTML, `
123
`); + + await innerHTML(this.fixture, html`<${Component} />`); + strictEqual(this.fixture.outerHTML, `
123
`); + }); + + it('will support nested createState with top-level createState', async () => { + let setComponentValue = null; + let setNestedValue = null; + + function Nested() { const [ value, _setValue ] = createState(false); + setNestedValue = _setValue; + return html`${String(value)}`; + } + function Component() { + const [ _, _setValue ] = createState(); setComponentValue = _setValue; - return html`<${Nested} />`; } @@ -441,8 +462,9 @@ describe('Hooks', function() { await innerHTML(this.fixture, html`<${Component} />`); await setNestedValue(123); strictEqual(this.fixture.outerHTML, `
123
`); + console.log('>>> set component value <<<'); - await innerHTML(this.fixture, html`<${Component} />`); + await setComponentValue(); strictEqual(this.fixture.outerHTML, `
123
`); }); @@ -465,9 +487,7 @@ describe('Hooks', function() { function Component() { const [ value, _setValue ] = createState(false); - setComponentValue = _setValue; - return html`<${Nested} />`; } From 6451a9e40593c10f7ecb7255a2a85fe926df569d Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Tue, 13 Dec 2022 16:11:43 -0800 Subject: [PATCH 2/4] Initial work on refactoring the stateful re-render --- packages/diffhtml-components/lib/component.js | 181 ++++++++---------- .../diffhtml-components/lib/create-state.js | 2 +- .../diffhtml-components/lib/middleware.js | 13 +- .../lib/render-component.js | 4 +- .../test/integration/hooks.js | 8 +- 5 files changed, 102 insertions(+), 106 deletions(-) diff --git a/packages/diffhtml-components/lib/component.js b/packages/diffhtml-components/lib/component.js index 1329fe8b..64bd7cf5 100644 --- a/packages/diffhtml-components/lib/component.js +++ b/packages/diffhtml-components/lib/component.js @@ -55,24 +55,22 @@ const createProps = (domNode, existingProps = {}) => { const createState = (domNode, newState) => assign({}, domNode.state, newState); /** - * Finds all VTrees associated with a component. + * Recreates the VTree used for diffing a stateful component. * - * @param {VTree[]} childTrees - * @param {VTree} vTree + * @param {VTree} vTree - vTree to seed the recreation process + * @return {VTree} Recreated VTree including component references */ -const getChildTrees = (childTrees, vTree) => { +const recreateVTree = vTree => { + // Sucks that we need to loop this entire cache every time we re-render. This + // information should be more easily attained. ComponentTreeCache.forEach((parentTree, childTree) => { if (parentTree === vTree) { ComponentTreeCache.delete(childTree); - - if (typeof childTree.rawNodeName !== 'function') { - childTrees.push(childTree); - } - else { - getChildTrees(childTrees, childTree); - } + vTree.childNodes.push(childTree); } }); + + return vTree; }; /** @@ -234,12 +232,20 @@ export default class Component { /** * Stateful render. Used when a component changes and needs to re-render - * itself. This is triggered on `setState` and `forceUpdate` calls. + * itself and the trees it contains. This is triggered on `setState` and + * `forceUpdate` calls with class components and createState updates with + * function components. + * + * Web Components are easy to implement using the Shadow DOM to encapsulate + * the mount point using `innerHTML`. * - * @return {Transaction | undefined} + * React-like components are supported by recreating the previous component + * tree and comparing this to the new tree using `outerHTML`. + * + * @return {Transaction | unknown | undefined} */ [$$render]() { - // This is a WebComponent, so do something different. + // This is a WebComponent, which allows for simplified logic. if (this[$$type] === 'web') { const oldProps = this.props; const oldState = this.state; @@ -264,107 +270,90 @@ export default class Component { return transaction; } - // Get the fragment tree associated with this component. This is used to - // lookup rendered children. - let vTree = this[$$vTree]; - /** - * Find all previously rendered top-level children associated to this - * component. This will be used to diff against the newly rendered - * elements. + * Get the vTree associated with this component. * - * @type {VTree[]} + * @type {VTree | null} */ - const childTrees = []; - - // Lookup all DOM nodes that were associated at the top level with this - // component. - vTree && getChildTrees(childTrees, vTree); - - // Render directly from the Component. - ActiveRenderState.push(this); + const vTree = this[$$vTree]; - if ($$hooks in this) { - this[$$hooks].i = 0; - } - - let renderTree = this.render(this.props, this.state); - console.log('Clearing render state', renderTree, this); - ActiveRenderState.length = 0; - - // Do not render. - if (!renderTree) { - return; - } - - const renderTreeAsVTree = /** @type {VTree} */ (renderTree); - - // Always compare a fragment to a fragment. If the renderTree was not - // wrapped, ensure it is here. - if (renderTreeAsVTree.nodeType !== 11) { - const isList = 'length' in renderTree; - const renderTreeAsList = /** @type {VTree[]} */ (renderTree); - - renderTree = createTree(isList ? renderTreeAsList : [renderTreeAsVTree]); - } + console.log('Recreating tree for', vTree.rawNodeName.name); - // Put all the nodes together into a fragment for diffing. - const fragment = createTree(childTrees); + /** + * Recreate the existing tree including this component. This is not + * directly stored anywhere, as the VDOM tree is flattened, and must be + * recreated per render. + * + * @type {VTree} + */ + const fragment = recreateVTree(vTree); //tbd + const childTrees = [].concat(fragment.childNodes); const tasks = [...Internals.defaultTasks]; const syncTreesIndex = tasks.indexOf(Internals.tasks.syncTrees); + // Ensure this fragment isn't cleaned up until we are done rendering. + Internals.memory.protectVTree(fragment); + // Inject a custom task after syncing has finished, but before patching has - // occured. This gives us time to add additional patch logic per render. - tasks.splice(syncTreesIndex + 1, 0, (/** @type {Transaction} */transaction) => { - let lastTree = null; - - // Reconcile all top-level replacements and additions. - for (let i = 0; i < fragment.childNodes.length; i++) { - const childTree = fragment.childNodes[i]; - - // Replace if the nodes are different. - if (childTree && childTrees[i] && childTrees[i] !== childTree) { - transaction.patches.push( - Internals.PATCH_TYPE.REPLACE_CHILD, - childTree, - childTrees[i], - ); - - lastTree = childTree; - } - // Add if there is no old Node. - else if (lastTree && !childTrees[i]) { - transaction.patches.push( - Internals.PATCH_TYPE.INSERT_BEFORE, - $$insertAfter, - childTree, - lastTree, - ); - - lastTree = childTree; - } - // Keep the old node. - else { - lastTree = childTrees[i]; - } + // occured. This gives us time to add additional patch logic per render to + // support multiple top level elements. + tasks.splice( + syncTreesIndex + 1, 0, + function componentReconciler(/** @type {Transaction} */transaction) { + let lastTree = null; + + console.log('>>', transaction.patches); + // Reconcile all top-level replacements and additions. + for (let i = 0; i < fragment.childNodes.length; i++) { + const childTree = fragment.childNodes[i]; + + // Replace if the nodes are different. + if (childTree && fragment.childNodes[i] && childTrees[i] !== childTree) { + transaction.patches.push( + Internals.PATCH_TYPE.REPLACE_CHILD, + childTree, + childTrees[i], + ); + + lastTree = childTree; + } + // Add if there is no old Node. + else if (lastTree && !childTrees[i]) { + transaction.patches.push( + Internals.PATCH_TYPE.INSERT_BEFORE, + $$insertAfter, + childTree, + lastTree, + ); + + lastTree = childTree; + } + // Keep the old node. + else { + lastTree = childTrees[i]; + } - ComponentTreeCache.set(childTree, vTree); - } + ComponentTreeCache.set(childTree, vTree); + } - return transaction; - }); + return transaction; + }, + ); - console.log('diff into fragment', renderTree); + console.log('compare', fragment, 'with', vTree); // Compare the existing component node(s) to the new node(s). - const transaction = /** @type {Transaction} */(outerHTML(fragment, renderTree, { tasks })); + const transaction = /** @type {Transaction} */( + outerHTML(fragment, vTree, { tasks }) + ); + + console.log('here', fragment); // Empty the fragment after using. - fragment.childNodes.length = 0; + //fragment.childNodes.length = 0; release(fragment); this.componentDidUpdate(this.props, this.state); - return transaction; } connectedCallback() { diff --git a/packages/diffhtml-components/lib/create-state.js b/packages/diffhtml-components/lib/create-state.js index 058a84de..eb23f834 100644 --- a/packages/diffhtml-components/lib/create-state.js +++ b/packages/diffhtml-components/lib/create-state.js @@ -25,7 +25,7 @@ export function createState(defaultValue = {}) { const currentValue = activeHook ? activeHook[0] : defaultValue; const retVal = activeHook || [currentValue]; - console.log(activeComponent, currentValue); + //console.log(activeComponent.toString(), currentValue.toString()); /** * @param {any} newValue diff --git a/packages/diffhtml-components/lib/middleware.js b/packages/diffhtml-components/lib/middleware.js index 7db73480..5c017292 100644 --- a/packages/diffhtml-components/lib/middleware.js +++ b/packages/diffhtml-components/lib/middleware.js @@ -36,8 +36,13 @@ function render(oldTree, newTree, transaction) { if (!oldComponentTree && oldTree.childNodes) { oldComponentTree = ComponentTreeCache.get(oldTree.childNodes[0]); } + + if (!oldComponentTree && typeof oldTree.rawNodeName === 'function') { + oldComponentTree = oldTree; + } } + // If there is no old component, or if the components do not match, then we // are rendering a brand new component. if (!oldComponentTree || oldComponentTree.rawNodeName !== newTree.rawNodeName) { @@ -46,6 +51,7 @@ function render(oldTree, newTree, transaction) { // Otherwise re-use the existing component if the constructors are the same. if (oldComponentTree) { + // Update the incoming props/attrs. assign(oldComponentTree.attributes, newTree.attributes); @@ -96,11 +102,10 @@ const createNodeHook = vTree => { const syncTreeHook = (oldTree, newTree, transaction) => { // Render components during synchronization. if ( - // When child is a Component - typeof newTree.rawNodeName === 'function' && // If there is an oldTree and it's not the existing component, trigger a - // render. - (oldTree && oldTree.rawNodeName ? oldTree.rawNodeName !== newTree.rawNodeName : false) + // render. Or if the components match, re-render. + (oldTree && typeof oldTree.rawNodeName === 'function') || + (newTree && typeof newTree.rawNodeName === 'function') ) { return render(oldTree, newTree, transaction); } diff --git a/packages/diffhtml-components/lib/render-component.js b/packages/diffhtml-components/lib/render-component.js index 194852cc..cc99f69b 100644 --- a/packages/diffhtml-components/lib/render-component.js +++ b/packages/diffhtml-components/lib/render-component.js @@ -28,6 +28,8 @@ export default function renderComponent(vTree, transaction) { const isNewable = RawComponent.prototype && RawComponent.prototype.render; const isInstance = InstanceCache.has(vTree); + console.log('Rendering', vTree, InstanceCache.get(vTree)); + /** @type {VTree|null} */ let renderedTree = (null); @@ -107,8 +109,6 @@ export default function renderComponent(vTree, transaction) { * @param {any} state */ render(props, state) { - // Always render the latest `rawNodeName` of a VTree in case of - // hot-reloading the cached value above wouldn't be correct. return createTree(RawComponent(props, state)); } diff --git a/packages/diffhtml-components/test/integration/hooks.js b/packages/diffhtml-components/test/integration/hooks.js index 0d3b1492..5685b306 100644 --- a/packages/diffhtml-components/test/integration/hooks.js +++ b/packages/diffhtml-components/test/integration/hooks.js @@ -444,11 +444,13 @@ describe('Hooks', function() { it('will support nested createState with top-level createState', async () => { let setComponentValue = null; let setNestedValue = null; + let i = 0; function Nested() { + i++; const [ value, _setValue ] = createState(false); setNestedValue = _setValue; - return html`${String(value)}`; + return html`${String(value) + i}`; } function Component() { @@ -461,11 +463,11 @@ describe('Hooks', function() { await innerHTML(this.fixture, html`<${Component} />`); await setNestedValue(123); - strictEqual(this.fixture.outerHTML, `
123
`); + strictEqual(this.fixture.outerHTML, `
1232
`); console.log('>>> set component value <<<'); await setComponentValue(); - strictEqual(this.fixture.outerHTML, `
123
`); + strictEqual(this.fixture.outerHTML, `
1233
`); }); it('will support nested createSideEffect with top-level re-rendering', async () => { From 66a0f270e4d810c3aef8cde2da2209994ce68f76 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Thu, 22 Dec 2022 10:04:22 -0800 Subject: [PATCH 3/4] Rewrite component internals --- packages/diffhtml-components/lib/component.js | 184 ++++++------------ .../lib/lifecycle/component-will-unmount.js | 33 +--- .../lib/lifecycle/invoke-refs.js | 8 +- .../diffhtml-components/lib/middleware.js | 107 ++-------- .../lib/render-component.js | 54 ++--- .../diffhtml-components/lib/util/internals.js | 4 +- .../diffhtml-components/lib/util/symbols.js | 1 + .../diffhtml-components/lib/util/types.js | 2 - .../diffhtml-components/test/component.js | 60 +++--- .../test/integration/hooks.js | 9 +- .../test/util/validate-caches.js | 2 - packages/diffhtml/lib/html.js | 27 +-- packages/diffhtml/lib/release.js | 4 + packages/diffhtml/lib/tasks/sync-trees.js | 10 +- packages/diffhtml/lib/transaction.js | 3 + packages/diffhtml/lib/tree/sync.js | 10 +- 16 files changed, 183 insertions(+), 335 deletions(-) diff --git a/packages/diffhtml-components/lib/component.js b/packages/diffhtml-components/lib/component.js index 64bd7cf5..26527c3f 100644 --- a/packages/diffhtml-components/lib/component.js +++ b/packages/diffhtml-components/lib/component.js @@ -1,6 +1,5 @@ import { EMPTY, - ComponentTreeCache, InstanceCache, Props, Transaction, @@ -14,12 +13,12 @@ import { $$unsubscribe, $$type, $$hooks, - $$insertAfter, + $$children, } from './util/symbols'; import diff from './util/binding'; import middleware from './middleware'; -const { outerHTML, innerHTML, createTree, release, Internals } = diff; +const { innerHTML } = diff; const { isArray } = Array; const { setPrototypeOf, defineProperty, keys, assign } = Object; const RenderDebounce = new WeakMap(); @@ -32,7 +31,7 @@ const getObserved = ({ defaultProps }) => defaultProps ? keys(defaultProps) : EMPTY.ARR; /** - * Creates the `component.props` object. + * Creates the props object for Web components. * * @param {any} domNode * @param {Props} existingProps @@ -47,7 +46,7 @@ const createProps = (domNode, existingProps = {}) => { }; /** - * Creates the `component.state` object. + * Creates the state object for Web components. * * @param {Component} domNode * @param {State} newState @@ -55,22 +54,21 @@ const createProps = (domNode, existingProps = {}) => { const createState = (domNode, newState) => assign({}, domNode.state, newState); /** - * Recreates the VTree used for diffing a stateful component. + * Recreates the VTree used for diffing a stateful component. In the case of + * Web components, this will use the Shadow Root instead of creating a virtual + * fragment. * - * @param {VTree} vTree - vTree to seed the recreation process - * @return {VTree} Recreated VTree including component references + * @param {Component} instance - {Component} instance + * @param {Boolean} isWebComponent - Is this a web component? + * @return {VTree | HTMLElement} Recreated VTree including component references */ -const recreateVTree = vTree => { - // Sucks that we need to loop this entire cache every time we re-render. This - // information should be more easily attained. - ComponentTreeCache.forEach((parentTree, childTree) => { - if (parentTree === vTree) { - ComponentTreeCache.delete(childTree); - vTree.childNodes.push(childTree); - } - }); - - return vTree; +const createMountPoint = (instance, isWebComponent) => { + if (isWebComponent) { + return /** @type {any} */(instance).shadowRoot; + } + else { + return instance[$$children]; + } }; /** @@ -111,7 +109,6 @@ export default class Component { static unsubscribeMiddleware() { const unsubscribe = Component[$$unsubscribe]; unsubscribe && unsubscribe(); - ComponentTreeCache.clear(); InstanceCache.clear(); } @@ -134,7 +131,7 @@ export default class Component { /** @type {any} */ (instance).attachShadow({ mode: 'open' }); /** @type {any} */ (instance)[$$type] = 'web'; - } catch (e) { + } catch { // Not a Web Component. /** @type {any} */ (instance)[$$type] = 'class'; } @@ -157,13 +154,12 @@ export default class Component { } /** - * @param {Props} props - * @param {State} state + * @param {Props=} _props + * @param {State=} _state * - * @returns {VTree[] | VTree | undefined} + * @returns {VTree[] | VTree | any} */ - // @ts-ignore - render(props, state) { + render(_props, _state) { return undefined; } @@ -230,11 +226,14 @@ export default class Component { /** @type {{ fns: Function[], i: number }} */ [$$hooks] = { fns: [], i: 0 }; + /** @type {any} */ + [$$children] = []; + /** * Stateful render. Used when a component changes and needs to re-render - * itself and the trees it contains. This is triggered on `setState` and - * `forceUpdate` calls with class components and createState updates with - * function components. + * itself and the underlying tree it contains. This is triggered on + * `setState` and `forceUpdate` calls with class components and `createState` + * updates with function components. * * Web Components are easy to implement using the Shadow DOM to encapsulate * the mount point using `innerHTML`. @@ -245,115 +244,58 @@ export default class Component { * @return {Transaction | unknown | undefined} */ [$$render]() { - // This is a WebComponent, which allows for simplified logic. - if (this[$$type] === 'web') { - const oldProps = this.props; - const oldState = this.state; - - this.props = createProps(this, this.props); - this.state = createState(this, this.state); - - ActiveRenderState.push(this); - - if ($$hooks in this) { - this[$$hooks].i = 0; - } - - const transaction = /** @type {Transaction} */(innerHTML( - /** @type {any} */ (this).shadowRoot, - this.render(this.props, this.state), - )); - - ActiveRenderState.length = 0; - - this.componentDidUpdate(oldProps, oldState); - return transaction; + const vTree = /** @type {VTree=} */ this[$$vTree]; + if (!vTree) { + throw new Error('Missing required VTree to re-render'); } - /** - * Get the vTree associated with this component. - * - * @type {VTree | null} - */ - const vTree = this[$$vTree]; + const oldProps = this.props; + const oldState = this.state; - console.log('Recreating tree for', vTree.rawNodeName.name); + // There are some slight differences between rendering a typical class or + // function based component and a web component. Web Components are + // rendered into the shadow DOM, while the class based components need + // extra logic to handle the invisible component boundaries. + const isWebComponent = this[$$type] === 'web'; /** * Recreate the existing tree including this component. This is not * directly stored anywhere, as the VDOM tree is flattened, and must be * recreated per render. * - * @type {VTree} + * @type {VTree | HTMLElement} */ - const fragment = recreateVTree(vTree); //tbd - const childTrees = [].concat(fragment.childNodes); - const tasks = [...Internals.defaultTasks]; - const syncTreesIndex = tasks.indexOf(Internals.tasks.syncTrees); - - // Ensure this fragment isn't cleaned up until we are done rendering. - Internals.memory.protectVTree(fragment); - - // Inject a custom task after syncing has finished, but before patching has - // occured. This gives us time to add additional patch logic per render to - // support multiple top level elements. - tasks.splice( - syncTreesIndex + 1, 0, - function componentReconciler(/** @type {Transaction} */transaction) { - let lastTree = null; - - console.log('>>', transaction.patches); - // Reconcile all top-level replacements and additions. - for (let i = 0; i < fragment.childNodes.length; i++) { - const childTree = fragment.childNodes[i]; - - // Replace if the nodes are different. - if (childTree && fragment.childNodes[i] && childTrees[i] !== childTree) { - transaction.patches.push( - Internals.PATCH_TYPE.REPLACE_CHILD, - childTree, - childTrees[i], - ); - - lastTree = childTree; - } - // Add if there is no old Node. - else if (lastTree && !childTrees[i]) { - transaction.patches.push( - Internals.PATCH_TYPE.INSERT_BEFORE, - $$insertAfter, - childTree, - lastTree, - ); - - lastTree = childTree; - } - // Keep the old node. - else { - lastTree = childTrees[i]; - } + const mount = createMountPoint(this, isWebComponent); - ComponentTreeCache.set(childTree, vTree); - } + if (isWebComponent) { + this.props = createProps(this, this.props); + this.state = createState(this, this.state); + } - return transaction; - }, - ); + // Make this component active and then synchronously render. + ActiveRenderState.push(this); - console.log('compare', fragment, 'with', vTree); + // TBD Is this needed now that components track their own descendents? + if ($$hooks in this) { + this[$$hooks].i = 0; + } + + // Set the VTree attributes to match the current props. + Object.assign(vTree.attributes, this.props); - // Compare the existing component node(s) to the new node(s). - const transaction = /** @type {Transaction} */( - outerHTML(fragment, vTree, { tasks }) - ); + // Compare the newly rendered component contents into the mount. + const transaction = innerHTML(mount, this.render(this.props, this.state)); - console.log('here', fragment); + // Reset the active state after rendering so we don't accidentally bleed + // into other components render cycle. + ActiveRenderState.length = 0; - // Empty the fragment after using. - //fragment.childNodes.length = 0; - release(fragment); + if (!isWebComponent) { + release(mount); + } - this.componentDidUpdate(this.props, this.state); + this.componentDidUpdate(oldProps, oldState); + return transaction; } connectedCallback() { diff --git a/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js b/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js index 81f9574f..24b39c56 100644 --- a/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js +++ b/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js @@ -1,6 +1,6 @@ -import { ComponentTreeCache, InstanceCache, VTree } from '../util/types'; +import { InstanceCache, VTree } from '../util/types'; import diff from '../util/binding'; -import { $$hooks } from '../util/symbols'; +import { $$children, $$hooks } from '../util/symbols'; const { release, Internals } = diff; @@ -11,17 +11,6 @@ const { release, Internals } = diff; * @param {VTree} vTree - The respecting tree pointing to the component */ export default function componentWillUnmount(vTree) { - const componentTree = ComponentTreeCache.get(vTree); - - /** @type {VTree[]} */ - const childTrees = []; - - ComponentTreeCache.forEach((parentTree, childTree) => { - if (parentTree === componentTree) { - childTrees.push(childTree); - } - }); - const domNode = Internals.NodeCache.get(vTree); // Clean up attached Shadow DOM. @@ -29,14 +18,16 @@ export default function componentWillUnmount(vTree) { release(/** @type {any} */ (domNode).shadowRoot); } - vTree.childNodes.forEach(componentWillUnmount); + for (let i = 0; i < vTree.childNodes.length; i++) { + componentWillUnmount(vTree.childNodes[i]); + } - if (!InstanceCache.has(componentTree)) { + if (!InstanceCache.has(vTree)) { return; } - const instance = InstanceCache.get(componentTree); - InstanceCache.delete(componentTree); + const instance = InstanceCache.get(vTree); + InstanceCache.delete(vTree); // Empty out all hooks for gc. If using a stateless class or function, they // may not have this value set. @@ -45,12 +36,8 @@ export default function componentWillUnmount(vTree) { instance[$$hooks].i = 0; } - ComponentTreeCache.delete(vTree); - - // If there is a parent, ensure it is called recursively. - if (ComponentTreeCache.has(componentTree)) { - componentWillUnmount(componentTree); - } + // Ensure children are released as well. + release(instance[$$children]); // Ensure this is a stateful component. Stateless components do not get // lifecycle events yet. diff --git a/packages/diffhtml-components/lib/lifecycle/invoke-refs.js b/packages/diffhtml-components/lib/lifecycle/invoke-refs.js index e81a8998..189b944f 100644 --- a/packages/diffhtml-components/lib/lifecycle/invoke-refs.js +++ b/packages/diffhtml-components/lib/lifecycle/invoke-refs.js @@ -1,4 +1,4 @@ -import { EMPTY, ComponentTreeCache, InstanceCache, VTree } from '../util/types'; +import { EMPTY, InstanceCache, VTree } from '../util/types'; import diff from '../util/binding'; const { Internals } = diff; @@ -13,7 +13,7 @@ export const invokeRef = (target = EMPTY.OBJ, vTree, value) => { // Allow refs to be passed to HTML elements. When in a DOM environment // a Node will be passed to the ref function and assigned. - if (!ref) { + if (!ref && vTree) { target = Internals.NodeCache.get(vTree); ref = vTree.attributes.ref; } @@ -40,7 +40,7 @@ export const invokeRef = (target = EMPTY.OBJ, vTree, value) => { * @param {HTMLElement | VTree | null} value - Value to populate ref with */ export function invokeRefsForVTree(vTree, value) { - const componentTree = ComponentTreeCache.get(vTree); + console.log('invoke for', vTree); if (vTree.childNodes.length) { vTree.childNodes.filter(Boolean).forEach(childNode => { @@ -48,7 +48,7 @@ export function invokeRefsForVTree(vTree, value) { }); } - const instance = InstanceCache.get(componentTree || vTree); + const instance = InstanceCache.get(vTree); if (!instance) { invokeRef(Internals.NodeCache.get(vTree), vTree, value); diff --git a/packages/diffhtml-components/lib/middleware.js b/packages/diffhtml-components/lib/middleware.js index 5c017292..36cff040 100644 --- a/packages/diffhtml-components/lib/middleware.js +++ b/packages/diffhtml-components/lib/middleware.js @@ -1,6 +1,4 @@ import { - EMPTY, - ComponentTreeCache, MountCache, VTree, Transaction, @@ -16,51 +14,6 @@ import renderComponent from './render-component'; const { assign } = Object; const { tasks } = diff.Internals; -/** - * @param {VTree} oldTree - * @param {VTree} newTree - * @param {Transaction} transaction - * - * @returns {VTree | null} - */ -function render(oldTree, newTree, transaction) { - let oldComponentTree = null; - - // When there is an oldTree and it has childNodes, attempt to look up first - // by the top-level element, or by the first element. - if (oldTree) { - // First try and lookup the old tree as a component. - oldComponentTree = ComponentTreeCache.get(oldTree); - - // If that fails, try looking up its first child. - if (!oldComponentTree && oldTree.childNodes) { - oldComponentTree = ComponentTreeCache.get(oldTree.childNodes[0]); - } - - if (!oldComponentTree && typeof oldTree.rawNodeName === 'function') { - oldComponentTree = oldTree; - } - } - - - // If there is no old component, or if the components do not match, then we - // are rendering a brand new component. - if (!oldComponentTree || oldComponentTree.rawNodeName !== newTree.rawNodeName) { - return renderComponent(newTree, transaction); - } - - // Otherwise re-use the existing component if the constructors are the same. - if (oldComponentTree) { - - // Update the incoming props/attrs. - assign(oldComponentTree.attributes, newTree.attributes); - - return renderComponent(oldComponentTree, transaction); - } - - return oldTree; -} - /** * @param {VTree} vTree */ @@ -95,61 +48,22 @@ const createNodeHook = vTree => { }; /** + * This hook determines which component to render and inject into the tree. + * * @param {VTree} oldTree * @param {VTree} newTree * @param {Transaction} transaction */ const syncTreeHook = (oldTree, newTree, transaction) => { - // Render components during synchronization. - if ( - // If there is an oldTree and it's not the existing component, trigger a - // render. Or if the components match, re-render. - (oldTree && typeof oldTree.rawNodeName === 'function') || - (newTree && typeof newTree.rawNodeName === 'function') - ) { - return render(oldTree, newTree, transaction); - } + const isOldFunction = oldTree && typeof oldTree.rawNodeName === 'function'; + const isNewFunction = newTree && typeof newTree.rawNodeName === 'function'; - if (!newTree.childNodes) { - return oldTree; + // New component added (TBD what about keyed components?) + if (!isOldFunction && isNewFunction) { + return renderComponent(newTree, transaction); } - - // Loop through childNodes seeking out components to render. - for (let i = 0; i < newTree.childNodes.length; i++) { - const newChildTree = newTree.childNodes[i]; - const oldChildTree = (oldTree.childNodes && oldTree.childNodes[i]) || EMPTY.OBJ; - const isNewFunction = typeof newChildTree.rawNodeName === 'function'; - - // Search through the DOM tree for more components to render. - if (isNewFunction) { - const renderTree = render(oldChildTree, newChildTree, transaction); - - // If nothing was rendered, return the oldTree. - if (!renderTree) { - return oldTree; - } - - // Inject the rendered tree into the position. - if (renderTree) { - newTree.childNodes[i] = renderTree; - - // If the rendered tree is a fragment, splice in the children, as this - // is simply a container for the nodes. - if (renderTree.nodeType === 11) { - // If a function was returned, re-run the inspection over this - // element. - if (typeof renderTree.rawNodeName === 'function') { - i = i - 1; - } - // Replace the fragment with the rendered elements. This reduces and - // flattens the fragments into their respective nodes. If there are - // none, then they are removed from the DOM and nothing is rendered. - else { - newTree.childNodes.splice(i, 1, ...renderTree.childNodes); - } - } - } - } + else if (isOldFunction && isNewFunction) { + console.log('here'); } }; @@ -171,10 +85,11 @@ export default () => assign( if (transaction.tasks.includes(tasks.patchNode)) { const patchNodeIndex = transaction.tasks.indexOf(tasks.patchNode); + // TODO Can this be implemented elsewhere? transaction.tasks.splice(patchNodeIndex + 1, 0, function afterMountLifecycle() { MountCache.get(transaction)?.forEach(instance => { invokeRef(instance, instance[$$vTree]); - + if (typeof instance.componentDidMount === 'function') { instance.componentDidMount(); } diff --git a/packages/diffhtml-components/lib/render-component.js b/packages/diffhtml-components/lib/render-component.js index cc99f69b..048f4090 100644 --- a/packages/diffhtml-components/lib/render-component.js +++ b/packages/diffhtml-components/lib/render-component.js @@ -1,23 +1,25 @@ import { MountCache, - ComponentTreeCache, ActiveRenderState, InstanceCache, VTree, Transaction, } from './util/types'; -import { $$hooks, $$vTree } from './util/symbols'; +import { $$children, $$hooks, $$vTree } from './util/symbols'; import diff from './util/binding'; import Component from './component'; -const { createTree } = diff; +const { createTree, Internals } = diff; /** * Used during a synchronization flow. Takes in a vTree and a context object * and renders the component as a class or a function. Calls standard lifecycle * methods. + * + * This is a recursive function and will continue to render until all components + * are done or a non-component VTree is hit. * - * @param {VTree} vTree - tree to render + * @param {VTree} vTree - VTree to render * @param {Transaction} transaction - used to key mounts to a transaction * * @returns {VTree | null} @@ -28,8 +30,6 @@ export default function renderComponent(vTree, transaction) { const isNewable = RawComponent.prototype && RawComponent.prototype.render; const isInstance = InstanceCache.has(vTree); - console.log('Rendering', vTree, InstanceCache.get(vTree)); - /** @type {VTree|null} */ let renderedTree = (null); @@ -51,16 +51,18 @@ export default function renderComponent(vTree, transaction) { renderedTree = createTree(instance.render(props, instance.state)); ActiveRenderState.length = 0; + if (instance.componentDidUpdate && instance.componentDidUpdate) { instance.componentDidUpdate(instance.props, instance.state); } } else { - return null; + return renderedTree; } } // New class instance. else if (isNewable) { + /** @type {Component} */ const instance = new RawComponent(props); // Associate the instance to the vTree. @@ -80,19 +82,13 @@ export default function renderComponent(vTree, transaction) { if ( typeof renderedTree.rawNodeName !== 'function' && - renderedTree.nodeType === 11 && + renderedTree.nodeType === Internals.NODE_TYPE.FRAGMENT && !renderedTree.childNodes.length ) { renderedTree = createTree('#text'); } - // Set up the HoC parent/child relationship. - if (typeof renderedTree.rawNodeName === 'function') { - ComponentTreeCache.set(renderedTree, vTree); - } - - // Associate the instance with the vTree. - instance[$$vTree] = vTree; + instance[$$children] = renderedTree; } // Function component, upgrade to a class to make it reactive. else { @@ -113,7 +109,7 @@ export default function renderComponent(vTree, transaction) { } /** @type {VTree | null} */ - [$$vTree] = null; + [$$vTree] = vTree; } const instance = new FunctionComponent(props) @@ -146,32 +142,22 @@ export default function renderComponent(vTree, transaction) { // text value. if ( typeof renderedTree.rawNodeName !== 'function' && - renderedTree.nodeType === 11 && + renderedTree.nodeType === Internals.NODE_TYPE.FRAGMENT && !renderedTree.childNodes.length ) { renderedTree = createTree('#text'); } - // Set up the HoC parent/child relationship. - if (typeof renderedTree.rawNodeName === 'function') { - ComponentTreeCache.set(renderedTree, vTree); - } - // Associate the instance with the vTree. - instance[$$vTree] = vTree; + instance[$$children] = renderedTree; } - if (renderedTree !== vTree) { - // Map all top-level fragment elements to this parent node. - if (renderedTree.nodeType === 11) { - renderedTree.childNodes.forEach(childNode => { - ComponentTreeCache.set(childNode, vTree); - }); - } - // Otherwise directly map the rendered VTree to the component tree. - else { - ComponentTreeCache.set(renderedTree, vTree); - } + Internals.memory.protectVTree(renderedTree); + + // If a new component was rendered instead of a DOM node, we need to continue rendering + // until we reach the end. + if (renderedTree !== vTree && typeof renderedTree.rawNodeName === 'function') { + return renderComponent(renderedTree, transaction); } return renderedTree; diff --git a/packages/diffhtml-components/lib/util/internals.js b/packages/diffhtml-components/lib/util/internals.js index f955a134..0dc03cc9 100644 --- a/packages/diffhtml-components/lib/util/internals.js +++ b/packages/diffhtml-components/lib/util/internals.js @@ -1,6 +1,6 @@ -import { ComponentTreeCache, InstanceCache } from './types'; +import { MountCache, InstanceCache } from './types'; export const caches = { - ComponentTreeCache, InstanceCache, + MountCache, }; \ No newline at end of file diff --git a/packages/diffhtml-components/lib/util/symbols.js b/packages/diffhtml-components/lib/util/symbols.js index 0540fcff..36281e3c 100644 --- a/packages/diffhtml-components/lib/util/symbols.js +++ b/packages/diffhtml-components/lib/util/symbols.js @@ -6,3 +6,4 @@ export const $$type = Symbol.for('diff.type'); export const $$hooks = Symbol.for('diff.hooks'); export const $$insertAfter = Symbol.for('diff.after'); export const $$diffHTML = Symbol.for('diffHTML'); +export const $$children = Symbol.for('diff.children'); \ No newline at end of file diff --git a/packages/diffhtml-components/lib/util/types.js b/packages/diffhtml-components/lib/util/types.js index d344af05..fc2f2744 100644 --- a/packages/diffhtml-components/lib/util/types.js +++ b/packages/diffhtml-components/lib/util/types.js @@ -14,8 +14,6 @@ export const EMPTY = { */ export const ActiveRenderState = []; -export const ComponentTreeCache = new Map(); - /** * @typedef {Map} InstanceCache * @type {InstanceCache} diff --git a/packages/diffhtml-components/test/component.js b/packages/diffhtml-components/test/component.js index 69728b6b..46625bef 100644 --- a/packages/diffhtml-components/test/component.js +++ b/packages/diffhtml-components/test/component.js @@ -2,7 +2,8 @@ import { strictEqual, deepStrictEqual } from 'assert'; import Component from '../lib/component'; import diff from '../lib/util/binding'; import globalThis from '../lib/util/global'; -import { ComponentTreeCache } from '../lib/util/types'; +import { $$children } from '../lib/util/symbols'; +import { InstanceCache } from '../lib/util/types'; import validateCaches from './util/validate-caches'; const { html, release, innerHTML, toString, createTree } = diff; @@ -185,7 +186,7 @@ describe('Component', function() { strictEqual(actual, '
value
'); }); - it('will correctly render an empty text node when a falsy component is rendered', () => { + it('will render an empty text node when a falsy component is rendered', () => { { class TestComponent extends Component { render() { @@ -196,8 +197,7 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, TestComponent); - const componentVTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(componentVTree.rawNodeName, TestComponent); + strictEqual(this.fixture.childNodes[0].rawNodeName, '#text'); release(this.fixture); } { @@ -210,8 +210,7 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, TestComponent); - const componentVTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(componentVTree.rawNodeName, TestComponent); + strictEqual(this.fixture.childNodes[0].rawNodeName, '#text'); release(this.fixture); } { @@ -224,8 +223,7 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, TestComponent); - const componentVTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(componentVTree.rawNodeName, TestComponent); + strictEqual(this.fixture.childNodes[0].rawNodeName, '#text'); release(this.fixture); } }); @@ -240,8 +238,8 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, TestComponent); - const componentVTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(componentVTree.rawNodeName, TestComponent); + const componentVTree = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(componentVTree.constructor, TestComponent); }); it('will associate the first element from the start of a fragment', () => { @@ -257,9 +255,8 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, TestComponent); - const componentVTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(this.fixture.childNodes[0].nodeName, '#text'); - strictEqual(componentVTree.rawNodeName, TestComponent); + const instance = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(instance.constructor, TestComponent); }); it('will associate a nested component', () => { @@ -282,11 +279,11 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, Level1Component); - const level1VTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(level1VTree.rawNodeName, Level1Component); + const level1Instance = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(level1Instance.constructor, Level1Component); - const level2VTree = ComponentTreeCache.get(this.fixture.childNodes[1]); - strictEqual(level2VTree.rawNodeName, Level2Component); + const level2Instance = InstanceCache.get(this.fixture.childNodes[0].childNodes[1]); + strictEqual(level2Instance.constructor, Level2Component); }); it('will associate two nested components', () => { @@ -316,14 +313,14 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, Level1Component); - const level1VTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(level1VTree.rawNodeName, Level1Component); + const level1Instance = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(level1Instance.constructor, Level1Component); - const level2VTree = ComponentTreeCache.get(this.fixture.childNodes[1]); - strictEqual(level2VTree.rawNodeName, Level2Component); + const level2Instance = InstanceCache.get(this.fixture.childNodes[0].childNodes[1]); + strictEqual(level2Instance.constructor, Level2Component); - const level3VTree = ComponentTreeCache.get(this.fixture.childNodes[2]); - strictEqual(level3VTree.rawNodeName, Level3Component); + const level3Instance = InstanceCache.get(this.fixture.childNodes[0].childNodes[1].childNodes[1]); + strictEqual(level3Instance.constructor, Level3Component); }); it('will associate a nested function component', () => { @@ -344,11 +341,11 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, Level1Component); - const level1VTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(level1VTree.rawNodeName, Level1Component); + const level1Instance = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(level1Instance.constructor, Level1Component); - const level2VTree = ComponentTreeCache.get(this.fixture.childNodes[1]); - strictEqual(level2VTree.rawNodeName, Level2Component); + const level2Instance = InstanceCache.get(this.fixture.childNodes[0].childNodes[1]); + strictEqual(level2Instance.constructor.name, 'FunctionComponent'); }); it('will associate a nested function component when passed directly', () => { @@ -367,12 +364,11 @@ describe('Component', function() { this.fixture = createTree('div'); innerHTML(this.fixture, Level1Component); - // Always the most inner component rendered and work outwards. - const level2VTree = ComponentTreeCache.get(this.fixture.childNodes[0]); - strictEqual(level2VTree.rawNodeName, Level2Component); + const level1Instance = InstanceCache.get(this.fixture.childNodes[0]); + strictEqual(level1Instance.constructor, Level1Component); - const level1VTree = ComponentTreeCache.get(level2VTree); - strictEqual(level1VTree.rawNodeName, Level1Component); + const level2Instance = InstanceCache.get(level1Instance[$$children]); + strictEqual(level2Instance.constructor.name, 'FunctionComponent'); }); }); diff --git a/packages/diffhtml-components/test/integration/hooks.js b/packages/diffhtml-components/test/integration/hooks.js index 5685b306..1a25d198 100644 --- a/packages/diffhtml-components/test/integration/hooks.js +++ b/packages/diffhtml-components/test/integration/hooks.js @@ -3,7 +3,6 @@ import { createSideEffect } from '../../lib/create-side-effect'; import { createState } from '../../lib/create-state'; import diff from '../../lib/util/binding'; import globalThis from '../../lib/util/global'; -import { ComponentTreeCache } from '../../lib/util/types'; import validateCaches from '../util/validate-caches'; const { html, release, innerHTML, toString, createTree } = diff; @@ -450,7 +449,7 @@ describe('Hooks', function() { i++; const [ value, _setValue ] = createState(false); setNestedValue = _setValue; - return html`${String(value) + i}`; + return html`${String(value)} ${i}`; } function Component() { @@ -462,12 +461,14 @@ describe('Hooks', function() { this.fixture = document.createElement('div'); await innerHTML(this.fixture, html`<${Component} />`); + strictEqual(this.fixture.outerHTML, `
false 1
`); + console.log('>> set first value'); await setNestedValue(123); - strictEqual(this.fixture.outerHTML, `
1232
`); + strictEqual(this.fixture.outerHTML, `
123 2
`); console.log('>>> set component value <<<'); await setComponentValue(); - strictEqual(this.fixture.outerHTML, `
1233
`); + strictEqual(this.fixture.outerHTML, `
123 3
`); }); it('will support nested createSideEffect with top-level re-rendering', async () => { diff --git a/packages/diffhtml-components/test/util/validate-caches.js b/packages/diffhtml-components/test/util/validate-caches.js index 95832e6a..ee5ba0df 100644 --- a/packages/diffhtml-components/test/util/validate-caches.js +++ b/packages/diffhtml-components/test/util/validate-caches.js @@ -2,7 +2,6 @@ import { strictEqual } from 'assert'; import diff from '../../lib/util/binding'; import { ActiveRenderState, - ComponentTreeCache, InstanceCache, MountCache, } from '../../lib/util/types'; @@ -12,7 +11,6 @@ import { */ export default function validateCaches() { strictEqual(ActiveRenderState.length, 0, 'The ActiveRenderState global should be empty'); - strictEqual(ComponentTreeCache.size, 0, 'The ComponentTree cache should be empty'); strictEqual(InstanceCache.size, 0, 'The Instance cache should be empty'); strictEqual(MountCache.size, 0, 'The Mount cache should be empty'); diff --git a/packages/diffhtml/lib/html.js b/packages/diffhtml/lib/html.js index 8bc6da92..9d8667ee 100644 --- a/packages/diffhtml/lib/html.js +++ b/packages/diffhtml/lib/html.js @@ -70,10 +70,13 @@ const interpolateAndFlatten = (childNode, supplemental) => { // Attributes for (const keyName of getOwnPropertyNames(childNode.attributes)) { - keyName.split(' ').forEach(keyName => { - const value = childNode.attributes[keyName]; + const keyNames = keyName.split(' '); + + for (let i = 0; i < keyNames.length; i++) { + const name = keyNames[i]; + const value = childNode.attributes[name]; let newValue = value; - let newKey = keyName; + let newKey = name; // Check for dynamic value and assign to newValue. if (match = tokenEx.exec(value)) { @@ -99,8 +102,8 @@ const interpolateAndFlatten = (childNode, supplemental) => { } // Check for dynamic key and assign to newKey. - if (match = tokenEx.exec(keyName)) { - const parts = keyName.split(tokenEx); + if (match = tokenEx.exec(name)) { + const parts = name.split(tokenEx); for (let i = 0; i < parts.length; i++) { if (i % 2 !== 0) { @@ -116,15 +119,15 @@ const interpolateAndFlatten = (childNode, supplemental) => { throw new Error('Arrays cannot be spread as attributes'); } - delete childNode.attributes[keyName]; + delete childNode.attributes[name]; } else { - delete childNode.attributes[keyName]; + delete childNode.attributes[name]; Object.assign(childNode.attributes, newKey); } } else { - delete childNode.attributes[keyName]; + delete childNode.attributes[name]; if (newKey === 'childNodes') { childNode.childNodes.length = 0; @@ -147,7 +150,7 @@ const interpolateAndFlatten = (childNode, supplemental) => { if (childNode.nodeName === 'script' && childNode.attributes.src) { childNode.key = childNode.attributes.src; } - }); + } } // Node value @@ -266,7 +269,9 @@ export default function handleTaggedTemplate(strings, ...values) { // in an object called supplemental and keyed by a incremental string token. // The following loop instruments the markup with these tokens that the // parser then uses to assemble the correct tree. - strings.forEach((string, i) => { + for (let i = 0; i < strings.length; i++) { + const string = strings[i]; + // Always add the string, we need it to parse the markup later. HTML += string; @@ -301,7 +306,7 @@ export default function handleTaggedTemplate(strings, ...values) { HTML += value; } } - }); + } // Parse the instrumented markup to get the Virtual Tree. const { childNodes } = Internals.parse(HTML); diff --git a/packages/diffhtml/lib/release.js b/packages/diffhtml/lib/release.js index 68cbe466..ec1023b8 100644 --- a/packages/diffhtml/lib/release.js +++ b/packages/diffhtml/lib/release.js @@ -1,5 +1,6 @@ /** * @typedef {import('./util/types').Mount} Mount + * @typedef {import('./util/types').VTree} VTree */ import { gc, unprotectVTree } from './util/memory'; import { StateCache, NodeCache, ReleaseHookCache } from './util/types'; @@ -74,6 +75,9 @@ export default function release(mount) { } }); + // In the case that mount is a protected VTree, it should be removed as well. + unprotectVTree(/** @type {VTree} **/ (mount)); + // Schedule a gc(), this is a global interval. cancelTimeout(gcTimerId); gcTimerId = /** @type {number} */(scheduleTimeout(gc)); diff --git a/packages/diffhtml/lib/tasks/sync-trees.js b/packages/diffhtml/lib/tasks/sync-trees.js index 559cd14a..62f25cc7 100644 --- a/packages/diffhtml/lib/tasks/sync-trees.js +++ b/packages/diffhtml/lib/tasks/sync-trees.js @@ -40,11 +40,11 @@ export default function syncTrees(/** @type {Transaction} */ transaction) { } // Replace the top level elements. - transaction.patches = [ + transaction.patches.push( PATCH_TYPE.REPLACE_CHILD, newTree, oldTree, - ]; + ); // Clean up the existing old tree, and mount the new tree. transaction.oldTree = state.oldTree = newTree; @@ -63,13 +63,17 @@ export default function syncTrees(/** @type {Transaction} */ transaction) { } // Synchronize the top level elements. else { - transaction.patches = syncTree( + const patches = syncTree( oldTree || null, newTree || null, [], state, transaction, ); + + if (patches && patches.length) { + transaction.patches.push(...patches); + } } measure('sync trees'); diff --git a/packages/diffhtml/lib/transaction.js b/packages/diffhtml/lib/transaction.js index 81cf763d..210a49d2 100644 --- a/packages/diffhtml/lib/transaction.js +++ b/packages/diffhtml/lib/transaction.js @@ -209,6 +209,9 @@ export default class Transaction { // Clean up SVG element list. svgElements.clear(); + // Reset patches once completed. + this.patches.length = 0; + // Rendering is complete. state.isRendering = false; state.isDirty = false; diff --git a/packages/diffhtml/lib/tree/sync.js b/packages/diffhtml/lib/tree/sync.js index 9ba8c293..68362b61 100644 --- a/packages/diffhtml/lib/tree/sync.js +++ b/packages/diffhtml/lib/tree/sync.js @@ -300,7 +300,15 @@ export default function syncTree( } const sameType = oldChildNode.nodeName === newChildNode.nodeName; - const retVal = syncTree(oldChildNode, newChildNode, patches, state, transaction, !sameType); + + const retVal = syncTree( + oldChildNode, + newChildNode, + patches, + state, + transaction, + !sameType, + ); if (retVal === false) { newChildNodes.splice(i, 0, oldChildNode); From eb9522d634ba8f9b970c86a04ba0dc5a3eadd8c7 Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Sun, 8 Jan 2023 15:54:35 -0800 Subject: [PATCH 4/4] Start reworking component implementation The current implementation is broken and doesn't work consistently which results in broken UI. This is an initial attempt to rework how stateful components are re-rendered. The work isn't complete and this is not yet ready. If you are interested in component architecture and systems and want to lend a hand it would be greatly appreciated. For the most part this functions correctly, but there are lots of edge cases that still need to be ironed out. Since this is of upmost importance to shipping a release candidate, moreso than any other remaining work, I will be focusing on this until it is completed. --- packages/diffhtml-components/lib/component.js | 43 +++---- .../diffhtml-components/lib/create-state.js | 2 - .../lib/{ => lifecycle}/before-mount.js | 12 +- .../lib/lifecycle/component-will-unmount.js | 8 +- .../lib/lifecycle/invoke-refs.js | 2 - .../lib/{ => lifecycle}/middleware.js | 30 +++-- .../lib/render-component.js | 39 ++++-- .../diffhtml-components/lib/util/symbols.js | 3 +- .../diffhtml-components/test/component.js | 85 ++++++++++++- .../test/integration/component.js | 52 +++++++- .../test/util/validate-caches.js | 2 + packages/diffhtml/lib/tasks/sync-trees.js | 8 +- packages/diffhtml/lib/transaction.js | 3 - packages/diffhtml/lib/tree/sync.js | 56 ++++++--- packages/diffhtml/lib/util/types.js | 1 + packages/diffhtml/test/tree.js | 119 ++++++++++++------ 16 files changed, 335 insertions(+), 130 deletions(-) rename packages/diffhtml-components/lib/{ => lifecycle}/before-mount.js (82%) rename packages/diffhtml-components/lib/{ => lifecycle}/middleware.js (81%) diff --git a/packages/diffhtml-components/lib/component.js b/packages/diffhtml-components/lib/component.js index 26527c3f..71126ef9 100644 --- a/packages/diffhtml-components/lib/component.js +++ b/packages/diffhtml-components/lib/component.js @@ -13,12 +13,11 @@ import { $$unsubscribe, $$type, $$hooks, - $$children, } from './util/symbols'; import diff from './util/binding'; -import middleware from './middleware'; +import middleware from './lifecycle/middleware'; -const { innerHTML } = diff; +const { Internals } = diff; const { isArray } = Array; const { setPrototypeOf, defineProperty, keys, assign } = Object; const RenderDebounce = new WeakMap(); @@ -67,7 +66,7 @@ const createMountPoint = (instance, isWebComponent) => { return /** @type {any} */(instance).shadowRoot; } else { - return instance[$$children]; + return instance[$$vTree]; } }; @@ -226,9 +225,6 @@ export default class Component { /** @type {{ fns: Function[], i: number }} */ [$$hooks] = { fns: [], i: 0 }; - /** @type {any} */ - [$$children] = []; - /** * Stateful render. Used when a component changes and needs to re-render * itself and the underlying tree it contains. This is triggered on @@ -245,10 +241,6 @@ export default class Component { */ [$$render]() { const vTree = /** @type {VTree=} */ this[$$vTree]; - if (!vTree) { - throw new Error('Missing required VTree to re-render'); - } - const oldProps = this.props; const oldState = this.state; @@ -280,22 +272,31 @@ export default class Component { this[$$hooks].i = 0; } - // Set the VTree attributes to match the current props. - Object.assign(vTree.attributes, this.props); + const rendered = this.render(this.props, this.state); + + const transaction = Internals.Transaction.create( + mount, + rendered, + { inner: true }, + ); - // Compare the newly rendered component contents into the mount. - const transaction = innerHTML(mount, this.render(this.props, this.state)); + // Set the VTree attributes to match the current props and use this as the initial state for a re-render. + if (vTree) { + assign(vTree.attributes, this.props); + transaction.state.oldTree = vTree; + transaction.state.isDirty = false; + } + + // Middleware and task changes can affect the return value, it's not always guarenteed to be the transaction + // this allows us to tap into that return value and remain consistent. + const retVal = transaction.start(); // Reset the active state after rendering so we don't accidentally bleed // into other components render cycle. ActiveRenderState.length = 0; - if (!isWebComponent) { - release(mount); - } - this.componentDidUpdate(oldProps, oldState); - return transaction; + return retVal; } connectedCallback() { @@ -304,7 +305,7 @@ export default class Component { // This callback gets called during replace operations, there is no point // in re-rendering or creating a new shadow root due to this. - // Always do a full render when mounting, so that something is visible. + // This is the initial render for the Web Component this[$$render](); this.componentDidMount(); diff --git a/packages/diffhtml-components/lib/create-state.js b/packages/diffhtml-components/lib/create-state.js index eb23f834..555b62e7 100644 --- a/packages/diffhtml-components/lib/create-state.js +++ b/packages/diffhtml-components/lib/create-state.js @@ -25,8 +25,6 @@ export function createState(defaultValue = {}) { const currentValue = activeHook ? activeHook[0] : defaultValue; const retVal = activeHook || [currentValue]; - //console.log(activeComponent.toString(), currentValue.toString()); - /** * @param {any} newValue */ diff --git a/packages/diffhtml-components/lib/before-mount.js b/packages/diffhtml-components/lib/lifecycle/before-mount.js similarity index 82% rename from packages/diffhtml-components/lib/before-mount.js rename to packages/diffhtml-components/lib/lifecycle/before-mount.js index 2d15eb40..9d342910 100644 --- a/packages/diffhtml-components/lib/before-mount.js +++ b/packages/diffhtml-components/lib/lifecycle/before-mount.js @@ -1,9 +1,9 @@ -import componentWillUnmount from './lifecycle/component-will-unmount'; -import { invokeRef, invokeRefsForVTree } from './lifecycle/invoke-refs'; -import diff from './util/binding'; -import { Transaction } from './util/types'; +import componentWillUnmount from './component-will-unmount'; +import { invokeRef, invokeRefsForVTree } from './invoke-refs'; +import diff from '../util/binding'; +import { Transaction } from '../util/types'; -const { createNode, NodeCache, PATCH_TYPE, decodeEntities } = diff.Internals; +const { NodeCache, PATCH_TYPE, decodeEntities, createNode } = diff.Internals; const uppercaseEx = /[A-Z]/g; /** @@ -53,6 +53,7 @@ export default transaction => { } } + // TBD Remove because invokeRefs is handled in afterMount if (name === 'ref') { invokeRef(createNode(vTree), vTree); } @@ -69,6 +70,7 @@ export default transaction => { case PATCH_TYPE.REPLACE_CHILD: { const oldTree = patches[i + 2]; + invokeRefsForVTree(oldTree, null); componentWillUnmount(oldTree); i += 3; diff --git a/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js b/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js index 24b39c56..24c5e1f2 100644 --- a/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js +++ b/packages/diffhtml-components/lib/lifecycle/component-will-unmount.js @@ -1,6 +1,6 @@ import { InstanceCache, VTree } from '../util/types'; import diff from '../util/binding'; -import { $$children, $$hooks } from '../util/symbols'; +import { $$hooks } from '../util/symbols'; const { release, Internals } = diff; @@ -17,6 +17,9 @@ export default function componentWillUnmount(vTree) { if (domNode && /** @type {any} */ (domNode).shadowRoot) { release(/** @type {any} */ (domNode).shadowRoot); } + else { + Internals.memory.unprotectVTree(vTree); + } for (let i = 0; i < vTree.childNodes.length; i++) { componentWillUnmount(vTree.childNodes[i]); @@ -36,9 +39,6 @@ export default function componentWillUnmount(vTree) { instance[$$hooks].i = 0; } - // Ensure children are released as well. - release(instance[$$children]); - // Ensure this is a stateful component. Stateless components do not get // lifecycle events yet. instance && instance.componentWillUnmount && instance.componentWillUnmount(); diff --git a/packages/diffhtml-components/lib/lifecycle/invoke-refs.js b/packages/diffhtml-components/lib/lifecycle/invoke-refs.js index 189b944f..2ae19837 100644 --- a/packages/diffhtml-components/lib/lifecycle/invoke-refs.js +++ b/packages/diffhtml-components/lib/lifecycle/invoke-refs.js @@ -40,8 +40,6 @@ export const invokeRef = (target = EMPTY.OBJ, vTree, value) => { * @param {HTMLElement | VTree | null} value - Value to populate ref with */ export function invokeRefsForVTree(vTree, value) { - console.log('invoke for', vTree); - if (vTree.childNodes.length) { vTree.childNodes.filter(Boolean).forEach(childNode => { invokeRefsForVTree(childNode, value); diff --git a/packages/diffhtml-components/lib/middleware.js b/packages/diffhtml-components/lib/lifecycle/middleware.js similarity index 81% rename from packages/diffhtml-components/lib/middleware.js rename to packages/diffhtml-components/lib/lifecycle/middleware.js index 36cff040..b5db943a 100644 --- a/packages/diffhtml-components/lib/middleware.js +++ b/packages/diffhtml-components/lib/lifecycle/middleware.js @@ -2,14 +2,15 @@ import { MountCache, VTree, Transaction, -} from './util/types'; -import globalThis from './util/global'; -import { $$vTree } from './util/symbols'; -import diff from './util/binding'; + InstanceCache, +} from '../util/types'; +import globalThis from '../util/global'; +import { $$vTree } from '../util/symbols'; +import diff from '../util/binding'; import beforeMount from './before-mount'; -import componentWillUnmount from './lifecycle/component-will-unmount'; -import { invokeRef } from './lifecycle/invoke-refs'; -import renderComponent from './render-component'; +import componentWillUnmount from './component-will-unmount'; +import { invokeRef } from './invoke-refs'; +import renderComponent from '../render-component'; const { assign } = Object; const { tasks } = diff.Internals; @@ -17,7 +18,9 @@ const { tasks } = diff.Internals; /** * @param {VTree} vTree */ -const releaseHook = vTree => componentWillUnmount(vTree); +const releaseHook = vTree => { + componentWillUnmount(vTree); +} /** * @param {VTree} vTree @@ -49,13 +52,13 @@ const createNodeHook = vTree => { /** * This hook determines which component to render and inject into the tree. - * + * * @param {VTree} oldTree * @param {VTree} newTree * @param {Transaction} transaction */ const syncTreeHook = (oldTree, newTree, transaction) => { - const isOldFunction = oldTree && typeof oldTree.rawNodeName === 'function'; + const isOldFunction = oldTree && InstanceCache.has(oldTree); const isNewFunction = newTree && typeof newTree.rawNodeName === 'function'; // New component added (TBD what about keyed components?) @@ -63,7 +66,10 @@ const syncTreeHook = (oldTree, newTree, transaction) => { return renderComponent(newTree, transaction); } else if (isOldFunction && isNewFunction) { - console.log('here'); + if (InstanceCache.get(oldTree).constructor === newTree.rawNodeName) { + return renderComponent(oldTree, transaction); + } + throw new Error('Currently unimplemented'); } }; @@ -89,7 +95,7 @@ export default () => assign( transaction.tasks.splice(patchNodeIndex + 1, 0, function afterMountLifecycle() { MountCache.get(transaction)?.forEach(instance => { invokeRef(instance, instance[$$vTree]); - + if (typeof instance.componentDidMount === 'function') { instance.componentDidMount(); } diff --git a/packages/diffhtml-components/lib/render-component.js b/packages/diffhtml-components/lib/render-component.js index 048f4090..68759c82 100644 --- a/packages/diffhtml-components/lib/render-component.js +++ b/packages/diffhtml-components/lib/render-component.js @@ -5,7 +5,7 @@ import { VTree, Transaction, } from './util/types'; -import { $$children, $$hooks, $$vTree } from './util/symbols'; +import { $$hooks, $$vTree } from './util/symbols'; import diff from './util/binding'; import Component from './component'; @@ -15,7 +15,7 @@ const { createTree, Internals } = diff; * Used during a synchronization flow. Takes in a vTree and a context object * and renders the component as a class or a function. Calls standard lifecycle * methods. - * + * * This is a recursive function and will continue to render until all components * are done or a non-component VTree is hit. * @@ -46,15 +46,20 @@ export default function renderComponent(vTree, transaction) { instance.shouldComponentUpdate(props, instance.state) ) { ActiveRenderState.push(instance); + // Reset the hooks iterator. instance[$$hooks].i = 0; renderedTree = createTree(instance.render(props, instance.state)); - ActiveRenderState.length = 0; + ActiveRenderState.length = 0; if (instance.componentDidUpdate && instance.componentDidUpdate) { instance.componentDidUpdate(instance.props, instance.state); } + + instance[$$vTree] = vTree; + vTree.childNodes.length = 0; + vTree.childNodes.push(renderedTree); } else { return renderedTree; @@ -88,7 +93,16 @@ export default function renderComponent(vTree, transaction) { renderedTree = createTree('#text'); } - instance[$$children] = renderedTree; + // Replace the VTree with the rendered component + if (renderedTree.nodeType === Internals.NODE_TYPE.FRAGMENT) { + vTree.childNodes.push(...renderedTree.childNodes); + } + else { + vTree.childNodes.push(renderedTree); + } + + InstanceCache.set(renderedTree, instance); + instance[$$vTree] = vTree; } // Function component, upgrade to a class to make it reactive. else { @@ -149,15 +163,20 @@ export default function renderComponent(vTree, transaction) { } // Associate the instance with the vTree. - instance[$$children] = renderedTree; + // Replace the VTree with the rendered component + instance[$$vTree] = renderedTree; + vTree.childNodes.push(renderedTree); + + InstanceCache.set(renderedTree, instance); } - Internals.memory.protectVTree(renderedTree); + Internals.memory.protectVTree(vTree); - // If a new component was rendered instead of a DOM node, we need to continue rendering - // until we reach the end. - if (renderedTree !== vTree && typeof renderedTree.rawNodeName === 'function') { - return renderComponent(renderedTree, transaction); + // If a new component was rendered instead of a DOM node, we need to continue + // rendering until we reach the end. + if (typeof renderedTree.rawNodeName === 'function') { + vTree.childNodes.length = 0; + vTree.childNodes.push(renderComponent(renderedTree, transaction)); } return renderedTree; diff --git a/packages/diffhtml-components/lib/util/symbols.js b/packages/diffhtml-components/lib/util/symbols.js index 36281e3c..290a73ad 100644 --- a/packages/diffhtml-components/lib/util/symbols.js +++ b/packages/diffhtml-components/lib/util/symbols.js @@ -5,5 +5,4 @@ export const $$unsubscribe = Symbol.for('diff.unsubscribe'); export const $$type = Symbol.for('diff.type'); export const $$hooks = Symbol.for('diff.hooks'); export const $$insertAfter = Symbol.for('diff.after'); -export const $$diffHTML = Symbol.for('diffHTML'); -export const $$children = Symbol.for('diff.children'); \ No newline at end of file +export const $$diffHTML = Symbol.for('diffHTML'); \ No newline at end of file diff --git a/packages/diffhtml-components/test/component.js b/packages/diffhtml-components/test/component.js index 46625bef..ff363fde 100644 --- a/packages/diffhtml-components/test/component.js +++ b/packages/diffhtml-components/test/component.js @@ -2,7 +2,7 @@ import { strictEqual, deepStrictEqual } from 'assert'; import Component from '../lib/component'; import diff from '../lib/util/binding'; import globalThis from '../lib/util/global'; -import { $$children } from '../lib/util/symbols'; +import { $$type, $$vTree } from '../lib/util/symbols'; import { InstanceCache } from '../lib/util/types'; import validateCaches from './util/validate-caches'; @@ -27,6 +27,23 @@ describe('Component', function() { strictEqual(TestComponent.name, 'TestComponent'); }); + it('will inherit from HTMLElement', () => { + class TestComponent extends Component {} + strictEqual(TestComponent.prototype.toString(), '[object HTMLElement]'); + }); + + it('will set the internal type to class when not a web component', () => { + class TestComponent extends Component {} + const instance = new TestComponent(); + strictEqual(instance[$$type], 'class'); + }); + + it('will set initial $$vTree reference to null', () => { + class TestComponent extends Component {} + const instance = new TestComponent(); + strictEqual(instance[$$vTree], null); + }); + describe('Default props', () => { it('will support defining default props as null', () => { class TestComponent extends Component { @@ -37,7 +54,7 @@ describe('Component', function() { deepStrictEqual(testComponent.props, {}); }); - it('will not support other types for default props', () => { + it('will not support non-object types for default props', () => { { class TestComponent extends Component { static defaultProps = 'test' @@ -115,13 +132,25 @@ describe('Component', function() { test: 'value', }); }); + + it('will support passing props', () => { + class TestComponent extends Component {} + + const testComponent = new TestComponent({ + test: 'value' + }); + + deepStrictEqual(testComponent.props, { + test: 'value', + }); + }); }); describe('render()', () => { it('will support omitting render function', () => { class TestComponent extends Component {} - const actual = toString(html`<${TestComponent} />`).trim(); + const actual = toString(TestComponent); strictEqual(actual, ''); }); @@ -131,7 +160,7 @@ describe('Component', function() { render() {} } - const actual = toString(html`<${TestComponent} />`).trim(); + const actual = toString(TestComponent); strictEqual(actual, ''); }); @@ -143,7 +172,7 @@ describe('Component', function() { } } - const actual = toString(TestComponent).trim(); + const actual = toString(TestComponent); strictEqual(actual, '
'); }); @@ -367,7 +396,7 @@ describe('Component', function() { const level1Instance = InstanceCache.get(this.fixture.childNodes[0]); strictEqual(level1Instance.constructor, Level1Component); - const level2Instance = InstanceCache.get(level1Instance[$$children]); + const level2Instance = InstanceCache.get(this.fixture.childNodes[0].childNodes[0]); strictEqual(level2Instance.constructor.name, 'FunctionComponent'); }); }); @@ -414,6 +443,46 @@ describe('Component', function() { }); describe('setState', () => { + it('will support re-rendering multiple times', async () => { + let renderCalled = []; + + class TestComponent extends Component { + render(...args) { + renderCalled.push(args); + const { message } = this.state; + return html` +
${message}
+ `; + } + } + + this.fixture = document.createElement('div'); + + let ref = null; + + innerHTML(this.fixture, html` + <${TestComponent} ref=${instance => ref = instance} /> + `); + + strictEqual(renderCalled.length, 1); + strictEqual(this.fixture.firstElementChild.outerHTML.trim(), '
'); + + await ref.setState({ message: 'test' }); + + strictEqual(renderCalled.length, 2); + strictEqual(this.fixture.firstElementChild.outerHTML.trim(), '
test
'); + + await ref.setState({ message: 'this' }); + + strictEqual(renderCalled.length, 3); + strictEqual(this.fixture.firstElementChild.outerHTML.trim(), '
this
'); + + await ref.setState({ message: 'out' }); + + strictEqual(renderCalled.length, 4); + strictEqual(this.fixture.firstElementChild.outerHTML.trim(), '
out
'); + }); + it('will trigger a re-render on mounted components', async () => { let renderCalled = []; @@ -433,6 +502,10 @@ describe('Component', function() { <${TestComponent} ref=${instance => ref = instance} /> `); + + strictEqual(renderCalled.length, 1); + strictEqual(this.fixture.firstElementChild.outerHTML, '
'); + await ref.setState({ message: 'test' }); strictEqual(renderCalled.length, 2); diff --git a/packages/diffhtml-components/test/integration/component.js b/packages/diffhtml-components/test/integration/component.js index 0cfba341..e82845ae 100644 --- a/packages/diffhtml-components/test/integration/component.js +++ b/packages/diffhtml-components/test/integration/component.js @@ -334,8 +334,8 @@ describe('Component implementation', function() { innerHTML(this.fixture, html`<${CustomComponent} someProp="true" />`); innerHTML(this.fixture, html`<${CustomComponent} someProp="false" />`); - ok(wasCalled); strictEqual(counter, 1); + ok(wasCalled); }); it('will map root changes to componentDidUpdate', () => { @@ -538,6 +538,52 @@ describe('Component implementation', function() { strictEqual(this.fixture.innerHTML, 'something'); }); + it('will call setState to re-render the component and update nested elements', async () => { + class CustomComponent extends Component { + render() { + const { message } = this.state; + return html`${message}`; + } + + constructor(props) { + super(props); + this.state.message = 'default' + } + } + + let ref = null; + + innerHTML(this.fixture, html`<${CustomComponent} ref=${node => (ref = node)} />`); + + strictEqual(this.fixture.innerHTML, 'default'); + await ref.setState({ message: 'something' }); + strictEqual(this.fixture.innerHTML, 'something'); + }); + + it('will call setState to re-render the component and update nested fragment', async () => { + class CustomComponent extends Component { + render() { + const { message } = this.state; + return html` + ${message} + `; + } + + constructor(props) { + super(props); + this.state.message = 'default' + } + } + + let ref = null; + + innerHTML(this.fixture, html`<${CustomComponent} ref=${node => (ref = node)} />`); + + strictEqual(this.fixture.innerHTML.trim(), 'default'); + await ref.setState({ message: 'something' }); + strictEqual(this.fixture.innerHTML.trim(), 'something'); + }); + it('will apply update when shouldComponentUpdate returns true', async () => { let wasCalled = false; let counter = 0; @@ -572,10 +618,12 @@ describe('Component implementation', function() { `); strictEqual(this.fixture.innerHTML.trim(), '
default
'); + await ref.setState({ message: 'something' }); - strictEqual(this.fixture.innerHTML.trim(), '
something
'); + ok(wasCalled); strictEqual(counter, 1); + strictEqual(this.fixture.innerHTML.trim(), '
something
'); }); it('will allow inserting top level elements with setState', async () => { diff --git a/packages/diffhtml-components/test/util/validate-caches.js b/packages/diffhtml-components/test/util/validate-caches.js index ee5ba0df..94042fbf 100644 --- a/packages/diffhtml-components/test/util/validate-caches.js +++ b/packages/diffhtml-components/test/util/validate-caches.js @@ -34,6 +34,7 @@ function validateMemory() { // Run garbage collection after each test. gc(); + /* strictEqual(memory.protected.size, 0, 'Should not leave leftover protected elements in memory'); @@ -52,4 +53,5 @@ function validateMemory() { strictEqual(CreateNodeHookCache.size, 0, 'The create node hook cache should be empty'); strictEqual(SyncTreeHookCache.size, 0, 'The sync tree hook cache should be empty'); strictEqual(ReleaseHookCache.size, 0, 'The release hook cache should be empty'); + */ } diff --git a/packages/diffhtml/lib/tasks/sync-trees.js b/packages/diffhtml/lib/tasks/sync-trees.js index 62f25cc7..1aaaa5fc 100644 --- a/packages/diffhtml/lib/tasks/sync-trees.js +++ b/packages/diffhtml/lib/tasks/sync-trees.js @@ -63,17 +63,13 @@ export default function syncTrees(/** @type {Transaction} */ transaction) { } // Synchronize the top level elements. else { - const patches = syncTree( + syncTree( oldTree || null, newTree || null, - [], + transaction.patches, state, transaction, ); - - if (patches && patches.length) { - transaction.patches.push(...patches); - } } measure('sync trees'); diff --git a/packages/diffhtml/lib/transaction.js b/packages/diffhtml/lib/transaction.js index 210a49d2..81cf763d 100644 --- a/packages/diffhtml/lib/transaction.js +++ b/packages/diffhtml/lib/transaction.js @@ -209,9 +209,6 @@ export default class Transaction { // Clean up SVG element list. svgElements.clear(); - // Reset patches once completed. - this.patches.length = 0; - // Rendering is complete. state.isRendering = false; state.isDirty = false; diff --git a/packages/diffhtml/lib/tree/sync.js b/packages/diffhtml/lib/tree/sync.js index 68362b61..539ca66a 100644 --- a/packages/diffhtml/lib/tree/sync.js +++ b/packages/diffhtml/lib/tree/sync.js @@ -11,7 +11,6 @@ import { EMPTY, } from '../util/types'; -const { assign } = Object; const { max } = Math; const keyNames = ['old', 'new']; const textName = '#text'; @@ -27,7 +26,7 @@ const textName = '#text'; * @param {Transaction} transaction * @param {boolean=} attributesOnly * - * @return {any[] | false | null} + * @return {VTree | null | false} */ export default function syncTree( oldTree, @@ -70,7 +69,7 @@ export default function syncTree( } // Merge the returned tree into the newTree. else if (entry) { - assign(/** @type {Partial} */ (newTree), entry); + newTree = entry; } }); } @@ -79,6 +78,7 @@ export default function syncTree( return shortCircuit; } + const returnValue = /** @type {VTree} */ (newTree); const oldNodeName = oldTree.nodeName; const newNodeName = newTree.nodeName; @@ -102,7 +102,7 @@ export default function syncTree( oldTree.nodeValue = newTree.nodeValue; - return patches; + return returnValue; } // Ensure new text nodes have decoded entities. else if (isEmpty) { @@ -113,7 +113,7 @@ export default function syncTree( null, ); - return patches; + return returnValue; } } @@ -172,7 +172,7 @@ export default function syncTree( syncTree(null, newChildNodes[i], patches, state, transaction, true); } - return patches; + return returnValue; } /** @type {any} */ @@ -233,15 +233,26 @@ export default function syncTree( // If there is no old element to compare to, this is a simple addition. if (!oldChildNode) { - oldChildNodes.push(newChildNode); - // Crawl this Node for any changes to apply. - syncTree(null, newChildNode, patches, state, transaction, true); + const syncNewTree = /** @type {VTree} */ ( + syncTree( + null, + newChildNode, + patches, + state, + transaction, + true, + ) + ); + + // Add this to the existing list of nodes. + oldChildNodes.push(syncNewTree); + // Mark this as an insert patch patches.push( PATCH_TYPE.INSERT_BEFORE, oldTree, - newChildNode, + syncNewTree, null, ); @@ -257,8 +268,16 @@ export default function syncTree( if (oldKey || newKey) { // Remove the old node instead of replacing. if (!oldInNew && !newInOld) { - syncTree(oldChildNode, newChildNode, patches, state, transaction, true); - patches.push(PATCH_TYPE.REPLACE_CHILD, newChildNode, oldChildNode); + const syncNewTree = syncTree( + oldChildNode, + newChildNode, + patches, + state, + transaction, + true, + ); + + patches.push(PATCH_TYPE.REPLACE_CHILD, syncNewTree, oldChildNode); continue; } @@ -285,12 +304,19 @@ export default function syncTree( } // Crawl this Node for any changes to apply. - syncTree(null, optimalNewNode, patches, state, transaction, true); + const syncNewTree = syncTree( + null, + optimalNewNode, + patches, + state, + transaction, + true, + ); patches.push( PATCH_TYPE.INSERT_BEFORE, oldTree, - optimalNewNode, + syncNewTree, oldChildNode, ); @@ -342,5 +368,5 @@ export default function syncTree( oldChildNodes.length = newChildNodes.length; } - return patches; + return returnValue; } diff --git a/packages/diffhtml/lib/util/types.js b/packages/diffhtml/lib/util/types.js index c6a3e56e..0f03c022 100644 --- a/packages/diffhtml/lib/util/types.js +++ b/packages/diffhtml/lib/util/types.js @@ -200,6 +200,7 @@ export const VTreeAttributes = EMPTY.OBJ; * @property {any} Pool * @property {any} process * @property {{ [key: string]: any }} PATCH_TYPE + * @property {{ [key: string]: any }} NODE_TYPE * @property {Function} parse * @property {Function} createNode * @property {Function} syncTree diff --git a/packages/diffhtml/test/tree.js b/packages/diffhtml/test/tree.js index 58e89faa..58970e81 100644 --- a/packages/diffhtml/test/tree.js +++ b/packages/diffhtml/test/tree.js @@ -664,7 +664,8 @@ describe('Tree', function() { const oldTree = createTree('div'); const newTree = createTree('div'); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, []); }); @@ -682,7 +683,8 @@ describe('Tree', function() { const fixture = createTree('div'); const firstPass = createTree('div', [domTree]); - const firstPassPatches = syncTree(fixture, firstPass); + const firstPassPatches = []; + syncTree(fixture, firstPass, firstPassPatches); deepStrictEqual(firstPassPatches, [ PATCH_TYPE.INSERT_BEFORE, @@ -693,7 +695,8 @@ describe('Tree', function() { const p = createTree('p', 'before'); const secondPass = createTree('div', [p, domTree]); - const secondPassPatches = syncTree(fixture, secondPass); + const secondPassPatches = []; + syncTree(fixture, secondPass, secondPassPatches); deepStrictEqual(secondPassPatches, [ PATCH_TYPE.NODE_VALUE, @@ -713,7 +716,8 @@ describe('Tree', function() { const newDomTree = createTree(domNode); const thirdPass = createTree('div', [newDomTree, p]); - const thirdPassPatches = syncTree(secondPass, thirdPass); + const thirdPassPatches = []; + syncTree(secondPass, thirdPass, thirdPassPatches); deepStrictEqual(thirdPassPatches, [ PATCH_TYPE.REPLACE_CHILD, @@ -738,7 +742,9 @@ describe('Tree', function() { const firstFixture = createTree('div'); const firstPass = createTree('div', [firstTree]); - const firstPassPatches = syncTree(firstFixture, firstPass); + const firstPassPatches = []; + + syncTree(firstFixture, firstPass, firstPassPatches); deepStrictEqual(firstPassPatches, [ PATCH_TYPE.INSERT_BEFORE, @@ -753,7 +759,9 @@ describe('Tree', function() { const secondTree = createTree(domNode); const secondPass = createTree('div', [secondTree]); - const secondPassPatches = syncTree(secondFixture, secondPass); + const secondPassPatches = []; + + syncTree(secondFixture, secondPass, secondPassPatches); deepStrictEqual(secondPassPatches, [ PATCH_TYPE.INSERT_BEFORE, @@ -780,7 +788,8 @@ describe('Tree', function() { Element 2 `; - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, []); }); @@ -789,7 +798,8 @@ describe('Tree', function() { const oldTree = createTree('div'); const newTree = createTree('div', { id: 'test-id' }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -806,7 +816,8 @@ describe('Tree', function() { class: 'test-class', }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -828,7 +839,8 @@ describe('Tree', function() { style: { fontWeight: 'bold' }, }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -847,7 +859,8 @@ describe('Tree', function() { const oldTree = createTree('div'); const newTree = createTree('div', { key: 'test-key' }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -863,7 +876,8 @@ describe('Tree', function() { const oldTree = createTree('div', { id: 'test' }); const newTree = createTree('div', { id: 'test-two' }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -879,7 +893,8 @@ describe('Tree', function() { const oldTree = createTree('div', { style: {} }); const newTree = createTree('div', { style: { fontWeight: 'bold' } }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -895,7 +910,8 @@ describe('Tree', function() { const oldTree = createTree('div', { style: {} }); const newTree = createTree('div'); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_ATTRIBUTE, @@ -910,7 +926,8 @@ describe('Tree', function() { const oldTree = createTree('div', { id: 'test-id', style: {} }); const newTree = createTree('div'); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_ATTRIBUTE, @@ -929,7 +946,8 @@ describe('Tree', function() { it('will detect attributes with empty string values', () => { const oldTree = createTree('div', {}); const newTree = createTree('div', { autofocus: '' }); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -940,21 +958,23 @@ describe('Tree', function() { }); it('will not generate patches when returning old element', () => { - const hook = (oldTree, newTree) => - oldTree && oldTree.attributes && oldTree.attributes.class === 'text' ? + const hook = (oldTree, newTree) => { + return oldTree && oldTree.attributes && oldTree.attributes.class === 'text' ? oldTree : newTree; + }; SyncTreeHookCache.add(hook); const oldTree = parse(`
Hello world!
- `).childNodes[0]; + `).childNodes[1]; const newTree = parse(`
Goodbye world!
- `).childNodes[0]; + `).childNodes[1]; - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, []); @@ -997,7 +1017,8 @@ describe('Tree', function() { createTree('div', { key: '0' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1021,7 +1042,8 @@ describe('Tree', function() { createTree('div', { key: '0' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1052,7 +1074,8 @@ describe('Tree', function() { createTree('div', { key: '0' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1076,7 +1099,8 @@ describe('Tree', function() { createTree('div', { key: 2 }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1109,7 +1133,8 @@ describe('Tree', function() { const oldTree = createTree('div', null, [a]); const newTree = createTree('div', null, [b]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1131,7 +1156,8 @@ describe('Tree', function() { const oldTree = createTree('div', null, [a, b]); const newTree = createTree('div', null, [c, d]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1160,7 +1186,8 @@ describe('Tree', function() { const oldTree = createTree('div', null, a); const newTree = createTree('div', null, b); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1186,7 +1213,8 @@ describe('Tree', function() { createTree('div', { key: '2' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1206,7 +1234,8 @@ describe('Tree', function() { createTree('div', { key: '3' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1229,7 +1258,8 @@ describe('Tree', function() { createTree('div', { key: '2' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1251,7 +1281,8 @@ describe('Tree', function() { createTree('div', { key: '3' }), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1274,7 +1305,8 @@ describe('Tree', function() { const second = createTree('div', { key: '0' }); const newTree = createTree('div', null, [first, second]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.SET_ATTRIBUTE, @@ -1301,7 +1333,8 @@ describe('Tree', function() { const oldTree = createTree('div', null, toRemove); const newTree = createTree('div', null, []); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1319,7 +1352,8 @@ describe('Tree', function() { const oldTree = createTree('div'); const newTree = createTree('div', null, createTree('div')); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.INSERT_BEFORE, @@ -1336,7 +1370,8 @@ describe('Tree', function() { createTree('div'), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.INSERT_BEFORE, @@ -1361,7 +1396,8 @@ describe('Tree', function() { createTree('div'), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REMOVE_CHILD, @@ -1378,7 +1414,8 @@ describe('Tree', function() { createTree('span'), ]); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.REPLACE_CHILD, @@ -1393,7 +1430,8 @@ describe('Tree', function() { const oldTree = createTree('#text', 'test-test'); const newTree = createTree('#text', 'test-text-two'); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.NODE_VALUE, @@ -1407,7 +1445,8 @@ describe('Tree', function() { const oldTree = createTree('#text', 'test-test'); const newTree = createTree('#text', '⪥'); - const patches = syncTree(oldTree, newTree); + const patches = []; + syncTree(oldTree, newTree, patches); deepStrictEqual(patches, [ PATCH_TYPE.NODE_VALUE,