Promise then() only supports a single argument

[jhuckaby]

Hello there! This is an awesome module, and is exactly what I am looking for. I just have one question. When using the callback syntax, I can pass multiple variables after the 1st error argument, and they all show up in the callback function, like this:

var pb = require('promise-breaker');

var myFunc = pb.make( function(done) {
    done(null, "Hello", "World");
} );

myFunc( function(err, val1, val2) {
    console.log("Got here, in classic callback: ", val1, val2 );
} );

In the above example, val1 is Hello, and val2 is World. This is exactly what I expect. Console output:

Got here, in classic callback:  Hello World

But when I use the same exact setup but call myFunc using a promise pattern, I get an undefined for the 2nd argument:

myFunc().then( function(val1, val2) {
        console.log("Got here, in promise then: ", val1, val2);
} );

Console output:

Got here, in promise then:  Hello undefined

Am I doing something wrong here?

Thanks!

[jwalton]

Unfortunately, this is a Promise thing. If you try:

var  x= new Promise( function(resolve, reject) {resolve(1,2);});
x.then(function(y,z) {console.log(y,z);};

It'll print out 1, undefined.

The "easy" fix to this is, instead of calling done(null, "hello", "world"), call done(null, ["hello", "world"]). Of course, this only helps if you don't have some pre-existing API you need to conform to.

One thing I thought about doing when I wrote this was to check the arguments count in the callback, and then return arguments verbatim if I was calling another callback, or else automagically gather them up into an array like this if there was more than one of them when calling into resolve(). This is pretty similar to what Bluebird's promisifyAll() does.

This would be a pretty easy thing to do here. (And you don't even have to worry about fixing it in break, since it's impossible to return more than one parameter in the break case. :)

If this is a fix that works for you, I'll whip up a PR, get you to try it out, and then I'll push it as 3.0?

[jhuckaby]

Ah, darn, I was afraid of that. Thanks for the explanation and quick reply.

Oh wow, sure, that sounds great! Just so I understand, you're saying that the classic callback case would not change, and continue to work as it does now, returning N inline arguments. But the promise resolve would return an array of arguments if and only if more than 1 was passed?

[jwalton]

Yeah, exactly. I'd want to make it a major number change, just in case anyone is accidentally passing more arguments than they're supposed to today. :) But the whole point of promise-breaker is supposed to be to make it easy to convert existing libraries to use Promises, so I definitely want a solution that doesn't change how an existing callback function works.

[jhuckaby]

Perfect, that sounds totally awesome. Yeah, I have an existing API that I'm trying to promiseify, which returns multiple arguments for some of its methods.

Totally understood on the major version bump. Thanks again -- I look forward to testing v3 out!

[jwalton]

Ok. There's a PR here:

#4

(There's a few other minor changes in master, too, because I added eslint and fixed all the things it found. Shouldn't be any behavior changes from this, though, so I just pushed that straight to master before cutting this branch.)

If you could please have a read through the README.md changes and make sure they make sense, and try this out and make sure it works for you, that would be awesome. :)

[jhuckaby]

PR looks good to me. The only thing I would change is perhaps the wording in the docs:

...if fn passes more than two arguments to callback(...),
+then (as of v3.0.0) these arguments will be transformed into an array and passed to the Promise...

I am wondering if this will trip anyone up, because technically only the 2nd argument onward is going into the array. The first argument is of course the "error" and isn't included in the promise resolve. Perhaps this wording would be a bit clearer:

if fn passes more than two arguments to callback(...),
+then (as of v3.0.0) any arguments after the error will be transformed into an array and passed to the Promise as a single combined argument.

Not sure if that is an improvement, but no big deal. Your code example makes it perfectly clear what is going on anyway. The PR is golden, sir!

[jwalton]

I like it. I made another minor change after re-reading it, too. Published as v3.0.0.

[jhuckaby]

Works perfectly, thank you so much!