diff --git a/src/hooks.ts b/src/hooks.ts index 55a941d5..07b34360 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,5 +1,6 @@ import type { Env } from "./app/app"; import { getCurrent } from "./component/component_node"; +import { onMounted, onPatched, onWillPatch, onWillUnmount } from "./component/lifecycle_hooks"; // ----------------------------------------------------------------------------- // useRef @@ -39,3 +40,53 @@ export function useSubEnv(envExtension: Env) { const node = getCurrent()!; node.childEnv = Object.freeze(Object.assign({}, node.childEnv, envExtension)); } + +// ----------------------------------------------------------------------------- +// useEffect +// ----------------------------------------------------------------------------- + +const NO_OP = () => {}; +/** + * @param {...any} dependencies the dependencies computed by computeDependencies + * @returns {void|(()=>void)} a cleanup function that reverses the side + * effects of the effect callback. + */ +type Effect = (...dependencies: any[]) => void | (() => void); + +/** + * This hook will run a callback when a component is mounted and patched, and + * will run a cleanup function before patching and before unmounting the + * the component. + * + * @param {Effect} effect the effect to run on component mount and/or patch + * @param {()=>any[]} [computeDependencies=()=>[NaN]] a callback to compute + * dependencies that will decide if the effect needs to be cleaned up and + * run again. If the dependencies did not change, the effect will not run + * again. The default value returns an array containing only NaN because + * NaN !== NaN, which will cause the effect to rerun on every patch. + */ +export function useEffect(effect: Effect, computeDependencies: () => any[] = () => [NaN]) { + let cleanup: () => void; + let dependencies: any[]; + onMounted(() => { + dependencies = computeDependencies(); + cleanup = effect(...dependencies) || NO_OP; + }); + + let shouldReapplyOnPatch = false; + onWillPatch(() => { + const newDeps = computeDependencies(); + shouldReapplyOnPatch = newDeps.some((val, i) => val !== dependencies[i]); + if (shouldReapplyOnPatch) { + cleanup(); + dependencies = newDeps; + } + }); + onPatched(() => { + if (shouldReapplyOnPatch) { + cleanup = effect(...dependencies) || NO_OP; + } + }); + + onWillUnmount(() => cleanup()); +} diff --git a/src/index.ts b/src/index.ts index 5a0ffa07..a7ac8a97 100644 --- a/src/index.ts +++ b/src/index.ts @@ -57,7 +57,7 @@ export { Portal } from "./misc/portal"; export { Memo } from "./misc/memo"; export { css, xml } from "./tags"; export { useState } from "./reactivity"; -export { useRef, useEnv, useSubEnv } from "./hooks"; +export { useRef, useEnv, useSubEnv, useEffect } from "./hooks"; export const utils = { EventBus, whenReady, loadFile }; export { diff --git a/tests/components/__snapshots__/hooks.test.ts.snap b/tests/components/__snapshots__/hooks.test.ts.snap index b829c6d3..16f52263 100644 --- a/tests/components/__snapshots__/hooks.test.ts.snap +++ b/tests/components/__snapshots__/hooks.test.ts.snap @@ -66,21 +66,6 @@ exports[`hooks can use onWillStart, onWillUpdateProps 2`] = ` }" `; -exports[`hooks can use sub env 1`] = ` -"function anonymous(bdom, helpers -) { - let { text, createBlock, list, multi, html, toggler, component } = bdom; - let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers; - - let block1 = createBlock(\`
\`); - - return function template(ctx, node, key = \\"\\") { - let d1 = ctx['env'].val; - return block1([d1]); - } -}" -`; - exports[`hooks can use useComponent 1`] = ` "function anonymous(bdom, helpers ) { @@ -169,6 +154,64 @@ exports[`hooks two different call to willPatch/patched should work 1`] = ` }" `; +exports[`hooks use sub env does not pollute user env 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let d1 = ctx['env'].val; + return block1([d1]); + } +}" +`; + +exports[`hooks useEffect hook dependencies prevent effects from rerunning when unchanged 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`hooks useEffect hook effect runs on mount, is reapplied on patch, and is cleaned up on unmount and before reapplying 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`hooks useEffect hook effect with empty dependency list never reruns 1`] = ` +"function anonymous(bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, component } = bdom; + let { withDefault, getTemplate, prepareList, withKey, zero, call, callSlot, capture, isBoundary, shallowEqual, setContextValue, toNumber } = helpers; + + let block1 = createBlock(\`
\`); + + return function template(ctx, node, key = \\"\\") { + let d1 = ctx['state'].value; + return block1([d1]); + } +}" +`; + exports[`hooks useRef hook: basic use 1`] = ` "function anonymous(bdom, helpers ) { diff --git a/tests/components/hooks.test.ts b/tests/components/hooks.test.ts index 2088ba13..45494d23 100644 --- a/tests/components/hooks.test.ts +++ b/tests/components/hooks.test.ts @@ -7,6 +7,7 @@ import { useComponent, useEnv, useSubEnv, + useEffect, onMounted, onPatched, onWillStart, @@ -318,4 +319,170 @@ describe("hooks", () => { window.dispatchEvent(new Event("click")); expect(n).toBe(1); }); + + describe("useEffect hook", () => { + test("effect runs on mount, is reapplied on patch, and is cleaned up on unmount and before reapplying", async () => { + let cleanupRun = 0; + let steps = []; + class MyComponent extends Component { + state = useState({ + value: 0, + }); + setup() { + useEffect(() => { + steps.push(`value is ${this.state.value}`); + return () => + steps.push(`cleaning up for value = ${this.state.value} (cleanup ${cleanupRun++})`); + }); + } + } + MyComponent.template = xml`
`; + + const component = await mount(MyComponent, fixture); + + steps.push("before state mutation"); + component.state.value++; + // Wait for an owl render + await new Promise((resolve) => requestAnimationFrame(resolve)); + steps.push("after state mutation"); + await component.__owl__.destroy(); + + expect(steps).toEqual([ + "value is 0", + "before state mutation", + // While one might expect value to be 0 at cleanup, because the value is + // read during cleanup from the state rather than captured by a dependency + // it already has the new value. Having this in business code is a symptom + // of a missing dependency and can lead to bugs. + "cleaning up for value = 1 (cleanup 0)", + "value is 1", + "after state mutation", + "cleaning up for value = 1 (cleanup 1)", + ]); + }); + + test("dependencies prevent effects from rerunning when unchanged", async () => { + let steps = []; + class MyComponent extends Component { + state = useState({ + a: 0, + b: 0, + }); + setup() { + useEffect( + (a) => { + steps.push(`Effect a: ${a}`); + return () => steps.push(`cleaning up for a: ${a}`); + }, + () => [this.state.a] + ); + useEffect( + (b) => { + steps.push(`Effect b: ${b}`); + return () => steps.push(`cleaning up for b: ${b}`); + }, + () => [this.state.b] + ); + useEffect( + (a, b) => { + steps.push(`Effect ab: {a: ${a}, b: ${b}}`); + return () => steps.push(`cleaning up for ab: {a: ${a}, b: ${b}}`); + }, + () => [this.state.a, this.state.b] + ); + } + } + MyComponent.template = xml`
`; + steps.push("before mount"); + const component = await mount(MyComponent, fixture); + steps.push("after mount"); + + steps.push("before state mutation: a"); + component.state.a++; + // Wait for an owl render + await new Promise((resolve) => requestAnimationFrame(resolve)); + steps.push("after state mutation: a"); + + steps.push("before state mutation: b"); + component.state.b++; + // Wait for an owl render + await new Promise((resolve) => requestAnimationFrame(resolve)); + steps.push("after state mutation: b"); + await component.__owl__.destroy(); + + expect(steps).toEqual([ + // All effects run on mount + "before mount", + "Effect a: 0", + "Effect b: 0", + "Effect ab: {a: 0, b: 0}", + "after mount", + + "before state mutation: a", + // Cleanups run in reverse order + "cleaning up for ab: {a: 0, b: 0}", + // Cleanup for b is not run + "cleaning up for a: 0", + + "Effect a: 1", + // Effect b is not run + "Effect ab: {a: 1, b: 0}", + "after state mutation: a", + + "before state mutation: b", + "cleaning up for ab: {a: 1, b: 0}", + "cleaning up for b: 0", + // Cleanup for a is not run + + // Effect a is not run + "Effect b: 1", + "Effect ab: {a: 1, b: 1}", + "after state mutation: b", + + // All cleanups run on unmount + "cleaning up for ab: {a: 1, b: 1}", + "cleaning up for b: 1", + "cleaning up for a: 1", + ]); + }); + + test("effect with empty dependency list never reruns", async () => { + let steps = []; + class MyComponent extends Component { + state = useState({ + value: 0, + }); + setup() { + useEffect( + () => { + steps.push(`value is ${this.state.value}`); + return () => steps.push(`cleaning up for ${this.state.value}`); + }, + () => [] + ); + } + } + MyComponent.template = xml`
`; + + const component = await mount(MyComponent, fixture); + + steps.push("before state mutation"); + component.state.value++; + // Wait for an owl render + await new Promise((resolve) => requestAnimationFrame(resolve)); + // Value was correctly changed inside the component + expect(component.el!.textContent).toBe("1"); + steps.push("after state mutation"); + await component.__owl__.destroy(); + + expect(steps).toEqual([ + "value is 0", + "before state mutation", + // no cleanup or effect caused by mutation + "after state mutation", + // Value being clean + "cleaning up for 1", + ]); + }); + }); });