[RFC] New Promise Binding

ryyppy · January 24, 2021, 1:44pm

Hello everyone!

For the past few weeks, we have been working hard on a proposal for replacing the original Js.Promise bindings that are currently shipped within the ReScript compiler, and we think we finally settled on a design we are happy with to share with the broader community.

Installation

We ship our new bindings as a third party npm package to be able to collect more feedback from our production users to see if the binding is practical enough to be considered as a Js.Promise replacement.

The bindings can be installed alongside your existing Js.Promise code.

npm install @ryyppy/rescript-promise --save

Don’t forget to update your bsconfig.json as well:

{
  "bs-dependencies": ["@ryyppy/rescript-promise"]
}

The new bindings are globally accessible via the Promise module.

The source code and issue tracker can be found here: github.com/ryyppy/rescript-promise.

Proposal Document & Examples

The rationale and decision making for the new APIs can be found in our PROPOSAL.md document.

Here is a quick usage example (more examples can be found in the README as well):

type user = {"name": string}
type comment = string
@val external queryComments: string => Js.Promise.t<array<comment>> = "API.queryComments"
@val external queryUser: string => Js.Promise.t<user> = "API.queryUser"

open Promise

queryUser("patrick")
->then(user => {
  // We use `then` instead of `map` to automatically
  // unnest our queryComments promise
  queryComments(user["name"])
})
->map(comments => {
  // comments is now an array<comment>
  Belt.Array.forEach(comments, comment => Js.log(comment))
})
->ignore

The repository also comes with an examples directory for more thorough “real world scenarios”, such as using and chaining some fetch promises.

The Proposal Process

We are convinced that our solution is hitting the right spot of being practical enough, while yielding efficient and idiomatic JS code without introducing too much complexity on the type level.

I’d ask everyone in the community to install / vendor our new Promise bindings, use all the proposed features and give us feedback in the rescript-promise issue tracker.

We’ve set a first review deadline of 2 months (until 2021-03-30T22:00:00Z), where we’ll assess if the bindings are acceptable to be shipped as a core binding. If there’s any deal-breaking issues, we will postpone the deadline and repeat the feedback loop if necessary.

Feedback Format

Please make sure to provide real-world use-cases from your actual app code to give proper context on potential problems. Our main goal is to stay JS runtime free, and also to stay close to JS API semantics.

Even though the final library looks pretty minimalistic, it took as several iterations to consider different solutions (some with runtime overhead, some without, etc). The simplest solution turned out to be the most practical for us, so we are happy that we don’t need to introduce a bunch of extra JS runtime code, just to be able to interact with promises efficiently.

We hope you like it!

yawaramin · January 24, 2021, 7:49pm

Hi, some feedback:

I would recommend to not do this–the reason-promise library already uses Promise in the global scope and adding this library will conflict with any project that uses that (even as a transitive dep, if I’m not mistaken). In the future this can be solved by putting Promise in Belt, so perhaps not a long-term issue. In the meantime I would recommend a namespace like Belt__Promise or something.

Basically, ReScript already ships with four globally namespaced modules–the goal should be to keep it as minimal as possible. OCaml learned this lesson the hard way and they are painstakingly migrating all their top-level modules inside Stdlib.

I assume people would use Promise.t here instead going forward.

I would recommend to normalize the function names with what Belt uses, for uniformity. E.g. introduce a Promise.forEach instead of using Promise.map(...)->ignore, and also introduce uncurried versions of the functions like Belt does for performance.

I also read the proposal doc and I think it’s mistaken about treating the nested promise problem as an unlikely edge case that will hopefully never happen, but then again these arguments already happened in ES promise proposals years ago so

I also wanted to say that the proposal doc is mistaken about Prometo–it’s not trying to ‘do too much’ or ‘trying to fix up Pomises, instead of taking them as they are’. Prometo is a type alias for Js.Promise.t<result<'a, 'e>>. This reuses the existing promise infrastructure while neatly solving two problems: (1) nested promises can’t be collapsed any more; and (2) errors in promises will be represented in a type-safe way (resolved with Error(...)) instead of rejecting at runtime and potentially crashing the app.

hoichi · January 25, 2021, 9:18am

