From 5ec3716419c9f0bb989460afd4afb41ae18ce8bb Mon Sep 17 00:00:00 2001 From: Tim Branyen Date: Thu, 31 Mar 2022 19:41:37 -0700 Subject: [PATCH] createSideEffect hook fixes and tests --- .../lib/create-side-effect.js | 44 +++++++++-- .../diffhtml-components/lib/middleware.js | 7 +- .../lib/render-component.js | 7 -- .../test/integration/hooks.js | 73 +++++++++++++++++++ 4 files changed, 113 insertions(+), 18 deletions(-) diff --git a/packages/diffhtml-components/lib/create-side-effect.js b/packages/diffhtml-components/lib/create-side-effect.js index 286b9761..89681ad7 100644 --- a/packages/diffhtml-components/lib/create-side-effect.js +++ b/packages/diffhtml-components/lib/create-side-effect.js @@ -9,7 +9,7 @@ import { ActiveRenderState } from './util/types'; * @param {Function=} didMountOrUpdate - A function that is called whenever the * component is mounted. To hook into component updates, return a function. This * returned function will be called whenever the component updates. - * + * * @param {Function=} unMount - A function that is called whenever a component * is unmounted. * @@ -25,27 +25,57 @@ export function createSideEffect(didMountOrUpdate, unMount) { } const [ activeComponent ] = ActiveRenderState; + const hooks = activeComponent[$$hooks]; + const isSet = Boolean(hooks.fns[hooks.i]); + + // Indicate this hook has been installed. + hooks.fns[hooks.i] = true; + + // Increment the hooks count. + activeComponent[$$hooks].i += 1; + + // Short-circuit if effects are already installed. + if (isSet) { + return; + } // Schedule a componentDidMount if a function was provided if (typeof didMountOrUpdate === 'function') { + const oldDidMount = activeComponent.componentDidMount; + + // Install the componentDidMount lifecycle hook. activeComponent.componentDidMount = () => { + if (typeof oldDidMount === 'function') { + oldDidMount(); + } + + // Install the componentDidUpdate lifecycle hook. const didUpdate = didMountOrUpdate(); + const oldComponentDidUpdate = activeComponent.componentDidUpdate; // Then if the user specifies a return function, use that as didUpdate if (typeof didUpdate === 'function') { activeComponent.componentDidUpdate = () => { + if (typeof oldComponentDidUpdate === 'function') { + oldComponentDidUpdate(); + } + didUpdate(); }; } - }; } - // Schedule a componentWillUnmount if a function is provided + // Install the componentWillUnmount lifecycle hook. if (typeof unMount === 'function') { - activeComponent.componentWillUnmount = () => unMount(); - } + const oldWillUnmount = activeComponent.componentWillUnmount; - // Increment the hooks count. - activeComponent[$$hooks].i += 1; + activeComponent.componentWillUnmount = () => { + if (typeof oldWillUnmount === 'function') { + oldWillUnmount(); + } + + unMount(); + }; + } } diff --git a/packages/diffhtml-components/lib/middleware.js b/packages/diffhtml-components/lib/middleware.js index 0fdedbcd..aa72af9d 100644 --- a/packages/diffhtml-components/lib/middleware.js +++ b/packages/diffhtml-components/lib/middleware.js @@ -1,4 +1,4 @@ -import { EMPTY, ComponentTreeCache, VTree, Transaction } from './util/types'; +import { EMPTY, InstanceCache, ComponentTreeCache, VTree, Transaction } from './util/types'; import globalThis from './util/global'; import onceEnded from './once-ended'; import componentWillUnmount from './lifecycle/component-will-unmount'; @@ -21,7 +21,7 @@ function render(oldTree, newTree) { // First try and lookup the old tree as a component. oldComponentTree = ComponentTreeCache.get(oldTree); - // If that fails, try looking up it's first child. + // If that fails, try looking up its first child. if (!oldComponentTree) { oldComponentTree = ComponentTreeCache.get(oldTree.childNodes[0]); } @@ -102,8 +102,7 @@ const syncTreeHook = (oldTree, newTree) => { const newChildTree = newTree.childNodes[i]; const oldChildTree = (oldTree.childNodes && oldTree.childNodes[i]) || EMPTY.OBJ; - // If the old slot was not a component, and the new tree is, then we are - // rendering a brand new component. + // Search through the DOM tree for more components to render. if (typeof newChildTree.rawNodeName === 'function') { const renderTree = render(oldChildTree, newChildTree); diff --git a/packages/diffhtml-components/lib/render-component.js b/packages/diffhtml-components/lib/render-component.js index d66ab77b..36418be5 100644 --- a/packages/diffhtml-components/lib/render-component.js +++ b/packages/diffhtml-components/lib/render-component.js @@ -39,13 +39,6 @@ export default function renderComponent(vTree) { instance.shouldComponentUpdate && instance.shouldComponentUpdate(props, instance.state) ) { - // Wipe out all old references before re-rendering. - ComponentTreeCache.forEach((parentTree, childTree) => { - if (parentTree === vTree) { - ComponentTreeCache.delete(childTree); - } - }); - ActiveRenderState.push(instance); // Reset the hooks iterator. instance[$$hooks].i = 0; diff --git a/packages/diffhtml-components/test/integration/hooks.js b/packages/diffhtml-components/test/integration/hooks.js index bff3dea4..a44602a5 100644 --- a/packages/diffhtml-components/test/integration/hooks.js +++ b/packages/diffhtml-components/test/integration/hooks.js @@ -156,6 +156,79 @@ describe('Hooks', function() { strictEqual(firedOnUnmount, 1); }); + it('will support multiple side effects', async () => { + let firedOnMount = 0; + let firedOnUnmount = 0; + + function Component() { + createSideEffect(() => { + firedOnMount++; + }); + + createSideEffect(null, () => { + firedOnUnmount++; + }); + + return html`
`; + } + + this.fixture = document.createElement('div'); + + await innerHTML(this.fixture, html`<${Component} />`); + await innerHTML(this.fixture, html``); + + strictEqual(firedOnMount, 1); + strictEqual(firedOnUnmount, 1); + }); + + it('will support multiple didMount side effects', async () => { + let firedOnMount = 0; + + function Component() { + createSideEffect(() => { + firedOnMount++; + }); + + createSideEffect(() => { + firedOnMount++; + }); + + return html`
`; + } + + this.fixture = document.createElement('div'); + + await innerHTML(this.fixture, html`<${Component} />`); + + strictEqual(firedOnMount, 2); + }); + + it('will support multiple didUpdate side effects', async () => { + let firedOnUpdate = 0; + + function Component() { + createSideEffect(() => () => { + firedOnUpdate++; + }); + + createSideEffect(() => () => { + firedOnUpdate++; + }); + + return html`
`; + } + + this.fixture = document.createElement('div'); + + await innerHTML(this.fixture, html`<${Component} />`); + strictEqual(firedOnUpdate, 0); + await innerHTML(this.fixture, html`<${Component} test="prop" />`); + strictEqual(firedOnUpdate, 2); + await innerHTML(this.fixture, html`<${Component} test="change" />`); + + strictEqual(firedOnUpdate, 4); + }); + it('will work with createState', async () => { let firedOnUpdate = 0; let firedOnUnmount = 0;