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

FR: Warn when using await on PromiseLike #39387

Closed
kwasimensah opened this issue Jul 2, 2020 · 6 comments
Closed

FR: Warn when using await on PromiseLike #39387

kwasimensah opened this issue Jul 2, 2020 · 6 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@kwasimensah
Copy link

TypeScript Version: 3.7.x-dev.201xxxxx

Search Terms:

PromiseLike await

Code

const promiseA = new Promise(resolve => {
  resolve(5);
})

async function main() {
  const a = await 5; // typescript will give warning 80007
  const b = await promiseA; // expected
  const c = await ({} as PromiseLike<number>); // The cast to PromiseLike silences 80007
}

Expected behavior:

There should be warning on the line with const c. I found this while using chai-as-promised. "await expect(promise).to.eventually.be(arg)" was throwing UnhandledPromiseRejectionWarning. Chai.PromisedAssertion is a PromiseLike and not a Promise (https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/chai-as-promised/index.d.ts#L105) and it's not safe to use with await. But the type system hid that from me.

Actual behavior:

No warning.

Playground Link:

https://www.typescriptlang.org/play/index.html?ssl=10&ssc=2&pln=1&pc=1#code/MYewdgzgLgBADgJxAWwJYQKYEEYF4ZgYDuMACkmpgBQIYQgA2AbhngHwwDeAUDDLfWYYqAVgCUAbm4BfMd24BDCAE8wwGADMArmqipwMZAtRgqYrrxihIsBXhgKix2CKl9r0GACN7j5-Ap0bDcrcE91fD9UWCpOaQcIMkDMABlUAGsMAB4wLWQvDAQ2SXlpIA

Related Issues:

#35419

@DanielRosenwasser
Copy link
Member

Thoughts @rbuckton?

@rbuckton
Copy link
Member

rbuckton commented Jul 3, 2020

I'm not sure I understand why you would issue a warning on await on PromiseLike. That is expressly a feature of ECMAScript's await keyword and allows the adoption of foreign (i.e., non-native) promises. If you try to await a "thenable" (i.e., something with a callable then that does not conform to the definition of PromiseLike), you already receive an error:

https://www.typescriptlang.org/play/index.html#code/MYewdgzgLgBADgJxAWwJYQKYEEYF4ZgYDuMACkmpgBQIYQgA2AbhngHwwDeAUDDLfWYYqAVgCUAbm4BfMd26owUDAgBmAQ2CsAKgAsMYdQCMGrHnyj6wVSTPnqIATzDAYqgK4uoqcDGTrFGy5eGFBIWHU8GHUiANgRKT4w6BgjKJi4+Ap0bETQ8BTXfAzUWCpOaWiIMmzMABlUAGsMAB4wd2QjFTZbJILYABN02NKYcsqHGD0DY1NbaSA

image

I'm not familiar with chai's PromisedAssertion or any of its caveats, but if it's not a valid promise it probably shouldn't be typed as one.

@kwasimensah
Copy link
Author

kwasimensah commented Jul 3, 2020

  1. Chai should use proper promises (in fact the extension that’s causing this seems dead)


  2. There’s nothing in the PromiseLike type that seems to actually guarantee that it works with await. In fact looking at the Javascript spec for await (https://tc39.es/ecma262/#sec-async-function-definitions) and its definition of what a Promise is (https://tc39.es/ecma262/#sec-ispromise) I don’t think custom Promise types are actually compatible with await/async and warning 80007 should actually fire for anything that’s not a native promise. Bluebird documents that’s it’s not compatible with async/await at Official recommendation for using Bluebird with async functions? petkaantonov/bluebird#1434 (comment)

@rbuckton
Copy link
Member

rbuckton commented Jul 3, 2020

The PromiseLike type was specifically crafted to work with async/await per the spec. What a non-native promise loses over a native promise is that there is an extra turn of the microtask queue and that async functions only return native promises. The fact that native promises take one less turn is a recent addition as well.

All that await cares about is that the object has a callable then that can accept two callback arguments, and that is specifically what PromiseLike in TypeScript was designed to represent.

@DanielRosenwasser DanielRosenwasser added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 3, 2020
@kwasimensah
Copy link
Author

Sorry for the noise. Turns out there's some brittle stuff with proxying that muddles the typing here. chaijs/chai-as-promised#266 (comment)

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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

4 participants