sindresorhus/debounce

Add support for promises

kilianc opened this issue · 8 comments

debounce is an async function by definition and it doesn't have a callback or any sort of completion handler. This makes it hard to test and use with promises or any I/O.

We should at at least return a promise.

debounce(mything, 100)()
  .then(() => console.log('done after 1s!'))

This is not a breaking change even. Phase two would be to just proxy what mything returns through the promise chain and being able to .catch

debounce(mything, 100)()
  .then(() => console.log('done after 1s!'))
  .catch((err) => console.log('failed after 1s!: %s', err.message))

where

function mything() {
  return Promise.resolve('something')
}

I can open a PR for this, as soon as the maintainer is fine making it return a Promise only when it's available on the runtime environment. In other words, if you are using oldIE then you don't get a Promise as a result if you don't load a polyfill. That also looks backwards compatible.

It might be worth it as a separate package at that point, tbh.

The first use of the debounced function which should return a promise was returning undefined

import debounce from 'debounce'

const search = (search) => {
  return new Promise((resolve,reject)=>{
   // ...
    resolve(res.data)
  })
}

const searchDebounce = debounce(search, 250)

searchDebounce("foobar").then() // threw error, promise wasn't returned, undefined was returned

// sometime later

searchDebounce("foobar").then() // working as intended

I was able to work around the issue by switching out debounce for debouce-promise.

https://github.com/bjoerge/debounce-promise

That's the difference between debounce and throttle.

So this issue seems to be alive.
Back then I ended up publishing debounce-with-result.

This seems like a 100% reasonable thing to expect from this library. I'll make sure I have permissions to do so tomorrow and try to get a working version published to npm.

EDIT - PR welcome. Will merge and publish, but the change is more involved than I need to be in this library right now (only incidentally stumbled upon this issue :/)

I mean, if https://github.com/bjoerge/debounce-promise does what we want, we don't need to over-complicate this library and just add a mention in the README.md. Unless the maintainers really care about this feature!

I don't think promise supports needs to live in this package. There's debounce-promise as mentioned above and also my p-debounce package.