Added support for .toPromise(PromiseConstructor)

[Ginden]

I'm not sure about error message handling and tests.

Did you consider splitting test/test.js to many files?

[vqvu]

Can you name the parameter PromiseCtor or something similar to highlight the fact that we do not use the global Promise object.

[vqvu]

Update docs.

You can just say the stream must emit at most one value or error and that the promise will resolve to that value when it is emitted, then link to [toCallback](#toCallback) for details.

[vqvu]

error message

I would factor out the toCallback handler in such a way that you can customize the error message.

function toCallbackHandler(transformName, cb) {
    var value;
    var hasValue = false; // In case an emitted value === null or === undefined.
    var hasValue = false; // In case an emitted value === null or === undefined.
 
    return function (err, x, push, next) {
        ...
        throws new Error(transformName + ' called on stream emitting multiple values');
        ...
    };
}

addMethod('toCallback', function (cb) {
    this.consume(toCallbackHandler('toCallback', cb)).resume();
});

addMethod('toPromise', function (PromiseCtor) {
    return new PromiseCtor(function(resolve, reject) {
        this.consume(toCallbackHandler('toPromise', function (err, x) {
            ...
        })).resume();
    });
});

tests

The tests themselves look fine, but can you organize them into suites instead (e.g., like exports.onDestroy). This will make it easier to run the entire suite by itself.

exports.toPromise = {
    ArrayStream: function ...
    'returns error for streams with multiple values': function ...
    ...
};

Did you consider splitting test/test.js to many files?

Yep! I would love that; similarly with lib/index.js. It's just a lot of boring work, so no one has volunteered to do it yet.

[protometa]

What if PromiseCtor was optional and defaulted to global Promise? Best of both worlds?

[vqvu]

I had considered making PromiseCtor, and I didn't suggest it for two reasons.

1. We expose these methods at the top-level as curried functions, and currying does not work well with optional arguments, so we try to stay away from them. This is why the sortBy and splitBy variants exist as separate methods from sort and split rather than having the latter methods have an optional argument.

2. It doesn't solve the underlying problem, which is that it would be possible to write highland code that, but will behave differently depending on the Javascript environment. The way I see it, having PromiseCtor be optional would be no different than having two separate methods: toPromise() and toPromiseWithCtor(PromiseCtor), and in such a scenario, I would still oppose to having toPromise().

---

@Ginden, sorry I haven't responded to your latest changes. I don't get an email notification when you update the commits in your PR, so you have to make a comment. Are you planning on placing the tests in a separate tests/toPromise.js file? If so, please let me know when you are done.

[Ginden]

. Are you planning on placing the tests in a separate tests/toPromise.js file?

I do not plan it.

[vqvu]

Can you remove the file from the PR then?

[Ginden]

@vqvu Done.

[vqvu]

Thanks!