lodash / lodash

A modern JavaScript utility library delivering modularity, performance, & extras.
https://lodash.com/
Other
59.77k stars 7.02k forks source link

Debounced async function returns no promise #4400

Closed Jtaks closed 5 years ago

Jtaks commented 5 years ago

Hi There,

node: v10.16.0 lodash: ^4.17.15

Using _.debounce with a function that returns a promise does not return a promise on the first call. For example:

const debounce = require('lodash/debounce')

const test = async () => 7

const debounced = debounce(test, 1000)

const calls = []

calls.push(debounced())
setTimeout(() => { calls.push(debounced()) }, 1500)
setTimeout(() => { calls.push(debounced()) && console.dir(calls) }, 3000)

// Output: [ undefined, Promise { 7 }, Promise { 7 } ]

I expect all calls to be promises. Am I mistaken?

Edit: Changed example to better demonstrate issue

jdalton commented 5 years ago

Hi @Jtaks!

Please check out the leading option to specify.

Obiwarn commented 4 years ago

What can I do if I don't want the leading option but still need the promise?

andrewsosa commented 4 years ago

Following up on @Obiwarn 's question. I'd like to debounce an async function making requests to an API. Would I be correct to assume the leading option will invoke my debounced function and only delay passing me back the result?

privatenumber commented 3 years ago

Ended up using https://github.com/sindresorhus/p-debounce

thelfensdrfer commented 3 years ago

Why is this issue closed?

acicali commented 3 years ago

It was unclear to me why this problem existed... until I took one minute to think about it.

We use debounce to not call the callback no matter how many times we run it until a given amount of time has passed between calls. At the same time, we're expecting that each time we run it, it'll pass back the promise created within the callback. These expectations contradict themselves.

So this will never work:

/* BAD EXAMPLE */
let debounced = lodash.debounce(function(){
    return someAsyncOperation();
}, 500);

debounced().then(function(results){ // DOH! debounced() doesn't return a promise
   console.log(results);            // on calls where it didn't run your callback,
});                                 // so there is no `then` property

Just use a different pattern for debouncing, something like this:

let debounced = lodash.debounce(function(callback){
    return callback();
}, 500);

debounced(function(){
    someAsyncOperation()
        .then(function(results){
            console.log(results, 'yay!');
        });
});

We can't reasonably expect lodash debounce to handle this as it wasn't created for this purpose. Perhaps an entirely new debounceAsync method would be in order. There are probably a lot of opinions on how that should behave though.

dlqqq commented 3 years ago

TL;DR lodash's debounce implementation always returns the result of the previous invocation of the specified callback argument. For example, if you set the debounce timeout to 500ms, and call the debounced function at 750ms, you receive the Promise returned from the invocation at 500ms. In the diagram below, X represents the times when the debounced function is called and | represents the times when the callback argument is invoked.

0ms              500ms     750ms               1250ms
X|-----------------|----------X-----------------|

The ideal behavior is that for asynchronous callback arguments, we want the async debounced function to not return the Promise from a previous invocation, but rather be a Promise that resolves only upon the next invocation of the callback argument.

dlqqq commented 3 years ago

My custom solution in TypeScript:

export function debounceAsync<T, Callback extends (...args: any[]) => Promise<T>>(
  callback: Callback,
  wait: number,
): (...args: Parameters<Callback>) => Promise<T> {
  let timeoutId: number | null = null;

  return (...args: any[]) => {
    if (timeoutId) {
      clearTimeout(timeoutId);
    }

    return new Promise<T>((resolve) => {
      const timeoutPromise = new Promise<void>((resolve) => {
        timeoutId = setTimeout(resolve, wait);
      });
      timeoutPromise.then(async () => {
        resolve(await callback(...args));
      });
    });
  };
}
zameschua commented 3 years ago

@diracs-delta I had this exact issue today and found your solution. Thank you so much!

TDola commented 3 years ago

@diracs-delta That works pretty good. Any way to fix the meaning of this when inside the callback?

ChrisBlueStone commented 2 years ago

@diracs-delta Your solution almost worked for me, except since each call to the returned function would return a new promise, only the last promise would ever get resolved, leaving anything awaiting on the earlier promises (ones made before the timeout completes) stuck in limbo.

lucasDechenier commented 2 years ago

Hello guys. Can anyone else help, I'm having the same problem.

Any javascript solution using lodash ?

zameschua commented 2 years ago

@lucasDechenier Just use @dlqqq 's solution and remove all the types (Requires es6 though)

export function debounceAsync(
  callback,
  wait,
): (...args) =>  {
  let timeoutId = null;

  return (...args) => {
    if (timeoutId) {
      clearTimeout(timeoutId);
    }

    return new Promise((resolve) => {
      const timeoutPromise = new Promise((resolve) => {
        timeoutId = setTimeout(resolve, wait);
      });
      timeoutPromise.then(async () => {
        resolve(await callback(...args));
      });
    });
  };
}
moontrio commented 2 years ago

my simple debounce async function

type AsyncFunction = (...args: any[]) => Promise<any>;

export default function debounceAsync(fn: AsyncFunction, wait: number) {
  let timeoutId: NodeJS.Timeout | undefined;

  return function (...args) {
    clearTimeout(timeoutId);

    return new Promise((resolve, reject) => {
      timeoutId = setTimeout(() => {
        fn(...args)
          .then(resolve)
          .catch(reject);
      }, wait);
    });
  };
}
alzalabany commented 1 year ago

