Skip to content

Promise.all returns all values with type of the most common interface. #16017

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
megaboich opened this issue May 23, 2017 · 5 comments
Closed
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@megaboich
Copy link

TypeScript Version: 2.3.3 / nightly (2.4.0-dev.20170523)

The problem can be reproduced with this code:

interface A {
    id: string
}

interface B {
    id: string
    fieldB: string
}

async function countEverything(): Promise<number> {
    const providerA = async (): Promise<A[]> => { return [] }
    const providerB = async (): Promise<B[]> => { return [] }

    const [resultA, resultB] = await Promise.all([
        providerA(),
        providerB(),
    ]);

    const dataA: A[] = resultA;
    const dataB: B[] = resultB;
    if (dataA && dataB) {
        return dataA.length + dataB.length;
    }
    return 0;
}

Expected behavior:
Compiles without errors

Actual behavior:
Compilation error:

20     const dataB: B[] = resultB;
             ~~~~~
promise-all-issue.ts(20,11): error TS2322: Type 'A[]' is not assignable to type 'B[]'.
  Type 'A' is not assignable to type 'B'.
    Property 'fieldB' is missing in type 'A'.

I see that interface B is compatible with interface A, but I don't think that Promise.all should return all values as A[].
When you change interface A for example adding a field fieldA, then interface B is not compatible anymore and the code compiles without errors.

I have created very small repository which reproduces this issue.
https://github.com/megaboich/ts-promise-all-issue

@mhegazy
Copy link
Contributor

mhegazy commented May 23, 2017

Looks like the issue here is we have two definitions for Promise.all, one in lib.es2015.promise.d.ts and one in lib.es2015.iterable.d.ts, the earlier defines a set of overloads with a tuple types and thus get type of [dataA, dataB] correctly, the other has an array overload, that takes a list and returns a list, and thus gets the type as the best common type between A and B.

The fix seems to reorder these overloads somehow to make sure the overloads from lib.es2015.promise.d.ts come after lib.es2015.iterable.d.ts,; or that lib.es2015.iterable.d.ts, redefines the same overloads.

As a workaround, list the --lib explicitly with this order: --lib es5,es2015.promise,es2015.iterable

@mhegazy mhegazy added the Bug A bug in TypeScript label May 23, 2017
@mhegazy mhegazy added this to the TypeScript 2.4 milestone May 23, 2017
@megaboich
Copy link
Author

megaboich commented May 24, 2017

Thanks @mhegazy!

I have applied your workaround and may confirm it works.
I modified my tsconfig.json to include this:

        "lib": [
            "dom",
            "es5",
            "es2015.promise",
            "es2015.iterable",
            "es2017"
        ],

Still I find this --lib option a bit confusing, I have read specifications here:
https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#including-built-in-type-declarations-with---lib

But it is still not very clear what to specify there if I target the browser and want to have available all features polifilled by core.js for instance. Should I specify es2015 or es2016 or es2016.array.include if I have already specified es2017?

Is there somewhere better documentation about all of this?

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

--target is what version of ECMAScript is your target engine supporting. for instance --target ES5 would be safe if you are using IE11. --target ES2017 is safe if you are using [email protected].

By default a matching version of the library is added to your compilation based on your target, so --target ES6 gets you the equivalent lib.es6.d.ts, and --target ES2017 gets you lib.es2017.d.ts. By default you also get things like DOM APIs.

Sometimes you want to add definitions that are not part of the standard, for instance ones you have polyfiled, e.g. Promise. for these we recommend explicitly adding the library portion that you need, e.g. --target es5 --lib es5,es2015.promise.

Library versions are cumulative, so if you set --lib es2017 this includes definitions in es2017, es2016 and es5.

hope that helps.

@ghost
Copy link

ghost commented May 28, 2017

I'm wondering why this code doesn't compile unless I provide type arguments explicitly.

function combine<T, U>(a: T, b: U): Promise<[T, U]> {
    return Promise.all([a, b] as [T, U]);
}

It compiles fine if I use Promise<T> or Promise<U> somewhere, but not if neither is a promise. (That shouldn't happen in real code, of course.)

@yuit
Copy link
Contributor

yuit commented Jun 15, 2017

@Andy-MS it is because we pick up overload all<TAll>(values: Iterable<TAll | PromiseLike<TAll>>): Promise<TAll[]>; from lib.es2015.iterable.d.ts due to how it is order.

I think the fix here is to remove Promise constructor definition from lib.es2015.iterable.d.ts and just reference ``lib.es2015.promise.d.ts`

@yuit yuit added the Fixed A PR has been merged for this issue label Jun 15, 2017
@yuit yuit modified the milestones: TypeScript 2.5, TypeScript 2.4.1 Jun 22, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants