update bimap implementation to reject result of leftFn, and improve type signature

[interacsean]

Updated the return type to be Promise, as it doesn't have to be the same type as the original promise.

I updated the test as it didn't look quite right – I think this method (.then, .catch rather than try/catch block) feels a bit more robust. I will update other tests to use this as well (and have similar updates for type signatures) if you're happy with it.

Some additional updates happened to index files as a result of running build

[dandro]

This is awesome man! thanks for that, specially the fact that bimap now maintains the state of the promise (as in rejected will remain rejected). As for changing from try/catch to then/catch it technically doesn't make any difference, so I'm fine if you want to change it but it's probably not worth much effort TBH.

[dandro]

Hey man, the type here is wrong... it should be (e: E): Promise<any> => ... since the leftFn is (E) => any. Which I should fix to be honest because we should not be using weak types.

[interacsean]

I wasn't sure what to do here. I still half-feel this is correct.

If we were to write out the rightFn it would look like this

function bimap<T, R, E>(
  leftFn: (E) => any,
  rightFn: (T) => R,
  p: Promise<T>
): Promise<R> {
  return p.then(
    **(v: T): Promise<R> => Promise.resolve(rightFn(v)),**
    (e: E): Promise<R> => Promise.reject(leftFn(e)),
  );
}

Since rightFn returns Promise<R>, shouldn't leftFn return Promise<R> too – so that the function on both sides returns the same type?

(If leftFn returned Promise<any>, would bimap then return Promise<any>?)

In reality, once the promise is failed, it won't contain an R anymore, and of course we can't annotate the type of the rejected state value

[dandro]

Oh I see the confusion. So promises only annotate the Success value, not the error. So as long as the promise on the left is rejected we cannot type it.
facebook/flow#1232
facebook/flow#5042

Alternatively we could say, incorrectly, Promise<R | SomeErrorType> but that's just us being super clever :)

[interacsean]

Yeah that's my understanding. Which is a huge caveat of this approach, but I still think there's a lot of benefits to it which is why I'm PRing :).

Unfortunately that union trick won't work for long:

https://flow.org/try/#0PTAEAEDMBsHsHcBQiAuBPADgU1AVQM5YBOoAvKAN6gB2AhgLZYBco+KRAltQOagC+AblSYcAUSJFYJclQDGsACbMaAV3oAjYvyGJIK6rJQdY1UNywoCxAEJoAkgoAUHBQEoWABUn0OhADxWJAA+oOKSRAB8lKBEFipEpl6wPoQAdLH4sNAAbliOVHSMLADkAMpYtNTF-K4C-Mjy1GygKoREnt6+WAFtoCFhUlHk5pZttg6OAIy1yK3EqSgAFljUjo5LWIEsga5kURSIoKCNmdBYqXDc68uBqYVYM3yuqbK0KLKLa8TtoRJSu6R9odjiZTudLo5ivIVNAFDRYCgzBYWm1qgBqUDfF6KB5CJ5AA

As a comment to the issues around being unable to type the rejection because anything might be thrown, as long as every Promise has a catch((err: any) => ConcreteErrVal) defined, shouldn't it then be possible to type the resulting Promises' rejected side with that Promise<SuccessType, ConcreteErrVal>. (And prior to that catch, it would have to be Promise<SuccessVal, any>?)

Getting off topic... how should we resolve the bimap?

[dandro]

If you're going to change from try/catch to .then/catch the can you remove the async keywords?

[interacsean]

Will do

[dandro]

Sorry dude, I'll merge it now as it makes sense enough and I honestly don't know the answer, but I think it makes sense (until we can type Errors in Promises).