Converting from Js.Promise

[aantron]

Converting from Js.Promise

[baransu]

My rule of thumb is:

1. when you know your promise won't fail (and the promise is just to express async) just use Promise.t('a) when binding.

[@bs.module "my-awesome-lib"]
external getStringAsync: unit => Promise.t(string) = "getStringAsync"

2. when I have no idea about the internals of the function I'm binding to I would use Promise.Js.t('a, Js.Promise.error) and convert to Result.t as soon as as possible by wrapping Js.Promise.error into poly variant for better error composability. I believe this can be achieved with catch as well and I'm not 100% if its correct.

[@bs.module "my-awesome-lib"]
external _getStringAsync: unit => Promise.Js.t(string, Js.Promise.t) = "getStringAsync"
let getStringAsync = () => {
  _getStringAsync()
  ->Promise.Js.toResult()
  ->Promise.mapError(error => `PromiseException(error))
}

---

I also think in your case there is no need for Token module to be async as the JS API is sync.

I would implement authLink like this:

// Note: Refactored in GH comment, may not compile :P
let authLink =
  ApolloClient.Link.ContextLink.makeAsync((~operation as _, ~prevContext as ctx) => {
    let token = Tokens.getUserToken();
    switch(token) {
      | None => Js.Obj.empty()
      | Some(token) => 
          let headers = Js.Obj.assign({
            "authorization": "Bearer " ++ token
          }, ctx.headers)
          {"headers": headers}
    }
  });

This code may not express your logic in 100% but it's a simplified version. In my experience wrapping sync API into promises makes things worse 😉

[aantron]

@idkjs I believe @baransu has answered your question. In summary, your binding (ideally) wouldn't be written in terms of Js.Promise, but in terms of reason-promise to begin with. Js.Promise from BuckleScript and reason-promise are just two incompatible types (deliberately, as the first one is not type-safe), so you can't use one where the other is expected, which is what your code appears to have been trying to do.

There are, however, conversions, in the common case that you do have two sets of code using the different types, and the README provides instructions on writing bindings for those cases where you can edit the code to replace Js.Promise by reason-promise already at the binding level, thus avoiding introducing any Js.Promises at all. @baransu has pretty much perfectly explained how to use the binding support in his comment (thanks!).

[idkjs]

Thank you, gentelman. Very instructive. Will revert with results.