[FIX] web: fix debounce utility function when used with immediate

Previously, the "debounce" util function was broken when passing
immediate=true, this was caused by the fact that the timeout was not set
to null after being executed, leading the function to always act as
though a call is already scheduled.

This commit basically rewrites the entire debounce function to fix this
problem, simplify the code, and make the API of debounce as close as
possible to underscorejs' debounce utility (with the exception that our
debounce function returns a Promise that gets resolved if and when the
call eventually goes through, and with the extra feature that the
debounced function has a cancel method to cancel the currently scheduled
call)

closes odoo/odoo#90297

X-original-commit: 24c39ba1744d83ef9712ef4d3df7db5baf35bc2e
Related: odoo/enterprise#26821
Signed-off-by: Samuel Degueldre <sad@odoo.com>
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

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

@@ -47,34 +47,28 @@ export function throttleForAnimation(func) {
  * @returns {{ (...args): Promise<T>, cancel: () => void }} the debounced function
  */
 export function debounce(func, wait, immediate = false) {
-    let cancelled;
     let timeout;
     const funcName = func.name ? func.name + " (debounce)" : "debounce";
     return Object.assign(
         {
             [funcName](...args) {
-                const context = this;
-                let resolve;
-                const prom = new Promise((r) => {
-                    resolve = r;
-                });
-                function later() {
-                    if (!immediate && !cancelled) {
-                        Promise.resolve(func.apply(context, args)).then(resolve);
+                return new Promise((resolve) => {
+                    const callNow = immediate && !timeout;
+                    browser.clearTimeout(timeout);
+                    timeout = browser.setTimeout(() => {
+                        timeout = null;
+                        if (!immediate) {
+                            Promise.resolve(func.apply(this, args)).then(resolve);
+                        }
+                    }, wait);
+                    if (callNow) {
+                        Promise.resolve(func.apply(this, args)).then(resolve);
                     }
-                }
-                const callNow = immediate && !timeout && !cancelled;
-                browser.clearTimeout(timeout);
-                timeout = browser.setTimeout(later, wait);
-                if (callNow) {
-                    Promise.resolve(func.apply(context, args)).then(resolve);
-                }
-                return prom;
+                });
             },
         }[funcName],
         {
             cancel() {
-                cancelled = true;
                 browser.clearTimeout(timeout);
             },
         }

addons/web/static/tests/core/utils/timing_tests.js

@@ -69,11 +69,7 @@ QUnit.module("utils", () => {
     });
 
     QUnit.test("debounce with immediate", async function (assert) {
-        patchWithCleanup(browser, {
-            setTimeout: (later) => {
-                later();
-            },
-        });
+        const execRegisteredTimeouts = mockTimeout();
         const myFunc = () => {
             assert.step("myFunc");
             return 42;
@@ -83,14 +79,27 @@ QUnit.module("utils", () => {
             assert.step("resolved " + x);
         });
         assert.verifySteps(["myFunc"]);
-        await Promise.resolve(); // wait for promise returned by myFunc
         await Promise.resolve(); // wait for promise returned by debounce
+        await Promise.resolve(); // wait for promise returned chained onto it (step resolved x)
+        assert.verifySteps(["resolved 42"]);
 
+        myDebouncedFunc().then((x) => {
+            assert.step("resolved " + x);
+        });
+        await execRegisteredTimeouts();
+        assert.verifySteps([]); // not called 3000ms did not elapse between the previous call and the first
+
+        myDebouncedFunc().then((x) => {
+            assert.step("resolved " + x);
+        });
+        assert.verifySteps(["myFunc"]);
+        await Promise.resolve(); // wait for promise returned by debounce
+        await Promise.resolve(); // wait for promise returned chained onto it (step resolved x)
         assert.verifySteps(["resolved 42"]);
     });
 
-    QUnit.test("debounced call can be canceled", async function (assert) {
-        assert.expect(1);
+    QUnit.test("debounced call can be cancelled", async function (assert) {
+        assert.expect(3);
         const execRegisteredTimeouts = mockTimeout();
         const myFunc = () => {
             assert.step("myFunc");
@@ -99,7 +108,11 @@ QUnit.module("utils", () => {
         myDebouncedFunc();
         myDebouncedFunc.cancel();
         execRegisteredTimeouts();
-        assert.verifySteps([], "Debounced call was canceled");
+        assert.verifySteps([], "Debounced call was cancelled");
+
+        myDebouncedFunc();
+        execRegisteredTimeouts();
+        assert.verifySteps(["myFunc"], "Debounced call was not cancelled");
     });
 
     QUnit.test("throttleForAnimation", async (assert) => {

addons/web/static/tests/helpers/utils.js

@@ -355,7 +355,7 @@ for (const propName of Object.keys(window.console)) {
 
 export function mockTimeout() {
     const timeouts = new Map();
-    let id = 0;
+    let id = 1;
     patchWithCleanup(browser, {
         setTimeout(fn) {
             timeouts.set(id, fn);
@@ -375,7 +375,7 @@ export function mockTimeout() {
 
 export function mockAnimationFrame() {
     const callbacks = new Map();
-    let id = 0;
+    let id = 1;
     patchWithCleanup(browser, {
         requestAnimationFrame(fn) {
             callbacks.set(id, fn);

addons/web/static/tests/webclient/navbar_tests.js

@@ -16,10 +16,11 @@ import {
     mount,
     nextTick,
     patchWithCleanup,
-    makeDeferred,
+    mockTimeout,
 } from "../helpers/utils";
 
 const { Component, xml } = owl;
+
 const systrayRegistry = registry.category("systray");
 const serviceRegistry = registry.category("services");
 
@@ -426,15 +427,7 @@ QUnit.test("'more' menu sections properly updated on app change", async (assert)
 QUnit.test("Do not execute adapt when navbar is destroyed", async (assert) => {
     assert.expect(5);
 
-    let prom = makeDeferred();
-
-    patchWithCleanup(browser, {
-        setTimeout: async (handler, delay, ...args) => {
-            await prom;
-            return handler(...args);
-        },
-        clearTimeout: () => {},
-    });
+    const execRegisteredTimeouts = mockTimeout();
     class MyNavbar extends NavBar {
         async adapt() {
             assert.step("adapt NavBar");
@@ -448,13 +441,10 @@ QUnit.test("Do not execute adapt when navbar is destroyed", async (assert) => {
     const navbar = await mount(MyNavbar, target, { env });
     assert.verifySteps(["adapt NavBar"]);
     window.dispatchEvent(new Event("resize"));
-    prom.resolve();
-    await prom;
-    prom = makeDeferred();
+    execRegisteredTimeouts();
     assert.verifySteps(["adapt NavBar"]);
     window.dispatchEvent(new Event("resize"));
     destroy(navbar);
-    prom.resolve();
-    await prom;
+    execRegisteredTimeouts();
     assert.verifySteps([]);
 });