[Typescript] Improving flow typings #1378

[nulladdict]

This is an attempt to fix #1378

This is a breaking change to flow typings that requires TS version 3.6+
I am still not sold on the final solution, but I see two major alternatives:

• the one described in this comment and implemented in this PR
• typing flow as Generator<Promise<any>, R, unknown>, so that we get inferred return type, type-check on yielded promises and that we're required to type yield results, which is somewhat similar to what we had before

Both of this are not perfect and still breaking, because TS changed Generator interface.
It seems that there's a possibility to escape the breaking change by utilising typesVersions as mentioned here, however I am not quite sure I understand how to do that

It is also worth mentioning that with arriving of actionAsync this problem might not be that important, and we might just use the second alternative to fix TS errors and not rely on lots of TS magic

[xaviergonz]

flow typings should be fixed (when using ts3.6) with version 3.15.0 that was just released, give it a try :)

[nulladdict]

3.15.0 looks good, however I believe it introduces a type regression when returning a Promise:

const f = flow(function*() {
  yield promise
  return Promise.resolve(2)
})
// f is () => Promise<Promise<number>>

const x = await f() // x is correctly inferred as number
f().then(x => { /* ... */ }) // x is Promise<number> when in fact it's just a number

taking a look at flow return code:

wrap((r: any) => { resolve(r) }, "flow_return", ret.value)

resolve(r) when r is a promise would produce a Promise<T> and not a nested one, so the correct return type in our example is Promise<number>, await just hides the issue

I've fixed this and added a test for this specific case
Also I've change yielded type from any to Promise<any> since we don't support any other yielded values anyway

[xaviergonz]

Technically I think then will also resolve to a number even when taking Promise<Promise<number>>, so that shouldn't matter much, but I guess it is better to keep API typing compatibility.

Also forcing yield to take promises should help catching bugs, so LGTM 😁

[nulladdict]

You’re absolutely right, it would resolve to a number, but typescript would complain, so yeah, it’s just for compatibility

[mweststrate]

Released in 3.16. Apologies for the late follow up!