I’m still not sure whether result-based approach is the way to go. You could as go the other way around: raise on matching an Error and handle it with Promise.catch. I mean, one upside of surrendering to promise poisoning is that the bindings to a lot of JS libraries are going to be much easier.

Result-based APIs would make more sense if we had (or had a goal to have) a whole lot of ReScript-specific libraries (like Elm has). So far those libraries failed to materialize (EDIT: no disrespect to the hard work by a lot of people, but there’s still not a lot of plug-and-play ReScript-specific libraries, compared to Elm), and the team moves towards better interop with the whole JS ecosystem.

Not sure about then. Since we have map already, maybe an explicit flatMap is going to be less ambiguous? Also, then will probably only trip up people coming from JS/TS, so they could benefit from early disambiguation too.

ryyppy · January 25, 2021, 9:23am

Yeah, the rescript-promise bindings are currently exposed as Promise, obviously we don’t want to propagate them as a global Promise within the rescript compiler.

For those who are currently running reason-promise in their stack, it would be probably better to just copy paste the Promise.res / Promise.resi from the rescript-promise repo in e.g. Promise2.res / Promise2.resi, just to try it out. I don’t think ppl should mix both libraries in general (although it’s perfectly fine, since both operate on the original Js.Promise.t type), so ideally they like it and migrate to rescript-promise, or they keep using their preferred promise version.

I wanted to keep the Promise module name though, because I am pretty sure only the parent module name will be decisive. Also I wanted to see how many users will complain about colliding module names. My hypothesis was that a lot of ppl are using their own hand-rolled promise implementation / third party library of some sorts.

It is not planned to call it Belt.Promise though, since this library tries to stay true to the zero-cost approach (so it’s actually a Js.Promise2 binding to be exact).

Cleaning up the stdlib modules / Js namespaces will be a separate topic that is unrelated to this proposal, but I agree, It’s definitely too many namespaces right now.

Yes, or whatever other namespace we decide on. I used this particular example to demonstrate that you are able to just use your Js.Promise.t and Promise.t type interchangeably.

When I started these bindings, I wanted to have a runtime, because I thought the Promise bindings would be fundamentally broken and unusable if nested promises wouldn’t be addressed. Now after several runs of trial and error (and writing app code with it), I suddenly changed my mind, because as I stated in the trade-off section, it would be really interesting to first see real-world issue reports where nested promises caused an issue. If we can get away with this dirty bit, we get really clean zero-cost bindings (see the Promise.js file). Only the catch function is actually somewhat involved.

On a theoretical level I agree. It feels weird to leave these known issues untreated, but on the other hand, as your Prometo library states correctly, every promise should be handled at some point. The same applies for rescript-promise, because in the end you can just introduce your catch call and be done with it.

I think the best comparison would be to Java’s checked vs unchecked exceptions: Whereas checked exceptions (intermediate results) force you to either implicitly map / handle each result between each promise transformation, the unchecked version will fail during runtime if you don’t handle a error correctly. This is also analogous to synchronous code (you could leave out a try / catch). The unchecked version is more performant and therefore a better fit as a smallest-denominator binding. It’s easy to build a type-safe version on top of unsafe code, but it’s hard to do it the other way around.

There need to be trade-offs in one sort or another, we are currently gauging if the trade-offs are worth it.

First off: I want to say that, for what complex topic Prometo tries to solve, the implementation is actually very lean. There is no doubt it’s a good library if you take the extra overhead / type complexity to be on the safe side, and it also gave me some good insights on how others would handle promises during runtime.

BUT, I still believe it tries to tackle way more problems that it should for a low-level binding, namely:

