[IMP] web: add tests and comments for reactivity utils

The reactivity utils were initially an ad-hoc solution to some problems
that came up when using the reactivity in places that are not
components. As it turned out that these needs are actually quite common,
the utils that were originally duplicated in many modules were moved to
the core utils in web, but there were no tests for these utils, and the
Reactive class was not documented.

This commit adds some tests for the reactivity utils and a docstring
motivating the existence of the Reactive class.

closes odoo/odoo#125549

Signed-off-by: Géry Debongnie <ged@odoo.com>

addons/web/static/src/core/utils/reactive.js

@@ -2,6 +2,25 @@
 
 import { reactive } from "@odoo/owl";
 
+/**
+ * This class should be used as a base when creating a class that is intended to
+ * be used within the reactivity system, it avoids a specific class of bug where
+ * callbacks that capture `this` declared in the constructor would escape the
+ * reactivity system and prevent the observers from being notified:
+ *
+ * const bus = new EventBus();
+ * class MyClass {
+ *   constructor() {
+ *     this.counter = 0;
+ *     bus.addEventListener("change", () => this.counter++);
+ *     //                                   ^ Will never be reactive, this mutation will be missed
+ *   }
+ * }
+ * const myObj = reactive(new MyClass(bus), () => console.log(myObj.counter));
+ * myObj.counter++; // logs 0;
+ * bus.trigger("change"); // logs nothing!
+ * myObj.counter++; // logs 2. counter == 1 was missed.
+ */
 export class Reactive {
     constructor() {
         return reactive(this);

addons/web/static/tests/core/reactive_tests.js

@@ -0,0 +1,194 @@
+/** @odoo-module */
+
+import { EventBus, reactive } from "@odoo/owl";
+import { Reactive, effect, withComputedProperties } from "@web/core/utils/reactive";
+
+QUnit.module("Reactive utils", () => {
+    QUnit.module("Reactive class", () => {
+        QUnit.test(
+            "callback registered without Reactive class constructor will not notify",
+            (assert) => {
+                // This test exists to showcase why we need the Reactive class
+                const bus = new EventBus();
+                class MyReactiveClass {
+                    constructor() {
+                        this.counter = 0;
+                        bus.addEventListener("change", () => this.counter++);
+                    }
+                }
+                const obj = reactive(new MyReactiveClass(), () => {
+                    assert.step(`counter: ${obj.counter}`);
+                });
+                obj.counter; // initial subscription to counter
+                obj.counter++;
+                assert.verifySteps(["counter: 1"]);
+                bus.trigger("change");
+                assert.equal(obj.counter, 2);
+                assert.verifySteps([
+                    // The mutation in the event handler was missed by the reactivity, this is because
+                    // the `this` in the event handler is captured during construction and is not reactive
+                ]);
+            }
+        );
+
+        QUnit.test("callback registered in Reactive class constructor will notify", (assert) => {
+            const bus = new EventBus();
+            class MyReactiveClass extends Reactive {
+                constructor() {
+                    super();
+                    this.counter = 0;
+                    bus.addEventListener("change", () => this.counter++);
+                }
+            }
+            const obj = reactive(new MyReactiveClass(), () => {
+                assert.step(`counter: ${obj.counter}`);
+            });
+            obj.counter; // initial subscription to counter
+            obj.counter++;
+            assert.verifySteps(["counter: 1"]);
+            bus.trigger("change");
+            assert.equal(obj.counter, 2);
+            assert.verifySteps(["counter: 2"]);
+        });
+    });
+
+    QUnit.module("effect", () => {
+        QUnit.test("effect runs once immediately", (assert) => {
+            const state = reactive({ counter: 0 });
+            assert.verifySteps([]);
+            effect(
+                (state) => {
+                    assert.step(`counter: ${state.counter}`);
+                },
+                [state]
+            );
+            assert.verifySteps(["counter: 0"]);
+        });
+
+        QUnit.test("effect runs when reactive deps change", (assert) => {
+            const state = reactive({ counter: 0 });
+            assert.verifySteps([]);
+            effect(
+                (state) => {
+                    assert.step(`counter: ${state.counter}`);
+                },
+                [state]
+            );
+            assert.verifySteps(["counter: 0"], "effect runs immediately");
+            state.counter++;
+            assert.verifySteps(["counter: 1"], "first mutation runs the effect");
+            state.counter++;
+            assert.verifySteps(["counter: 2"], "subsequent mutations run the effect");
+        });
+
+        QUnit.test(
+            "Original reactive callback is not subscribed to keys observed by effect",
+            (assert) => {
+                let reactiveCallCount = 0;
+                const state = reactive(
+                    {
+                        counter: 0,
+                    },
+                    () => reactiveCallCount++
+                );
+                assert.verifySteps([]);
+                assert.equal(reactiveCallCount, 0);
+                effect(
+                    (state) => {
+                        assert.step(`counter: ${state.counter}`);
+                    },
+                    [state]
+                );
+                assert.verifySteps(["counter: 0"]);
+                assert.equal(reactiveCallCount, 0, "did not call the original reactive's callback");
+                state.counter = 1;
+                assert.verifySteps(["counter: 1"]);
+                assert.equal(reactiveCallCount, 0, "did not call the original reactive's callback");
+                state.counter; // subscribe the original reactive
+                state.counter = 2;
+                assert.verifySteps(["counter: 2"]);
+                assert.equal(
+                    reactiveCallCount,
+                    1,
+                    "the original callback was called because it is subscribed independently"
+                );
+            }
+        );
+
+        QUnit.test("mutating keys not observed by the effect doesn't cause it to run", (assert) => {
+            const state = reactive({ counter: 0, unobserved: 0 });
+            effect(
+                (state) => {
+                    assert.step(`counter: ${state.counter}`);
+                },
+                [state]
+            );
+            assert.verifySteps(["counter: 0"]);
+            state.counter = 1;
+            assert.verifySteps(["counter: 1"]);
+            state.unobserved = 1;
+            assert.verifySteps([]);
+        });
+    });
+
+    QUnit.module("withComputedProperties", () => {
+        QUnit.test("computed properties are set immediately", (assert) => {
+            const source = reactive({ counter: 1 });
+            const derived = withComputedProperties(reactive({}), [source], {
+                doubleCounter(source) {
+                    return source.counter * 2;
+                },
+            });
+            assert.equal(derived.doubleCounter, 2);
+        });
+
+        QUnit.test("computed properties are recomputed when dependencies change", (assert) => {
+            const source = reactive({ counter: 1 });
+            const derived = withComputedProperties(reactive({}), [source], {
+                doubleCounter(source) {
+                    return source.counter * 2;
+                },
+            });
+            assert.equal(derived.doubleCounter, 2);
+            source.counter++;
+            assert.equal(derived.doubleCounter, 4);
+        });
+
+        QUnit.test("can observe computed properties", (assert) => {
+            const source = reactive({ counter: 1 });
+            const derived = withComputedProperties(reactive({}), [source], {
+                doubleCounter(source) {
+                    return source.counter * 2;
+                },
+            });
+            const observed = reactive(derived, () => {
+                assert.step(`doubleCounter: ${observed.doubleCounter}`);
+            });
+            observed.doubleCounter; // subscribe to doubleCounter
+            assert.verifySteps([]);
+            source.counter++;
+            assert.verifySteps(["doubleCounter: 4"]);
+        });
+
+        QUnit.test("computed properties can use nested objects", (assert) => {
+            const source = reactive({ subObj: { counter: 1 } });
+            const derived = withComputedProperties(reactive({}), [source], {
+                doubleCounter(source) {
+                    return source.subObj.counter * 2;
+                },
+            });
+            const observed = reactive(derived, () => {
+                assert.step(`doubleCounter: ${observed.doubleCounter}`);
+            });
+            observed.doubleCounter; // subscribe to doubleCounter
+            assert.equal(observed.doubleCounter, 2);
+            assert.verifySteps([]);
+            source.subObj.counter++;
+            assert.equal(derived.doubleCounter, 4);
+            assert.verifySteps(
+                ["doubleCounter: 4"],
+                "reactive gets notified even for computed properties dervied from nested objects"
+            );
+        });
+    });
+});