if what you need is to call a promise at most once every N ms

export function debounceAsync<T, Callback extends (...args: any[]) => Promise<T>>(
  callback: Callback,
  wait = 1000
): (...args: Parameters<Callback>) => Promise<T> {
  let promise: Promise<T> | null = null;

  return (...args: any[]) => {
    if (promise !== null) {
      // never been called;
      return promise;
    }

    promise = callback(...args).finally(() => {
      setTimeout(() => {
        promise = null;
      }, wait || 0);
    });

    return promise;
  };
}

if you want o call it once with a gap of N; you can adjust list of promise = to add

    promise = new Promise(resolve=>setTimeout(resolve,wait)).then(()=>callback(...args)).finally(() => {

now can this leak memory.. I don't think so, but I did not test tbh.

synox commented 11 months ago

I ended up using https://github.com/sindresorhus/p-debounce instead

yantakus commented 5 months ago

Why not add another method to lodash that works with promises?

mryellow commented 2 months ago

Surprising most of these pass.

When { leading: false, trailing: true } the first calls have an undefined response as they are being debounced.

Might be difficult to determine that the expected response will be a promise and return one to resolve later on the trailing edge.

import _ from 'lodash';
import { expect } from 'chai';
//import debounce from '../src/debounce.js';

const delayedResolve = (wait = 250, result = 'OK') => {
    return () => {
        return new Promise(resolve => {
            setTimeout(() => {
                if (_.isFunction(result)) {
                    Promise.resolve()
                        .then(() => result())
                        .then(resolve);
                } else {
                    resolve(result);
                }
            }, wait);
        });
    };
};

describe('debounce', () => {
    beforeEach(() => {

    });

    it('should provide a function', () => {
        expect(_.debounce).to.be.a('function');
    });

    it('should execute immediately when "leading"', () => {
        const debouncedResolve = _.debounce(
            delayedResolve(250),
            500,
            { leading: true }
        );

        const started = new Date().getTime();
        debouncedResolve().then(() => {
            const finished = new Date().getTime();
            expect(finished - started).to.equal(250);
        });
    });

    it('should execute immediately when "leading" and "trailing"', () => {
        const debouncedResolve = _.debounce(
            delayedResolve(250),
            1000,
            { leading: true, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve();
        debouncedResolve().then(() => {
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
    });

    it('should execute at end when "leading" and "trailing"', () => {
        const debouncedResolve = _.debounce(
            delayedResolve(250),
            1000,
            { leading: true, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve();
        debouncedResolve().then(() => {
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
    });

    it('should resolve initial promise with initial result when "leading" and "trailing"', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(250, () => cnt++),
            1000,
            { leading: true, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve().then(res => {
            expect(res).to.equal(0);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(250);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
    });

    // FIXME: Could return a promise then resolve it on the trailing edge
    it('should resolve initial promise with final result when only "trailing"', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(250, () => cnt++),
            1000,
            { leading: false, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
    });

    it('should resolve any promises made in middle with final result', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(250, () => cnt++),
            1000,
            { leading: true, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve().then(res => {
            expect(res).to.equal(0);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(250);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
        debouncedResolve().then(res => {
            expect(res).to.equal(5);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(1000);
        });
    });

    it('should continue to delay when "leading" promise is pending', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(2000, () => cnt++),
            500, // Debounce smaller than delay
            { leading: true, trailing: true }
        );

        const started = new Date().getTime();
        debouncedResolve().then(res => {
            expect(res).to.equal(1);
            const finished = new Date().getTime();
            expect(finished - started).to.equal(2000);
        });

        // Wait until after debounce delay, but before promise returns
        setTimeout(() => {
            debouncedResolve().then(res => {
                expect(res).to.equal(5);
                const finished = new Date().getTime();
                expect(finished - started).to.equal(2000);
            });
            debouncedResolve().then(res => {
                expect(res).to.equal(5);
                const finished = new Date().getTime();
                expect(finished - started).to.equal(2000);
            });
            debouncedResolve().then(res => {
                expect(res).to.equal(5);
                const finished = new Date().getTime();
                expect(finished - started).to.equal(2000);
            });
            debouncedResolve().then(res => {
                expect(res).to.equal(5);
                const finished = new Date().getTime();
                expect(finished - started).to.equal(2000);
            });
            debouncedResolve().then(res => {
                expect(res).to.equal(5);
                const finished = new Date().getTime();
                expect(finished - started).to.equal(2000);
            });
        }, 1000); // Larger than debounce wait
    });

    it('should return promises immediately when only "leading"', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(250, () => cnt++),
            500, // Debounce smaller than delay
            { leading: true, trailing: false }
        );

        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
    });

    // FIXME: Could return a promise then resolve it on the trailing edge
    it('should return promises immediately when only "trailing"', () => {
        let cnt = 0;
        const debouncedResolve = _.debounce(
            delayedResolve(250, () => cnt++),
            500, // Debounce smaller than delay
            { leading: false, trailing: true }
        );

        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
        expect(debouncedResolve()?.then).to.be.a('function');
    });
});