Future rework

[emmanueltouzery]

this is the PR that I promised in pr #8.
it changes the Future api in a number of ways and of course that would mean a new major version of prelude. On top of that I have a few other Future improvements in mind, but that's the core of the changes (what I have in mind besides this are additions not changes).

@RPallas92, @qm3ster if you have any feedback let me know (now or in the future of course, including after I merge this PR).

Here is a summary of the changes:

1. Future is now eager (autostarts when created, like Promise)
2. Add the Future.do call (merge PR from @RPallas92, change his apidoc)
3. Improve/fix Future.onFailure, onComplete and onSuccess: the old implementation was returning new Futures, now we return 'this'. The old implementation was incorrect, this is an API change so I'm bringing it up in this PR too
4. modify ofCallbackApi to take the same parameters as 'new Promise', and rename it to 'ofPromiseCtor' (thanks @qm3ster for the suggestion)
5. add Future.ofCallback (thanks @qm3ster for the suggestion) -- node-style callbacks. Maybe could still rename to ofNodeCallback?

It's quite something but in the end porting code using the current implementation of Futures shouldn't be that bad. Any feedback on these changes (naming, implementations,...) is welcome. I might merge this PR quite fast, maybe before you have time to review it, if that's the case feel free to open an issue to discuss the changes, even after the merge.

[emmanueltouzery]

Regarding @qm3ster's idea of lifting promises I don't know how to write a lifting function that would work across all arities, but we could add Function2.liftFuture, Function3.liftFuture.. We already have a number of lift functions there. I didn't do it yet in that PR though, not 100% sure whether to add (or if there's a better way?).

[emmanueltouzery]

Another change in the PR which I forgot to list:

6. remove Future.orElse, we now have Future.recoverWith instead

[qm3ster]

lol random
Probably put something crossplatform (like a file from inside the repo) or your own (spy?) function.

[qm3ster]

*failed

[qm3ster]

Could we store a reference to the original promise and return that instead?

const unit = Promise.resolve()
const a = Future.of(Future.of(unit).toPromise()).toPromise()
const b = Future.do(()=>unit).toPromise()
console.log(...[a,b].map(x=>x===unit))
// > false false

😱 😱

[emmanueltouzery]

hmm I'm not sure if that's really important it's the same Promise. We don't get much from that, but if we must keep the original promise then we do waste some memory potentially. The fact it's a different promise is the reason that I named it toPromise and not getPromise (the apidoc also says 'get a Promise', not 'get the Promise'). And also the fact that there's an underlying promise is (theoretically at least) an implementation detail.

One thing that could trip a user could be more the fact that if you call like three times toPromise you get three different promises.

But I still don't know whether that's worth the extra memory use of keeping the original promise hanging around. I mean it's not much memory, it's just if you have a list of tens of thousands of futures or something.. I don't know, do you think it's worth the memory use?

[qm3ster]

Not sure. It's (keeping 2 Promises around vs one) vs (creating a promise for every call to toPromise vs giving out references to the same one)

first harms

const f = Future.of(Promise.resolve(0))
new Array(100000).fill(0).map(_=>f.toPromise())
// 2 Promises

new Array(100000).fill(0).map(_=>Future.of(Promise.resolve(_))
// 200000 Promises

while the second harms

const f = Future.of(Promise.resolve(0))
new Array(100000).fill(0).map(_=>f.toPromise())
// 100001 Promises

new Array(100000).fill(0).map(_=>Future.of(Promise.resolve(_))
// 100000 Promises

[emmanueltouzery]

I mean in the second approach you sometimes get more if toPromise is called multiple times on one Future, but the extra ones get GC'd immediately (when you get rid of the Future). I don't know, it doesn't seem worth changing.

I think building the Promise is very cheap, I think promise equality needn't be a strong requirement. If you make a PR maybe you convince me otherwise, but currently I don't see myself changing that.

[emmanueltouzery]

Thank you for the feedback @qm3ster. I fixed two of the three issues that you raised (good points, thanks for the suggestion to pick a file from the project for the node test). For the third one I answered, I would probably leave it as-is, but I'm listening if you really think that it's a flaw, do answer to what I wrote.