search

emmanueltouzery / prelude-ts

Functional programming, immutable collections and FP constructs for typescript and javascript
ISC License
377 stars 21 forks source link

Future rework #11

Closed emmanueltouzery closed 6 years ago

emmanueltouzery commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Another change in the PR which I forgot to list:

  1. remove Future.orElse, we now have Future.recoverWith instead
emmanueltouzery commented 6 years ago

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.