There’s a differentiation between a Prometo.t and Js.Promise.t, although both map to native promises which causes mental overhead
It uses type concepts that are rather unique: type t(+_, 'e) constraint 'e = [> error ];
It obfuscates the semantics of then (which is a flatMap), so you need to learn new concepts
It introduces some JS runtime (created a gist of Prometo.bs.js)
It introduces cancelation concepts (which might be good as an external wrapper?)

Additionally (but this is more related to the cross-usage with OCaml probably), it uses pipe-last apis and labeled arguments, so everything that a JS developer knows from the Promise api has been completely erased in the final ReScript API. A goal for us was to keep it very close to the original Promise APIs without introducing too much runtime code.

Also due to the lack of full examples, I had a hard time understanding how one is supposed to match on specific ReScript vs JS errors. It’s probably easy to do, not sure. I also tried to convert my FetchExample.res to Prometo, but got stuck unfortunately (any pointers on how to convert this example 1-to-1 to Prometo would be highly appreciated).

I tried to add Prior Art, so I can credit the folks who have been investing their time in solving the issue, instead of trying to reinvent everything myself. So I hope it doesn’t sound like I am talking down on your approach (if so I will rephrase it). All of them are viable.

ryyppy · January 25, 2021, 9:37am

I only introduced map because I really had no other choice, if I wanted to have an easy way to transform a promise mapped value to another … map is pretty well known in the JS world ([].map, etc.) so it was easier for me to introduce map than flatMap (and honestly, in my whole JS career, I’ve never seen a [].flatMap in the wild, which is weird).

then is technically a flatMap, so I tried to choose one for another, and then is more familiar and maps directly to the JS output as well. I really dislike that I had to introduce a map, just to be able to transform a value more conveniently without explicitly calling resolve all the time, especially because map maps to then in the JS source.

hoichi · January 25, 2021, 9:48am

Well, if you expect users to write a lot of their own bindings, maybe you can also expect them to handle two ReScript bindings to a single JS function I mean, for me the interop is probably the hardest part of ReScript, so if newcomers are anything like me, and they can handle marrying the ~~OCaml~~ ReScript type system with the JS reality, they really should be able to learn what map and flatMap map to, and why there are two of them. If anything, this difference could be a part of the beginners’ tutorial.

Kingdutch · January 25, 2021, 10:02am

I understand wanting to cater to JavaScript developers. However, I feel that dumbing things down for a resistance against using flatMap does a major disservice to ReScript and the previous work it does.

The reason I pushed Reason (now ReScript) in the organization I work for is because it has a more advanced and more eloquent way of dealing with types than JavaScript. Rust is one of the languages that has shown me the power of flatMap and I’m sad it doesn’t exist in JavaScript.

Your proposal document states the following about why you’re not a fan of reason-promise.

The APIs are harder to understand, and the library tries to tackle more problems than we care about. E.g. it tracks the rejectable state of a promise, which means we need to differentiate between two categories of promises.

“Harder to understand”, compared to what? For me figuring out that then and map are not the same is more difficult (to grasp and explain) than flatMap vs map. The latter more clearly not being synonyms that should perform the same function.

Additionally, I love the distinction between rejectable and non-rejectable promises. Since it means that I know that promises I make from reason (non-rejectable) will never blow up my program at runtime, whereas I need to tread carefully with rejectable promises. This distinction gives me compile time guarantees about run-time behaviour, which is very powerful (Rust is build around that as a core principle and is wildly successful because of it).

It also adds uncaughtError handlers, and result / option based apis, which are easy to build in userspace, and should probably not be part of a core module.

Being able to handle promises as result and option has been hugely beneficial in the code I’ve written. It means that I can use ReScript’s exhaustiveness checks to make sure that all cases of promises are properly handled. Again, moving run-time behaviour to compile time checks. The core type system should encourage the use of result and option so core modules should use it.

It has a preference for map and flatMap over the original then naming, probably to satisify its criteria for Reason / OCaml usage (let* operator)

See my earlier comment. It has preference for well established and clearly behaved functional method names, over somewhat more niche JavaScript operators that have ambiguous behaviours.

it uses list instead of Array, which causes unnecessary convertion (e.g. in Promise.all. This causes too much overhead to be a low-level solution for Promises in ReScript.

That’s a fair point, with the focus on JavaScript this is something that could be changed. This can be done without re-inventing the API though.

Finally regarding the matter of module name.

I wanted to keep the Promise module name though, because I am pretty sure only the parent module name will be decisive. Also I wanted to see how many users will complain about colliding module names.

I’d be the first user to complain about colliding module names. I use things built upon reason-promise and I use reason-promise directly. I’m curious to see how this will break code when other libraries start using this promise library.

My hypothesis was that a lot of ppl are using their own hand-rolled promise implementation / third party library of some sorts.

I hope not, I’ve seen reason-promise being consistently recommended when asking about this.

ryyppy · January 25, 2021, 10:49am

Compared to the proposed bindings. The thing that strikes mostly is the unfriendly error messages. reason-promise uses a type t<+a> = rejectable<'a, never> and type rejectable<+'a, 'e>, so that the promise can track the kind of error that is being rejected, which can result in rather chaotic error messages that are harder to resolve, depending on the complexity of the handled promise, and the individual skill of the developer.

Honestly, am I the only person in this community who feels like these errors are way to complex? I constantly had to struggle with them and getting really frustrated with very simple tasks, and I don’t consider myself as a newcomer.

That’s fine, you can continue using reason-promise then, and nothing will change on your side, or you could easily add the behavior yourself in a few more lines. The concept of rejectable types was something I explored in this PR, and I actually did my live-code stream about that (unfortunately the video is not published yet). Ah, and additionally there’s a branch for implementing the same thing with a runtime.

First, and most important step was to get the most simplistic solution done. Based on that, it’s easy to add your own specific helpers to express rejectable promises or result based APIs (see prev. mentioned PR).

With the current solution, mapping to a result based Promise is easy to do, and kinda equivalent to JS semantics:

fetch("https://reqres.in/api/login", params)
    ->then(res => {
      Response.json(res)
    })
    ->map(data => {
      // Notice our pattern match on the "error" / "token" fields
      // to determine the final result. Be aware that this logic highly
      // depends on the backend specificiation.
      switch Js.Nullable.toOption(data["error"]) {
      | Some(msg) => Error(msg)
      | None =>
        switch Js.Nullable.toOption(data["token"]) {
        | Some(token) => Ok(token)
        | None => Error("Didn't return a token")
        }
      }
    })
    ->catch(e => {
      let msg = switch e {
      | JsError(err) =>
        switch Js.Exn.message(err) {
        | Some(msg) => msg
        | None => ""
        }
      | _ => "Unexpected error occurred"
      }
      Error(msg)
    })
    // From here on we are handling a Promise.t<result<...>>

Also, in ReScript, both ReScript exceptions and JS errors are encoded as an exn type. Using a catch on a promise will catch every little single error scenario that can happen during runtime. You will get the similar safety guarantees with exhaustive pattern matching there.

Also that’s not 100% true. There are cases where promises in reason-promise can still blow up for various exceptional reasons, and then you need to use an independent onUnhandledException hook which by default prints the error on console instead.

So we have rejectable promises, unrejectable promises, and a few edge-cases where I need to handle errors in the onUnhandledException callback. Is this really necessary as the smallest baseline?

Just to be sure I mention it again: The published rescript-promise package is a call to action to try the proposal bindings. It’s a way to test drive the bindings and see if the general api makes sense, and feels ergonomic enough for a low level Js.Promise replacement. If it makes sense and proofs to be useful, the rescript-promise npm package will be deprecated and hopefully be upstreamed as Js.Promise2 or similar. Alternatively, just copy pasting that two dependency-free files and rename them within your app repo won’t be too hard either, just for test driving.

reason-promise allows lifting Js.Promise.t values, and rescript-promise is only handling an alias to Js.Promise.t. Both libraries should work the same way, everything is compatible with the Js.Promise.t type.

Does this make sense?

benadamstyles · January 25, 2021, 11:13am

I use flatMap every day at work, in pure JS. Slightly surprised that people think it is unknown in JS! It’s available in Node LTS.

hoichi · January 25, 2021, 12:08pm

It’s also available both in Chrome and in FF, but that per se doesn’t mean it’s widely used. Modern JS APIs are huge, so I’m not really surprised if some methods are used by less than 0.1% of frontend devs.

johnj · January 25, 2021, 1:39pm

I think this has been touched on, but just to be clear and manage expectations: this proposal is supposed to be a low-level replacement for the Js.Promise module, and not a replacement for reason-promise, prometo, etc? Users can continue to use those promise libraries without anything changing? And, in the future, new promise libraries will presumably build on top of this proposed API instead of the existing Js.Promise?

I think the comparisons to reason-promise may have given the impression this was supposed to replace it. If that was the case, then it would be obviously lacking. As far as a Js.Promise replacement though, then it seems like a nicer API. (Based on just glancing at it. I may have more feedback once I use it in practice.)

I agree that a flatMap function would be nice, even if it was just an alias for then. The comment that flatMap isn’t familiar to the JS world seems weird because: A) flatMap does exist in JS and B) flatMap is well established in the libraries that already ship with ReScript. I assume that consistency within the ReScript stdlib should be a priority.

ryyppy · January 25, 2021, 2:09pm

Yes, that’s correct. It is aiming to replace the existing Js.Promise bindings as the low-level primitive that all the other libraries such as reason-promise and prometo are using.

That’s probably the most realistic suggestion, having both functions with an alias (maybe with the other having a deprecation warning to point to the other function). Other alternative would be to remove the map function alltogether, then there is no point of confusion that there might be even a flatMap.

Honestly, my bigger concern is currently usage patterns. When telling someone to start from scratch, doing multiple async operations and chaining them together, how would they do it?

Does the type system yield user friendly errors?
does the promise nesting edge case cause problems?
Is error handling (ReScript errors / JS errors) approachable and easy to remember?

Or maybe another scenario: The user sets out to bind to a third-party js library function, creating a promise, and using it in the existing system. Is this intuitive and easy to do? Is there anything missing that prevents them from handling specific error scenarios, or other edge-cases (e.g. some promises may throw a JS value instead of an Error… does the user know how to access that error)?

These are really important questions I’d like to see answered at some point.

yawaramin · January 25, 2021, 3:37pm

OK, if this is intended to go in Js, then I agree that it should have (almost) no runtime. I had assumed it was a more opinionated ReScript approach, in the style of Belt.

alexeygolev · January 25, 2021, 6:15pm

I know that this might be considered “bikeshedding” but I’d like to voice my concern with using then especially for “js users” sake.
I might be missing something obvious but is there any common understanding that “js crowd” perceives then as flatMap? I’ve been using @ryyppy’s repo lately (this commit where then is actually map) and I can say that there is no way out of confusion for “fresh js crowd” with Promise bindings if then is used.

Some people will expect then to be map some to be flatMap and both functions will have to have a “Note:…” in the docs. Why not just skip the then to remove any chance of confusion all together. If one doesn’t use those functions when working with options they can learn (map is then when you don’t return a promise and flatMap is when you do) them instead of overloading the then.

It’s also a good idea to consider that some people write in both rescript and js (sometimes js for work and rescript while learning). Overloading then might not be that good in this case.

Ironically I would even prefer then and flatThen no matter how weird they might seem. Those names are clearer than then and map

alexeygolev · January 25, 2021, 6:22pm

On the subject of adding result to the bindings I agree with @ryyppy decision. I used reason-promise quite a bit and in our case it actually did more damage when the codebase grew. Most of the time we’d have to get back to simple promises and create our own “monad” stacks on top of it.

Some thoughts on “over”-using result I found in the wild (just opinions of course, also f# , useful nonetheless)

yawaramin · January 25, 2021, 9:06pm

I’d be interested to continue this in a new thread if you are willing to discuss.

alexeygolev · January 25, 2021, 9:19pm

I would love to. Might be a useful discussion to have

hoichi · January 26, 2021, 8:21am

Scott Wlaschin’s recommendation to only use Result for domain errors makes a lot of sense to me. But yeah, I’d read that discussion

Kingdutch · January 26, 2021, 9:40am

Thanks for the explanation ryyppy. Evaluating this as replacement for Js.Promise rather than reason-promise makes a difference I was making the latter comparison.

I’m still in the map/flatMap camp but will refrain re-iterating what people have said above.

bloodyowl · January 26, 2021, 12:41pm

I’ve worked on an alternative take a few months ago to handle some pain points I’ve had with Js.Promise.

Not being able to type errors correctly (as it only has a single type parameter): that makes me use Futures containing result in all of my codebases. I’d personally favor that over retro-compatibility.
missing map/flatMap, but those two are addressed in the proposal

Here’s are the bindings: https://github.com/bloodyowl/rescript-js/blob/main/src/Js__Promise.res