Implement FutureJs.toPromise

[seprich]

Even if conversion from (js)Promise to Future is often sufficient
on the assumption that all following computation is done within the
Future monad, it is still sometimes necessary to convert Future
back to Promise. E.g. When writing aws lambdas the ultimate return
value must be a promise.

[coveralls]

Pull Request Test Coverage Report for Build 38

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+1.5%) to 86.131%

---

Totals
Change from base Build 34:	1.5%
Covered Lines:	107
Relevant Lines:	121

---

💛 - Coveralls

[gilbert]

This looks great. Can you do a quick rebase? I think there's a small redundancy in the test file now.

[scull7]

Should we always assume that the future is a future of a result?

[seprich]

Considering that FutureJs.fromPromise converts Js.Promise.t('a) => Future.t(Belt.Result.t('a, 'e)) then yes in my opinion the toPromise should have reverse type signature. Using explicit type signatures would make this clearer but I have followed your convention here to leave all types to be inferred.

[gilbert]

@scull7 has a point. How does one translate a non-result future to a promise? It's possible we should have both toPromise and resultToPromise – with the current implementation renamed to resultToPromise, to be clear.

[scull7]

FutureJs.fromPromise by necessity must convert the Js.Promise.t('a) to something which denotes the possibility of an error unless we're resigned to allowing an exception to be thrown. The converse does not hold for a Future.t('a). While I agree that fromPromise and toPromise are sister methods, I don't know that it's obvious and I can see an opportunity for confusion. If I have some Future.t('a) given to me by some library and I just want to turn it into a Js.Promise.t('a) I now have to know the relationship with fromPromise to understand why it must be a Future.t(Result.t('a, 'b)).

edit: My suggestion for the implementation is below. I apologize if this came off as harsh in any way.

let toPromise = (future) =>
  Js.Promise.make((~resolve, ~reject as _) =>
    future->Future.get(value => resolve(. value))
);

Perhaps there should be a convenience method for Future.t(Result.t('a, 'b))?

[seprich]

1. translating non-result future would be quite trivially:

otherFuture -> Future.map(Belt.Result.Ok) -> FutureJs.toPromise()

2. However I understand your desire to support plain future in the first place so yes I can make those changes.

[scull7]

Looks good to me.

[MoOx]

Any plan to ship this in a release? =)

[gilbert]

Done!