[IMP] hooks: introduce useEffect

Co-Authored-By: Samuel Degueldre <sad@odoo.com>

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());
+}

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 {

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(\`<div><block-text-0/></div>\`);
-  
-  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(\`<div><block-text-0/></div>\`);
+  
+  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(\`<div/>\`);
+  
+  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(\`<div/>\`);
+  
+  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(\`<div><block-text-0/></div>\`);
+  
+  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
 ) {

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`<div/>`;
+
+      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`<div/>`;
+      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`<div t-esc="state.value"/>`;
+
+      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",
+      ]);
+    });
+  });
 });