Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Implied Promise return type in async functions #7284

Closed
nevir opened this issue Feb 28, 2016 · 27 comments
Closed

Suggestion: Implied Promise return type in async functions #7284

nevir opened this issue Feb 28, 2016 · 27 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@nevir
Copy link

nevir commented Feb 28, 2016

Apologies if this has already been brought up, but I wasn't able to find any issues that cover this topic (#5911 seems closest).

The Current State

Currently, if you want to enforce typing of an async function's resolved value, you must declare a return type of :Promise<ResolveType>:

async function makeStuff():Promise<Stuff> {}

This is a bit counterintuitive when it comes to async functions, as they generally mask the fact that there's a Promise there to begin with. If you are utilizing promises purely via async/await, you're never going to see the token Promise in your code.

Proposal

When providing the return type of an async function, implicitly wrap it in a Promise<T> (unless it is an explicit Promise<T> type expression?). This seems reasonable now that #6631 is in.

async function makeStuff():Stuff {}

Seems like the least surprising way of declaring an async function

@yortus
Copy link
Contributor

yortus commented Feb 29, 2016

Would you suggest doing the same for generator functions for consistency:

function* foo() { // inferred return type is `IterableIterator<number>`
    yield 1;      // change that to just `number` under this proposal?
    yield 2;
    yield 3;
}

Also, I think a case could be made for this change being equally confusing, even as you say when the Promise token appears nowhere in the source:

function foo(): string {/***/}
var s1: string = foo(); // OK

async function bar(): string {/***/}
var s2: string = bar(); // ERROR

@nevir
Copy link
Author

nevir commented Feb 29, 2016

Would you suggest doing the same for generator functions for consistency:

I think that would make sense; though I haven't wrapped my head around generators enough to be confident.


Also, I think a case could be made for this change being equally confusing, even as you say when the Promise token appears nowhere in the source:

async function bar(): string {/***/}
var s2: string = bar(); // ERROR

That's a good error to have! Lets you know that you should be awaiting bar(). Ideally the error message would hint about await, as well.

@tinganho
Copy link
Contributor

I also think it is annoying to have to specify the Promise type, when it can be inferred from the async keyword already.

@ljqx
Copy link

ljqx commented Feb 29, 2016

Then how could we specify that we want to use Bluebird's Promise, not the original one?

@nevir
Copy link
Author

nevir commented Feb 29, 2016

Then how could we specify that we want to use Bluebird's Promise, not the original one?

As of #6631, you can't do that any more (except by providing your own __awaiter implementation)

@yortus
Copy link
Contributor

yortus commented Feb 29, 2016

I also think it is annoying to have to specify the Promise type, when it can inferred from the async keyword already.

@tinganho the whole Promise<T> return type can be inferred already without having to specify a return type annotation at all. I take it you are referring to a situation where the compiler cannot work out from the function body what is the T type in Promise<T>, so that you have to annotate it yourself? How common is this?

@tinganho
Copy link
Contributor

@yortus sorry my bad, i was referring to the original proposal that Promise is not required to annotate a return type for an async function. There is an implied Promise when you use the async keyword.

@yortus
Copy link
Contributor

yortus commented Feb 29, 2016

I think a fuller example showing the pain point / motivating scenario would be useful. The OP gave this example:

async function makeStuff():Stuff {}

But it's unclear from this line why the : Stuff annotation needs to be added at all. What does the function body look like, that the compiler cannot infer the return type? Usually it can accurately infer the return type with no annotation at all.


And on the point of return type inference, @nevir what do you propose the language service should do with this? E.g. if in VS Code I hover over the makeStuff identifier, does it pop up function makeStuff(): Promise<Stuff> like at present, or something else?

@tinganho
Copy link
Contributor

But it's unclear from this line why the : Stuff annotation needs to be added at all. What does the function body look like, that the compiler cannot infer the return type? Usually it can accurately infer the return type with no annotation at all.

It is good practice to specify the intent first and then implement the body. If you intend the type to be of some type T, then the checker enforces the function body to return that type. I believe specifying intention before implementation yields overall less buggy result.

And in this case my intention of a function is to be async and have a return type of T. But I think it is redundant to have to specify that the return type is Promise<T>, when we have already provided the async keyword.

@yortus
Copy link
Contributor

yortus commented Feb 29, 2016

It is good practice to specify the intent first and then implement the body

Interesting.... I agree with this statement but draw a rather different conclusion. I like to express my intent through clear interfaces that can be publicly shared with callers, and then provide an implementation that satisfies these interfaces without callers needing to know the implementation details.

In this light, using an async function is often an implementation detail - it's one of several possible ways to implement a function that returns a promise.

For example, I may provide some public API function that returns a 'thenable', without wanting to couple callers to the specific promise implementation, which might be bluebird for some builds and ES6 Promise for others. So I'd like my public API to look like this:

export class Foo {
    makeStuff(): Thenable<Stuff>;
}

Now I can implement this class in a variety of ways that satisfy the contract, keeping the implementation details decoupled from the public interface.

One such way would be like this:

export class Foo {
    async makeStuff(): Thenable<Stuff> {
        // make some stuff...
        // returns an ES6 Promise of stuff, which is-a Thenable<Stuff>
    }
}

Another way would be like this:

export class Foo {
    makeStuff(): Thenable<Stuff> {
        return new bluebird.Promise(...)
    }
}

Either way, the public interface remains the same.

The declaration async makeStuff(): Thenable<Stuff> would not be possible under this proposal. In fact, it's already not possible due to the already divergent treatment of async function return type annotations. I'd prefer that treatment of async function return type annotations was made more consistent with how they currently work on ordinary and generator functions, e.g. so the async function implementation in the above example works just as well as the bluebird implementation.

@tinganho
Copy link
Contributor

@nevir I think your proposal misses how interface declarations should look like but I guess they should use the async keyword as well?

interface A {
    async method(): string;
}

@yortus
Copy link
Contributor

yortus commented Feb 29, 2016

How is this:

interface A {
    async method(): string;
}

different from this:

interface A {
    method(): Promise<string>;
}

Does the first one mean that the interface only matches an async function, and not an ordinary function that returns a promise? Putting async in an interface declaration adds no expressivity to the type system, but possibly introduces an artificial distinction between two kinds of promise-returning functions that are interchangeable (and for most purposes indistinguishable) at runtime.

@tinganho
Copy link
Contributor

@yortus my thought was that there is no difference between them.

@DanielRosenwasser
Copy link
Member

interface A {
    async method(): string;
}

Actually, that's not a valid signature - async isn't allowed as part of a type (since all you need to know is that there's promise returned).

@tinganho
Copy link
Contributor

Actually, that's not a valid signature - async isn't allowed as part of a type

I think this proposal is to make it valid. Otherwise it is a little bit inconsistent that you can write async with implied Promise on function declarations, but not in interface declarations. Or do you mean that it is not technically possible?

@nevir
Copy link
Author

nevir commented Feb 29, 2016

But it's unclear from this line why the : Stuff annotation needs to be added at all. What does the function body look like, that the compiler cannot infer the return type?

The compiler can infer the type. However, there are times when I am in favor of explicit typings:

  • I prefer to have explicit types, particularly on public methods, so that the contract is clear.
  • It helps catch issues directly (e.g. when implementing the method), rather than indirectly (e.g. when consuming it).
  • It's pretty common to have explicit typing enforced as a code style.

And on the point of return type inference, @nevir what do you propose the language service should do with this? E.g. if in VS Code I hover over the makeStuff identifier, does it pop up function makeStuff(): Promise like at present, or something else?

Ideally, I would think that you'd want to display async function makeStuff():Stuff in the intellisense dropdown. This would be for all functions that return a Promise<T>, which implies a mode for when async/await is available. Don't want to balloon this suggestion out of proportion, though.


I think this proposal is to make it valid. Otherwise it is a little bit inconsistent that you can write async with implied Promise on function declarations, but not in interface declarations.

Ah, yeah, I didn't think about interfaces, but yes, that would be ideal.

@nevir
Copy link
Author

nevir commented Feb 29, 2016

To be clear: My main proposal is inferring the return type when implementing an async function - it was surprising to me that the compiler didn't (at first glance).

Supporting it in other situations (interfaces, the language service, etc) is ideal - but I would assume a large undertaking?


The motivation behind this request is:

  • When working on a project that's opted into async/await, you tend to stop thinking in terms of promises.
  • As async/await usage becomes more common, it'll become more important to keep compiler & editor behavior in line with people's expectations.
    • I am making an assumption that Promises will be treated as legacy/language implementation details over time.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Feb 29, 2016
@Arnavion
Copy link
Contributor

Since Promises can't nest (i.e. you can't have a Promise<Promise<T>> like you can have a Task<Task<T>> in C#), it does reduce the importance of having the Promise<> in the return type.

@yortus
Copy link
Contributor

yortus commented Mar 1, 2016

@nevir do you have a motivating example with code?

@nevir
Copy link
Author

nevir commented Mar 1, 2016

I don't have any I can share, unfortunately. But here's an example from around the community:

In both cases, declaring the type instead of letting it be inferred provides extra security. Both make calls to other functions w/ similar contracts, and you would want to know if they modify their signature (say during a refactor, etc)

@jods4
Copy link

jods4 commented Mar 5, 2016

⚠️ This is a dangerous proposal as it would interact badly with possible type system enhancements.

If this proposal was accepted, it would be impossible to declare an async method as returning Promise<number> | number. This is not accepted by the compiler today, but desirable (see my comment here: #6686 (comment)).

Fundamentally, this is lying about the return type of the function, which can clash with more complex type declarations. I don't think that saving nine characters Promise<> is a good enough motivation.
If you're keen on saving characters you often can let TS infer the actual return type of your method and type nothing at all.

If you are utilizing promises purely via async / await, you're never going to see the token Promise in your code.

That is not a good argument because it is not true.
You might still want to use Promise.all() or Promise.race().
You might still construct some promises using the ctor new Promise((resolve, reject) => {}). There are things that you can't do without that, in particular wrapping promises around async code that is based on callbacks. This can be your own code, e.g. if you wrap some user interactions inside Promises.
You might still need to type abstract functions or interface members, which require Promise for asynchronous API. This use case would be slightly confusing for less experienced devs, as an interface declaring Promise<T> would be implemented by an async methods declared as T.
IntelliSense would still show you Promise<T> as the return type of many functions (yours and 3rd party libraries). Again that could be confusing to see a different type from the declaration.

@nevir
Copy link
Author

nevir commented Mar 5, 2016

it would be impossible to declare an async method as returning Promise | number.

It is my understanding that an async function must return a Promise; it would be a compiler error not to.

At least, that's how I'm interpreting section 1.1.8 of the async/await spec. All conditions of EvaluateBody return a Promise. This is also the current behavior of both TypeScript and Babel.


I.e.

async function foo() { return 123; }
const result = foo(); // This is a Promise that resolves to 123.

@jods4
Copy link

jods4 commented Mar 5, 2016

This is not accepted by the compiler today, but desirable

@nevir Today the compiler does not allow this type annotation, but did you read the comment I linked next to that remark?

That other issue #6686 is about allowing such annotations and my comment provides a motivation for that.

@nevir
Copy link
Author

nevir commented Mar 6, 2016

@nevir Today the compiler does not allow this type annotation, but did you read the comment I linked next to that remark?

That other issue #6686 is about allowing such annotations and my comment provides a motivation for that.

I understand that, but it's not something that TypeScript can control. If you want to change that behavior, you really should be bringing it up with TC39, as it will affect all JavaScript based platforms, not just TypeScript.

At some point, the various vendors will implement this natively (and probably will start doing so relatively soon, since it's a stage 3 proposal). IMO It would be disastrous for TypeScript's async/await to behave differently from the native implementation.

The async/await spec's repo would be a better place to bring it up.

@jods4
Copy link

jods4 commented Mar 6, 2016

@nevir I think you did not get it right.
It is not about changing what an async method actually returns. It will always be a promise.
It is about the return type annotation, which is a TS thing.

It would be good to be able to say that an async method returns a union type that includes a promise. For example Promise<T> | T.
That does not change the fact that the async function always returns Promise<T>, but it is a valid return type declaration.
It is useful because of interfaces and inheritance. For instance, you may want to allow a derived class to have a synchronous implementation. There is a real world example in the linked issue.

@yortus
Copy link
Contributor

yortus commented Mar 6, 2016

@nevir just to be clear, @jods4 is fully aware that async functions always return a promise and is not suggesting otherwise. There is a clear distinction between the actual return type and the return type annotation, the latter being completely in TypeScript's control.

In TypeScript, return type annotations just have to be assignment-compatible. E.g., function self(): {length: number} { return self; } is valid. This clearly always returns a function. The return type anntotation doesn't change or contradict that, but only exposes a small assignment-compatible part of the return type.

His example shows a practical benefit of this distinction, where a base class method is always async, but allows derived classes to override this method with a sync implementation, and it all passes type-checks.


Here is a practical example. Suppose we maintain a library that uses promises in it's public API.

But we don't want our library users to depend on the specific Promises/A+ implementation we use to implement the library, so that we can change the implementation later, say from bluebird promises to native async/await, or so that we can support polyfilling in browsers, etc.

If our API specifically exposes say bluebird promises or es6 promises, then our users may start
writing things like p.finally(...) or if (p instanceof Promise) {...}, which only work with a specific promise type. Then if we change the promise implementation, we will break client code.

So instead, we write a generic interface Thenable covering only the interoperable part of Promises/A+, and make the public API use that interface instead, like so:

import * as BluebirdPromise from 'bluebird';

export interface Thenable<R> {
    then<U>(onFulfilled?: (value: R) => U | Thenable<U>, onRejected?: (error: any) => U | Thenable<U>): Thenable<U>;
    then<U>(onFulfilled?: (value: R) => U | Thenable<U>, onRejected?: (error: any) => void): Thenable<U>;
    catch<U>(onRejected?: (error: any) => U | Thenable<U>): Thenable<U>;
}

export function getFoo(): Thenable<Foo> {
    // returns a BluebirdPromise...
    return new BluebirdPromise((resolve, reject) => {...});
}
export function getBar(): Thenable<Bar> {
    // returns a BluebirdPromise...
    return getFoo().then(...).then(...);
}

Problem solved! Now our library users just see Thenables, and don't depend on the specific promise implementation, which we are free to change in future versions or in browser builds etc.

Later, let's say we want to update getBar's implementation to use async/await, still keeping the public API strictly about Thenables.

The obvious approach would be:

export async getBar(): Thenable<Bar> {
    let foo = await getFoo();
    ...
    return bar;
}

This implementation clearly always returns an es6 promise, but our library users see the same Thenable-based interface as before, which is exactly what we want.

And that's where we get to current TypeScript and this proposal:

  • current TypeScript allows assignment-compatible return type annotations everywhere except async functions. That already means the above update of getBar doesn't compile. Some of us are asking the team to reconsider this and make async functions consistent with the rest of the type system in this regard.
  • the current proposal, while saving some keystrokes in a few places, would also make the above example impossible, and that's the point @jods4 is making.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label May 9, 2016
@RyanCavanaugh RyanCavanaugh removed In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels May 9, 2016
@RyanCavanaugh
Copy link
Member

Discussed and basically came to the same conclusions as @jods4 and @yortus. Apparently the people working on async thought of doing this originally as well and decided it'd be a bad idea. There's also the fundamental tenet that a return type annotation shouldn't depend on modifiers on the other side of the screen to be interpreted correctly.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

8 participants