callback may has multiple arguments

[lmk123]

Most of chrome.* API 's callback only has one argument, but someone not, for example:chrome.hid.receive(integer connectionId, function callback)

And, the native resolve() function only accept one argument, even though resolve.apply(null, arguments ).

So below code only get the first argument:

var cp = new ChromePromise();
var connectionId = 4;

cp.hid.receive( connectionId ).then( function( reportId, data ) {
  console.log( reportId ); // 3
  console.log( data ); // undefined
}  );

I think when callback has multiple argumnets, we need put the whole arguments like resolve(arguments) but not resolve.apply(null,arguments).

So the above code will be:

var cp = new ChromePromise();
var connectionId = 4;

cp.hid.receive( connectionId ).then( function( args ) {
  var reportId = args[0];
  var data = args[1];
  console.log( reportId ); // 3
  console.log( data ); // ArrayBuffer
}  );

[tfoxy]

Thanks for the PR.

I'm not sure if this is the best way to add this feature. I can see a few problems:

1. This behaviour may be unexpected if someone only wants the first parameter.
2. If some version of chrome adds a second parameter to a callback, it would break the resolve function.

I will check more functions on the chrome API that use multiple parameters and then I will try to come up with other solutions to this problem.

Will keep in touch.

[lmk123]

If some version of chrome adds a second parameter to a callback, it would break the resolve function.

I think so. Maybe always resolve the whole arguments array is the best way?

// change resolve.apply(null, arguments); to
resolve(arguments);

[lmk123]

I got a solution.

If we pass true as the first parameter, then the promise return the whole arguments:

var cp = new ChromePromise();
var connectionId = 4;

// if pass true as first param
cp.hid.receive(true, connectionId ).then( function( args ) {
  var reportId = args[0];
  var data = args[1];
  console.log( reportId ); // 3
  console.log( data ); // -------> ArrayBuffer
}  );

// otherwise
cp.hid.receive( connectionId ).then( function( reportId, data ) {
  console.log( reportId ); // 3
  console.log( data ); // --------> undefined
}  );

I use this solution in my chrome-call, but I'm not sure there isn't have any chrome api that exactly use true as first param.

[tfoxy]

I like that solution. From what I can see, the chrome developers prefer to use an object instead of a boolean.

If you update the PR, I will merge it.

[amio]

IMHO, the true param still looks a little tricky, how about adding a new method:

cp.hid.receiveCombine( connectionId ).then(function ( args ) {
  // args === [reportId, data]
})
cp.hid.receive( connectionId ).then(function ( reportId ) {
  // warn user will only get the first result for regular callback.
})

Consider it's like the relation between fs.readFile() and fs.readFileSync(). PS, English is not my native language, you probably can choose an other word better than "Combine", that's just a demonstration of the idea.

[tfoxy]

I think that would be the best way. It would be similar to Promise.promisifyAll from bluebird.

I was thinking of changing the constructor to only have one argument: options. Then it would be possible to choose the suffix for this methods.

English is not my native language either. So I will take some time to think of a better word. But Combine is a good start.

[riggs]

I just submitted a PR with a similar change, but also using the spread operator to be more explicit about what is happening.

[riggs]

Regarding amio's suggestion, I don't know of any way to determine which functions need the 'combine' version for multiple callback arguments. Would you just add it for every function, even though the vast majority of callbacks only take one argument? Perhaps you could wrap the native functions to give them a .multipleArgs method for this functionality without polluting the chrome.promise namespace.

I still think simply returning an array of arguments when the callback takes multiple is the simpler, saner approach. And while it's technically a compatibility-breaking change, I think it's a much better API than having two different versions of the function, especially when the default (and current implementation) is to throw away data.

[tfoxy]

Yeah, it's a simpler API. And it's better to not drag this any longer.

In any case, I will release it as a new major version because it can break some existing code.

[tfoxy]

I'm closing this as the issue is now solved. Sorry for taking so long. I'm not longer maintaining a chrome extension so I don't have the need of this library.