0

We've got a number of libraries that currently support callbacks that we're updating to support Promises. There are two approaches I'm looking at.

  1. Default to cb behavior and do something special if a Promise is needed
const cbAndPromiseOption1 = cb => {
    if(!cb){
        return new Promise((resolve, reject) => {
            cbAndPromiseOption1((err, data) => {
                try {
                    err ? reject(err) : resolve(data);
                } catch (uncaughtErr) {
                    /*handle uncaught error from reject / resolve*/
                }
            })
        })
    }

    let err; 
    let data;
    //...do stuff
    cb(err, data);

}
  1. Default to Promise behavior and do something special if cb is provided
const cbAndPromiseOption2 = cb => {
    let promiseToReturn = new Promise((resolve, reject) => {
        let err; 
        let data; 
        //...do stuff

        err ? reject(err) : resolve(data);
    });

    if(cb){
        promiseToReturn = promiseToReturn.then(data => cb(null, data)).catch(err => cb(err)).catch(uncaughtErr => {/*handle uncaught error from cb to avoid unhandled promise errors*/});
    }

    return promiseToReturn; 
}

My question is whether these are functionally equivalent or if there's some obvious difference I'm overlooking. Option 1 is the approach I've seen in numerous places, but Option 2 is more appealing to me because it gets the dev in the mindset of Promise-first behavior.

In any case, any feedback on which of these approaches is "better" would be very much appreciated.

Caleb Weeks
  • 1
  • 1
    `cbAndPromiseOption2` always returns a Promise whereas `cbAndPromiseOption1` only returns a Promise if there is no callback supplied - the first option seems to be a more likely solution – Jaromanda X Aug 28 '19 at 06:31
  • Checkout [this](https://nodejs.org/dist/latest-v10.x/docs/api/util.html#util_util_promisify_original) – Vibhas Aug 28 '19 at 06:46
  • 1
    When you are invoking the above function you don't want to check whether to use `.then` or not so I would suggest using a common type of return. accept a `callback` or return a `promise`. so Option 2 is more feasible. – AZ_ Aug 28 '19 at 06:55
  • "*handle uncaught error from reject / resolve*" - `reject` and `resolve` do never throw. you don't need that `try`/`catch`. – Bergi Aug 28 '19 at 07:53
  • 1
    [`.then(data => cb(null, data)).catch(err => cb(err))` is problematic](https://stackoverflow.com/q/24662289/1048572) – Bergi Aug 28 '19 at 07:54
  • "*Option 2 is more appealing to me because it gets the dev in the mindset of Promise-first behavior.*" - yes, I'd say this is the way to go. Especially if you have functions in your library that call some other of your functions, they can directly benefit from *using* promises. – Bergi Aug 28 '19 at 07:57

0 Answers0