bindNodeCallback returns incorrect result if undefined success argument in callback

[stusherwin]

RxJS version:
5.0.0-beta.6 but bug is present in master as of f8ba77d

Code to reproduce:

let nodeFunction = function(param1: string, param2: string, callback: any) {
  // Bug only manifests when success parameter is implicitly undefined
  callback(null /*error*/); // no success parameter
}

Observable.bindNodeCallback(nodeFunction)('a', 'b')
  .subscribe(succ => console.log(succ));

Expected behavior:
undefined

Actual behavior:
[]

Additional information:
This is a bug IMO because it does not preserve the behaviour of the wrapped method. There are actual libraries in production that have this behaviour, e.g. node-sqlite3 (which is how I found it):

Database#get(sql, [param, ...], [callback])

Runs the SQL query with the specified parameters and calls the callback with the first result row afterwards [...] The signature of the callback is function(err, row) {}. If the result set is empty, the second parameter is undefined, otherwise it is an object containing the values for the first row.

The problem appears to be in this line:
https://github.com/ReactiveX/rxjs/blob/master/src/observable/BoundNodeCallbackObservable.ts#L109

It could be fixed with something like this:

if(!innerArgs.length) {
  subject.next(undefined);
} else {
  subject.next(innerArgs.length === 1 ? innerArgs[0] : innerArgs);
}

But there's probably a more elegant way of doing it!

[trxcllnt]

@stusherwin you're right, that's the correct fix. Happy to review if you want to submit a PR.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.