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

es6-promise Promise.all error #5935

Closed
OliverJAsh opened this issue Dec 4, 2015 · 10 comments
Closed

es6-promise Promise.all error #5935

OliverJAsh opened this issue Dec 4, 2015 · 10 comments
Assignees
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this

Comments

@OliverJAsh
Copy link
Contributor

When I use the es6-promise type definitions, given this code:

Promise.all([
    Promise.resolve('foo'),
    Promise.resolve(['bar'])
]);

I get the following error:

The type argument for type parameter 'R' cannot be inferred from the usage. Consider specifying the type arguments explicitly.
Type argument candidate 'string' is not a valid type argument because it is not a supertype of candidate 'string[]'.

Can you explain why this is? I don't understand the error.

@blakeembrey
Copy link
Contributor

If you're using https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/es6-promise/es6-promise.d.ts#L59, I found it's because those two items in the array resolve to different promise types. Try specifying it explicitly, usually works for me:

Promise.all<[string, string[]]>([
    Promise.resolve('foo'),
    Promise.resolve(['bar'])
])

Edit: Wrong syntax, but Promise.all<Promise<string> | Promise<string[]>> works.

@OliverJAsh
Copy link
Contributor Author

Can you help me understand why this can't be inferred?

On Fri, 4 Dec 2015, 18:01 Blake Embrey notifications@github.com wrote:

If you're using
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/es6-promise/es6-promise.d.ts#L59,
I found it's because those two items in the array resolve to different
promise types. Try specifying it explicitly, usually works for me:

Promise.all<[string, string[]]>([
Promise.resolve('foo'),
Promise.resolve(['bar'])
])


Reply to this email directly or view it on GitHub
#5935 (comment)
.

@blakeembrey
Copy link
Contributor

Can you help me understand why this can't be inferred?

I'm not the best person for that, I'll leave it for the TypeScript team to answer.

@OliverJAsh
Copy link
Contributor Author

@blakeembrey I believe this is the correct syntax:

Promise.all<string | string[]>([
    Promise.resolve('foo'),
    Promise.resolve(['bar'])
])

If you try adding .then((x) => x) to the end of your assertion, you get errors.

@blakeembrey
Copy link
Contributor

Not causing issues for me. (link) What version (of TypeScript) are you using?

@OliverJAsh
Copy link
Contributor Author

Yes, that works. Sorry, I was trying to say the type assertion you provided didn't work (Promise.all<Promise<string> | Promise<string[]>>).

This is what I have ended up with:

Promise.all<string | number>([
    Promise.resolve('1'),
    Promise.resolve(1)
])
    .then(([x, y]: [string, number]) => {
        x
        y
    })

I provide the type of the tuple in the then callback function so I can continue to use the type checker. If I did the following:

Promise.all<string | number>([
    Promise.resolve('1'),
    Promise.resolve(1)
])
    .then(([x, y]) => {
        x
        y
    })

… then the compiler wouldn't know if x is a string or number.

This is a bit of a PITA. I'm trying to understand if it's possible to write a better type definition for Promise.all so assertions are not needed?

@blakeembrey
Copy link
Contributor

Right, I understand. I don't think it's currently possible to do that since there's no way to express the generic variadically, but this discussion might relate to the proposal in #5453. The TypeScript team should chime in if there's a better approach at the moment, but I've done the same thing as you ended up with in the past.

@mhegazy mhegazy added the Question An issue which isn't directly actionable in code label Dec 7, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Dec 7, 2015

untill #5453 is in, we can add a punch of overloads (say 2-10) to make Promise.all more user friendly. something like:

interface PromiseConstructor {
    all<T1, T2>(values: [T1 | PromiseLike<T1>, T2 | PromiseLike<T2>]): Promise<[T1, T2]>;
    all<T1, T2, T3>(values: [T1 | PromiseLike<T1>, T2 | PromiseLike<T2>, T3 | PromiseLike<T3>]): Promise<[T1, T2, T3]>;
    all<T1, T2, T3, T4>(values: [T1 | PromiseLike<T1>, T2 | PromiseLike<T2>, T3 | PromiseLike<T3>, T4 | PromiseLike<T4>]): Promise<[T1, T2, T3, T4]>;
    ..............
}

@sandersn thoughts?

@sandersn
Copy link
Member

sandersn commented Dec 7, 2015

Yes, multiple overloads is the accepted workaround until we have variadic generics from #5453.

I put variadic kinds behind this-types functions because this-types don't have a simple (albeit verbose) workaround. So I think it's worthwhile to add the overloads now until variadic kinds are in. I'll link the PR when it's done.

@mhegazy mhegazy added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this and removed Question An issue which isn't directly actionable in code labels Dec 7, 2015
@sandersn
Copy link
Member

sandersn commented Dec 7, 2015

Here's the PR: #5973.

@mhegazy mhegazy added this to the TypeScript 1.8 milestone Dec 7, 2015
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Dec 7, 2015
@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
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

4 participants