Skip to content

Type checking not behaving as expected for Promise<T> signatures #10524

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

Closed
jonnycornwell opened this issue Aug 24, 2016 · 8 comments
Closed

Type checking not behaving as expected for Promise<T> signatures #10524

jonnycornwell opened this issue Aug 24, 2016 · 8 comments
Assignees
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@jonnycornwell
Copy link

jonnycornwell commented Aug 24, 2016

TypeScript Version: 2.0.0@beta && 2.1.0-dev.20160824

Code

var fooPromiseLike: PromiseLike<string>;
var fooPromise: Promise<string>;
var bar: (f: PromiseLike<boolean>) => void;

// Errors with PromiseLike<string> is not assignable to PromiseLike<boolean>
bar(fooPromiseLike);

// Should also error?
bar(fooPromise);

Expected behavior:

Should give an error when passed an incorrect signature, works as expected in 1.8.10. Unless I am missing something?

I couldn't find any open issues related to this and it seems to still be broken in the nightly build

@jonnycornwell jonnycornwell changed the title Type checking not behaving as expected for () => Promise<T> signatures Type checking not behaving as expected for Promise<T> signatures Aug 24, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Aug 24, 2016

@rbuckton can you take a look

@rbuckton
Copy link
Member

The issue boils down to how TypeScript handles overloads and generic type erasure.

Then we check bar(fooPromise), we compare the overloaded signatures of "then" on both Promise<string> and PromiseLike<boolean>.

For Promise<string> we choose:

() => Promise<string>

For PromiseLike<boolean> we choose (and erase):

(onfulfilled?: (value: boolean) => any, onrejected?: (reason: any) => any) => PromiseLike<any>

When we check assignability of these signatures, they are compatible as in TypeScript you can always assign something with fewer arguments to something that expects to call it with more arguments.

We will have to discuss in the team if there is a viable solution to this issue.

@mhegazy mhegazy added the In Discussion Not yet reached consensus label Aug 26, 2016
@falsandtru
Copy link
Contributor

Type checking is passed by the following code.

    /**
     * Creates a new Promise with the same internal state of this Promise.
     * @returns A Promise.
     */
    then(): Promise<T>;

https://github.com/Microsoft/TypeScript/blob/master/lib/lib.es2015.promise.d.ts#L48

It seems a not expected behavior.

@yortus
Copy link
Contributor

yortus commented Aug 26, 2016

That nullary then overload causes problems. See also #9953 (comment). I said there I didn't understand why it happened, but @rbuckton's comment above about assignability clarifies that.

@rbuckton
Copy link
Member

rbuckton commented Sep 6, 2016

The problem with the then(): Promise<T> overload is that T isn't used in the signature in any place that can be used to reject this overload. Since T isn't used in any fields of Promise<T>, the return type cannot be used to reject the overload. Instead, we need to inject a bare T somewhere in every overload signature.

The fix in #10448 uses the following overload instead:

  then(
    onfulfilled?: ((value: T) => T | PromiseLike<T>) | undefined | null, 
    onrejected?: ((reason: any) => T | PromiseLike<T>) | undefined | null): Promise<T>;

(yes, the undefined is unnecessary since the argument is optional, however I added it for the sake of clarity)

Although verbose, this overload can be used as if it was then() but still uses a bare T in the parameter list. The remaining reasons for the more complex overload is to provide better typings to fix #4903.

@falsandtru
Copy link
Contributor

Although verbose,

I agree, I hope you will provide an intuitive way.

@rbuckton
Copy link
Member

rbuckton commented Sep 7, 2016

@falsandtru The declaration above allows you to consume a promise using then() just as you could before. Could you clarify what you mean by "provide an intuitive way"?

@falsandtru
Copy link
Contributor

Now TypeScript is unable to detect type mismatches of Promise.

declare var fooPromiseLike: PromiseLike<string>;
declare var fooPromise: Promise<string>;
declare var bar: (f: PromiseLike<boolean>) => void;

bar(fooPromiseLike); // error
bar(fooPromise); // no error, but wrong

The previous case can throw errors correctly if then(): Promise<T>; signature in lib.es2015.promise.d.ts is removed.

bar(fooPromise); // error

It seems there is a bug in overload processing. So I want you to fix the type checking by fixing that bug while keeping current intuitive definitions if it is a bug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants