Suggestion for improving generator and async function type checking

This is a suggestion to improve both the consistency and the type-safety of return type checking for
generator functions (GFs) and async functions (AFs).

NB: this issue refers to tsc behaviour for target >=ES6 with typescript@next as of 2016-01-28 (ie since commit a6af98e)

Current Behaviour

Consider the following examples of current behaviour for checking return type annotations:

// Current behaviour - generator functions
class MyIterator implements Iterator<any> {next}
function* gf1(): any {}             // OK
function* gf2(): MyIterator {}      // OK (but really is an error)

// Current behaviour - async functions
class MyPromise extends Promise<any> {}
async function af1(): any {}        // ERROR (but is really not an error)
async function af2(): MyPromise {}  // ERROR

Problems with Current Behaviour

Firstly, type checking is not consistent across the two kinds of functions. In the examples the GF checking is too loose and the AF checking is too strict. The inconsistency is due to the different approach to checking the return type. The two approaches may be summarised like this:

Generator functions: accept any return type annotation that is assignable from IterableIterator<T>.
Async functions: reject all return type annotations other than references to the global Promise<T>. This is a recent change, the rationale
for it can be followed from here.

Secondly, the type checker only gets 2 out of 4 of the above checks right (gf1 and af2). Explanation:

GOOD: gf1's return type annotation is not super helpful but is 100% consistent with the type system. No sense erroring here, so the implementation is good.
BAD: gf2's return type annotation passes type checking because it passes the assignability check.
However gf2 definitely does not return an instance of MyIterator. All generator functions
return a generator object, so at runtime gf2() instanceof MyIterator is false. A compile error would have been helpful.
BAD: af1's return type annotation is just like gf1: not super helpful but 100% consistent with the type system. The compiler errors here even though nothing is wrong (reason for the error is here).
GOOD: af2's return type annotation fails type checking because it's not Promise<T>. The return type definitely won't be an instance of any class other than Promise, so the implementation is good.

Suggested Improvement

Since GFs and AFs always return instances of known instrinsic types, we can rule out any type annotation that asserts they will return an instance of some other class.

Both generator and async functions could therefore be checked with the same two steps:

Is Assignable: Ensure the return type of the GF/AF is assignable to the annotated return type. This is the basic check for all kinds of function return types. If not assignable, type checking fails. Otherwise, continue to step 2.
Not a Class Type: Ensure the return type annotation is not a class type (except Promise<T> which is allowed for AFs). For example if the return type annotation is Foo, ensure it does not refer to class Foo {...} or another class-like value.

These rules have the following effects:

GF and AF type checking are mutually consistent.
This fixes gf2 by ruling out class types like MyIterator in addition to checking assignability. GF type checking is made safer in general by catching a class of errors that currently slip through.
This fixes af1, because it's no longer necessary to rule out all annotations other than Promise<T>, but just those that are assignment-compatible class types like MyPromise. This approach will catch the breaking change from 1.7 as a compile error (as desired for reason here), but allow harmless (and correct) things like any and PromiseLike<T>.

Working Implementation

This is a small code change. I implemented this in a branch as best I could (but I may have made errors since I'm still getting my head around the codebase). The diff can be seen here.

With this version of tsc the above code works as follows:

// Suggested behaviour - generator functions
class MyIterator implements Iterator<any> {next}
function* gf1(): any {}             // OK
function* gf2(): MyIterator {}      // ERROR: A generator cannot have a return type annotation of a class type.

// Suggested behaviour - async functions
class MyPromise extends Promise<any> {}
async function af1(): any {}        // OK
async function af2(): MyPromise {}  // ERROR: An async function cannot have a return type annotation of a class type other than Promise<T>.

TL;DR: Generator functions and async functions always return instances of intrinsic types, so they can never return instances of other class types. This observation could be used to improve consistency and correctness of return type checking for both generator functions (currently over-loose) and async functions (currently over-strict).

I asked this question before, and i do not think any thing has changed since #6068

How is the compiler expected to emit this:

async function af1(): any {}

if as Promise, then either type it as Promise or write it as function af1(): MyPromise<someType>

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) things have changed since #6068. For ES6+, the emit always uses the global Promise, and the checker reserves the identifier Promise. The return type annotation is no longer used for emit, so now is conceptually no different to that of a generator function.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) I just checked the emit for ES6, so now regardless of the return type annotation the emit is:

