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.
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.