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

Should awaiting a non-Promise value be an error? #8310

Closed
Arnavion opened this Issue Apr 26, 2016 · 16 comments

Comments

Projects
None yet
7 participants
@Arnavion
Contributor

Arnavion commented Apr 26, 2016

TypeScript Version:

1.9.0-dev.20160426

Code

var x = await 5;

Currently this compiles just fine and behaves correctly at runtime. But is there a situation where the user would want to deliberately await a non-Promise? Or should it be a compiler error?


I thought of:

  1. Normalizing a sync-or-async value:

    declare function mayReturnSyncOrAsyncResult(): number | Promise<number>;
    
    var result = await mayReturnSyncOrAsyncResult(); // number

    Either await could still be allowed for unions that contain Promise, or the user could be required to use await Promise.resolve instead of just await.

  2. Extracting the value of a thenable:

    declare function foo(): bluebird.Promise<number>;
    
    var result = await foo(); // number

    Again await could still be allowed, or the user could be required to use await Promise.resolve

  3. Deliberately introducing a yield point to make the rest of the function be a separate async continuation. I'm not sure if such a use actually exists in the wild. If it does exist, it too can be handled by await Promise.resolve

The downside of await Promise.resolve is that it introduces one extra Promise, since await will wrap the awaited value in one anyway.

I'm not completely convinced one way or the other. Opening this issue to get TS team's and other people's thoughts.


Alternatively I suppose we can make a tslint rule for "stricter awaits" or something.

@basarat

This comment has been minimized.

Contributor

basarat commented Apr 26, 2016

But is there a situation where the user would want to deliberately await a non-Promise? Or should it be a compiler error?

Yes. When the API consumer wants a promise but the API provider doesn't need to be async. The provider code can then just await 🌹

@basarat

This comment has been minimized.

Contributor

basarat commented Apr 26, 2016

instead of

function foo(){
      return Promise.resolve(3);
}

you get to

async function foo(){
      return await 3;
}
@Arnavion

This comment has been minimized.

Contributor

Arnavion commented Apr 26, 2016

@basarat That should be

async function foo() {
    return 3;
}
@basarat

This comment has been minimized.

Contributor

basarat commented Apr 26, 2016

That should be return 3

Indeed. Sorry. I know I should have been sleeping at that time 🌹

@mhegazy

This comment has been minimized.

Contributor

mhegazy commented Apr 26, 2016

// CC @bterlson

I believe it is not an error to await a non-thenbale non-promise object. awaiting a literal as in the example should be equivalent to await Promise.resolve(3). @bterlson can point to the spec details.

one thing to note, var x = await 5; is not the same as var x = 5;; var x = await 5; will assign x 5 in the next tern, where as var x = 5; will evaluate immediately.

@mhegazy mhegazy added the By Design label Apr 26, 2016

@Arnavion

This comment has been minimized.

Contributor

Arnavion commented Apr 26, 2016

Yes, I said all that in the OP.

@basarat

This comment has been minimized.

Contributor

basarat commented Apr 26, 2016

But is there a situation where the user would want to deliberately await a non-Promise

This is the heart of the question by @Arnavion ^.

Me : I can't think of a good reason. The only use case (next tern) is not one the user should be depending upon. There might be some obscure debounce style stuff that one can cook up 🌹

@mhegazy

This comment has been minimized.

Contributor

mhegazy commented Apr 26, 2016

@bterlson, can you shed some light on the rational/motivating use cases for the current design?

@tp

This comment has been minimized.

tp commented Apr 27, 2016

Just some anecdotal info: I got unwanted behavior when writing my first TypeScript program, since in many libs the TS version uses *Async names for the Promise-returning version of the function.

So since the callback was optional, I "awaited" on void and then the program would continue too early...

So if there no real reason to keep this behavior, I can assure you that this has already created some bug (even though luckily only during development).

@bterlson

This comment has been minimized.

Member

bterlson commented Apr 28, 2016

The goal of the current semantics isn't to allow uselessly awaiting known synchronous values but instead to allow awaiting maybe-promises and to align with the semantics of various existing promise utilities including Promise.resolve (promises and thenables unwrapped, values passed through) and then handlers (returned promises and thenables unwrapped, values passed through).

@mhegazy

This comment has been minimized.

Contributor

mhegazy commented Apr 28, 2016

Thinking about this more, this could be enforced under a flag, by default if the compiler can attest that the type of await expression can not be a promise, then it should report an error, if the flag is set, the error reporting is skipped.

@RyanCavanaugh

This comment has been minimized.

Member

RyanCavanaugh commented Jun 9, 2016

Main points from discussion:

  • Not necessarily clear this an error (maybe you intend to effectively yield)
  • What if the awaited expression is a type parameter? Not clear how to determine if that's an error or not
  • We clearly want to allow await T | Promise<T> (for reasons @bterlson mentioned above) but then it's weird to allow await Promise<T> but disallow await T since we're effectively saying either might happen

We can't tell that you definitely meant to not await any arbitrary value, so disallowing it isn't something we can do for certain.

@Arnavion

This comment has been minimized.

Contributor

Arnavion commented Jun 9, 2016

but then it's weird to allow await Promise<T> but disallow await T since we're effectively saying either might happen

I dunno, atleast to me it doesn't seem weird.

@tinganho

This comment has been minimized.

Contributor

tinganho commented Jul 25, 2016

Would be great if there was a flag for raising an error.

@RyanCavanaugh

This comment has been minimized.

Member

RyanCavanaugh commented Jul 25, 2016

Rumor has it tslint is getting type-based rules in the near future thanks to some work from the Angular team, so that might be a good place to go.

@basarat

This comment has been minimized.

Contributor

basarat commented Jul 25, 2016

Rumor has it tslint is getting type-based rules

Indeed it is. The reason why I've dropped my usage of ntypescript (which could support ts extension) and moved to a simpler automation of bring your own typescript : https://github.com/basarat/byots (as the extension can now be done using tslint) and also working on first class integration into alm : alm-tools/alm#155 🌹

@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.