function asyncFunction() {
    return __awaiter(this, void 0, void 0, function* () {/* async function body */});
}

and the __awaiter helper now starts with:

    return new (P || (P = Promise))(function (resolve, reject) {
        ...

So basically for ES6, P is always passed as undefined and then set to Promise, regardless of how the async function was annotated.

so what is the scenario we are trying to do accomplish here? you can not have a function marked as async that returns MyPromise. what value is there to annotate it as any and keep async.

The value is in consistency in the type system. Annotations would ideally work consistently right across the language. I can do all of the following:

function f(): any[] { return [1,2,3]; }                        // OK
function p(): PromiseLike<any> { return Promise.resolve(42); } // OK
function* g(): Iterator<any> { yield 1; yield 2; yield 3; }    // OK

Note that none of these return type annotations is an exact match to what is actually returned, they are simply assignment-compatible. So now I have my nice consistent mental model, and I try out the following expecting it to work in the same way as the examples above:

async function a(): PromiseLike<any> { return 42; } // ERROR

...but it in fact raises a compiler error. The inconsistency is confusing (#5911 raised this very example). And as of #6631, there's no longer a need to keep this inconsistency.

Troy Gerwien (@yortus)

Troy Gerwien (@yortus) suggested I copy my comment from #6631:

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; } ) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

class BaseViewModel {
  async isValid() : boolean | Promise<boolean> { 
    /* some async implementation, maybe server call */  
    return await fetch("/valid");
  }
}

class DerivedViewModel extends BaseViewModel {
  isValid() : boolean | Promise<boolean> {
    return true;  // Non-async implementation
  }
}

but this wouldn't work :(

jods (@jods4)

jods (@jods4) async is not applicable in this context. marking a function as async means that the compiler will wrap it in a new Promise call all the time. if the function is not guaranteed to return a Promise it should not be marked as async, it should rather return T | Promise<T>.

This goes back to the issue that an async function will be rewritten by the compiler, there is no way to change that unless it is not marked as async. if it is marked as async, the only possible type that can come out of it is the Promise object, any thing else we allow here will be a lie.

The only proposal i see here is making a generator function returning something other than the generator interface an error to match the Promise implementation.

Mohamed Hegazy (@mhegazy)

Mohamed Hegazy (@mhegazy) jods (@jods4)'s example is valid both in terms of its type assertions and its runtime behaviour.

if the function is not guaranteed to return a Promise it should not be marked as async

isValid in the base class is guaranteed to return a promise so is correctly marked as async. It is annotated with boolean | Promise<boolean> so that derived classes can override it with either a sync or an async implementation, without TypeScript complaining about incompatibility.

The following workaround adds pointless boilerplate that is only neded to satisfy TypeScript (pointless since jods (@jods4)'s version works fine at runtime as is currently is written, just removing the annotations) (actually it's much worse than jods (@jods4)'s original because it creates a new closure instance on every call):

class BaseViewModel {
  isValid() : boolean | Promise<boolean> { 
      return (async () => {
          /* some async implementation, maybe server call */
      })();
  }
}

class DerivedViewModel extends BaseViewModel {
  isValid() : boolean | Promise<boolean> {
    return true;  // Non-async implementation
  }
}

Another workaround just to satisfy the compiler is this (which is also worse than jods (@jods4)'s original because it instantiates a new implementation of isValid for every class instance, rather than adding the method once to the prototype):

class BaseViewModel {
    isValid: () => boolean | Promise<boolean> = async () => {
          /* some async implementation, maybe server call */
    };
}

jods (@jods4)

The only proposal i see here is making a generator function returning something other than the generator interface an error to match the Promise implementation.

Then please look differently :)

Using annotations the way jods (@jods4) does is a great example of the Liskov substitution principle. This surely is part of any good programmer's toolkit.

I'd much rather allow LSP to work on async functions, than deliberately stopping it from working on generator functions (where it currently works just fine).

the only possible type that can come out of it is the Promise object, any thing else we allow here will be a lie

It is categorically not a lie to describe a Promise instance as a union type that includes Promise. It's useful for applying LSP as mentioned above. More generally, for some value t whose real type is T, it's correct to say var t: T|U whatever U is.

function foo(): string|number { return 42; }

That's guaranteed to return a number. It's certainly not a lie to annotate the return type as string|number.

These are also not lies and may actually be useful annotations for keeping interfaces separate from implementations:

var p: PromiseLike<number> = Promise.resolve(42);
var q: Thenable = Promise.reject(new Error('fail!'));

The compiler allows these useful 'abstracting' annotations basically everywhere except on async function return types.

I hate to raise this issue from the dead, but I just ran into this issue while trying to write types for the child-process-promise library. The functions return a subclass of Promise that have an additional childProcess member so that you can, for instance, listen to data on the childProcess.stdout stream. However, declaring the type:

class ChildProcessPromise extends Promise<ChildProcess> {
  public childProcess: ChildProcess;
}

I'm unable to return a ChildProcessPromsie from an async function:

import { ChildProcessPromise, spawn } from 'child-process-promise';

async function ls(): ChildProcessPromise {
  const dir = await getDir();
  const promise = spawn('ls', [dir]);
  promise.childProcess.stdout.on('data', (data) => process.stdout.write('[spawn] stdout: ', data.toString()));
  return promise;
}

This produces an error:

The return type of an async function or method must be the global Promise<T> type.

It's impossible for the return type to be anything other than a Promise, but I know that that promise will always have that additional property, and I want my types to reflect that. Is there an alternative here, or will I just have to cast the return value as any when I want to use it?

Nathan Sarang-Walters (@nwalters512)

Nathan Sarang-Walters (@nwalters512) for ES6+ targets, async functions will never return anything other than the built-in Promise type. You can check it by debugging your own sample code above. If you set a breakpoint on the return promise; line and inspect the value, it has the childProcess property as you describe. However if you add code to call this async function and then inspect the result type, it is a standard Promise and does not have the childProcess property.

This is the expected behaviour of async functions. They work fine with promise-like values in their body, but their return value is always a built-in Promise.

Ah! I had this idea that returning a promise would bypass the usual wrapping of the return value in a promise, not sure where that came from. Thanks so much. Making the return type Promise<ChildProcessPromise> was able to fix the type error.

The return value will not be a Promise<ChildProcessPromise> despite passing the type checker. A promise never resolves to another promise, by design. The Promise instance returned by a call to ls will adopt the state of the special promise instance that was created inside the function. So it will resolve or reject when that inner promise resolves or rejects. But anything else special about that inner promise is lost.

Navigation Menu

Repository navigation

Suggestion for improving generator and async function type checking #6686

Current Behaviour

Problems with Current Behaviour

Suggested Improvement

Working Implementation

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Suggestion for improving generator and async function type checking #6686

Description

Current Behaviour

Problems with Current Behaviour

Suggested Improvement

Working Implementation

Activity

yortus commented on Jan 28, 2016

mhegazy commented on Jan 28, 2016

yortus commented on Jan 28, 2016

yortus commented on Jan 28, 2016

mhegazy commented on Jan 28, 2016

yortus commented on Jan 28, 2016

jods4 commented on Jan 29, 2016

mhegazy commented on Feb 20, 2016

yortus commented on Feb 20, 2016

yortus commented on Feb 20, 2016

nwalters512 commented on Jul 18, 2018

yortus commented on Jul 19, 2018

nwalters512 commented on Jul 19, 2018

yortus commented on Jul 19, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions