Flow: Async functions should not require `Promise<T>`

[phpnode]

This is related to #971.

At the moment flow will reject this:

async function something (): string {
  return "nope";
}

Because string is not a promise. The following is valid:

async function something (): Promise<string> {
  return "ok";
}

Async functions _always_ return promises as part of the spec, so the Promise annotation seems redundant here because we're repeating something that is already statically verifiable. I think the first example should be valid.

I realise that this is possibly a preference issue, and some people will say that Promise<T> is clearer, so perhaps both options should be allowed, but to me it seems much friendlier if the return type annotation always matches the type of the value the function returns. The fact that async functions run on promises under the hood is an implementation detail that most users shouldn't need to care _too much_ about.

[samwgoldman]

Yeah, I had the same reaction, but @gabelevi ended up convincing me that this is better. Thanks for bringing this up, but at this point I doubt we will change this.

One reason to keep the annotation explicit (Promise<T>) is for consistency with generator functions, which return Generator<Y,R,N>. For generators, there are 3 types involved in the return type, so it's unclear how we might go about "unwrapping" that in a non ambiguous way.

Maybe Gabe will chime in with his reasoning too. Regardless, thanks for taking the time to write up your opinion and please keep them coming!

[ide]

I'd like to revisit this because in many code bases a significant percent of the functions end up being async functions and writing Promise after each of them gets redundant and verbose. So for standard language-level function types (async, generator) what if Flow had sugar like this?

async function asyncFunction(): T {
  return t;
}

function* generatorFunction(): <Y, R, N> {
  let n = yield y;
  return r;
}

Flow could be completely backwards compatible (function * gen(): Generator<Y, R, N> is still valid), the above suggestions are simply sugar. It would also be straightforward to write a codemod for this new syntax.

[vjeux]

Fwiw, I've lobbied to get Hack to behave the way you describe but failed :(

[jeffmo]

FWIW: I also agree that it would be really nice to not have to write Promise for these. I think @gabelevi has to be convinced to move forward, though

[phpnode]

I'm not so sure I agree with myself any more, e.g. what should happen if the async function is really returning a Bluebird promise rather than the global?

It's also quite easy to explain to people - the return type annotation signals what you'd get if you just invoked the function normally. That's simpler than having a bunch of caveats about these 3 (with async generators) edge cases, particularly as there will likely be more edge cases in future.

[vkurchatkin]

what should happen if the async function is really returning a Bluebird promise rather than the global?

Well, it can't really happen with native async-await. You will always get native promise

[phpnode]

@vkurchatkin I'm not sure how promise assimilation works with subclassing and async functions, but I'm imagining something in general like:

class ChildPromise extends Promise {};

async function demo (): ChildPromise<number> {
  const value = await doSomething();
  return somethingWhichCreatesAChildPromiseInstance(value);
}

Some people also transpile async functions to bluebird + generators, but I don't know if it's a goal to accommodate that in Flow.

[vkurchatkin]

This shouldn't work according to spec this won't work

Some people also transpile async functions to bluebird + generators,

Well, if they rely on this, they are asking for trouble

[phpnode]

This shouldn't work according to spec this won't work

Because a Promise has already been created and returned to the caller before the ChildPromise is created?

In that case maybe a better example would be an async function which returns a promise rather than awaiting it:

async function a () {}
async function b () {
  await a();
  return new Promise(resolve => c(resolve));
}

function c (callback) {
  callback(123);
}

What should Flow accept as an annotation for b? Accepting just number would seem strange.

[vkurchatkin]

Whatever you return from async function is basically wrapped in NativePromise.resolve(), so it always going to be NativePromise

[jeffmo]

Just wanted to add in the relevant spec text that asserts that async functions can never return anything but a native promise object:

https://tc39.github.io/ecma262/#sec-async-functions-abstract-operations-async-function-start

When starting an async function, an implementation calls the .resolve() method for the passed-in "promiseCapability". And the "promiseCapability" that is passed in is always the intrinsic Promise constructor (referred to as %Promise% in the spec):

https://tc39.github.io/ecma262/#sec-async-function-definitions-EvaluateBody
https://tc39.github.io/ecma262/#sec-async-arrow-function-definitions-EvaluateBody

The "intrinsic" constructor is the built-in one, not one retrieved from a scope lookup or from any accesses on the global object

You can in theory mutate the intrinsic constructor's prototype, but you can't actually replace it (same as Array and other intrinsics)

[StreetStrider]

In fact, implicit R instead of Promise<R> for ret type is not just a sugar, but incorrect way to define async function. Truely, async function always returns a promise, so you just can't short-cut here with R, but you can return instances of R inside of async function. So, async function is heterohenous on R inside of it, and not outside (in ret val). I suppose maintainers are absolutely correct about making it explicit.

Until I can return instances of <R> inside of async function, I'm OK with that.

[ide]

The static return type already refers only to the type the caller sees. We don’t care whether the implementation returned R or Promise<R> — the static return type is always Promise<R>.

With this suggested update to Flow, “async function x(): R” would mean “a function that returns Promise<R> because that’s how JS async functions are defined.” There’s no loss of information AFAICT. How the function is implemented is orthogonal.

[StreetStrider]

@ide, hmm, if async keyword in game, maybe you're right. async word already implies Promise which allow to off-load ret type, make it less noisy, by removing Promise. 🤔

However, if that would legit, async keyword must be supported both by type = function type and interface { function interface } (and maybe somewhere else where I can't recall now).

[mhelvens]

I agree, there doesn't seem to be a good reason to require the full Promise<T> annotation.

And making this backwards compatible shouldn't even require special-case code to implement, since Promise<Promise<T>> is already collapsed into Promise<T>. So the simple rule would be: If the function is async, wrap Promise<...> around whatever its given return type is. (In theory, anyway. I admit I haven't read the flow source-code.)

In fact, for async functions that don't specify a return type, this is already being done. They behave to callers as having return type Promise<*>, not *. Consistency with _that_ behaviour seems, to me, more important than consistency with notation for generators.

Correction: Promise<Promise<T>> is not collapsed into Promise<T> (it should, though, shouldn't it?)