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

Promise.all not correctly inferring types. #34925

Closed
afrieder opened this issue Nov 5, 2019 · 16 comments · Fixed by #34501
Closed

Promise.all not correctly inferring types. #34925

afrieder opened this issue Nov 5, 2019 · 16 comments · Fixed by #34501
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@afrieder
Copy link

afrieder commented Nov 5, 2019

TypeScript Version: 3.7.2, v3.8.0-dev.20191102

Search Terms:
Promise.all
Wrong type

Code

const f1 = async (): Promise<string> => {
    return '';
};

const f2 = async (): Promise<number> => {
    return 0;
};

const f3 = async (): Promise<Object> => {
    return {};
};

const res = Promise.all([f1(), f2(), f3()]);

Expected behavior:
res has type Promise<[string, number, Object]>.

Actual behavior:
res has type Promise<Object[]>.

Playground Link:
http://www.typescriptlang.org/play/?ts=3.8.0-dev.20191102&ssl=2&ssc=1&pln=5&pc=1#code/MYewdgzgLgBAZgRhgXhgQwgTzMGAKASgC4YAFAJxAFsBLCAUwB5pyawBzAPhW4G8AoGEJjl6UAK7kwMAOQyA3PwC+-fqEiw4AJhTosOfMTKVaDRmHFUARvXLdkfQcNESpMAAyKVa8NHgBmXQxsXEISCmo6JgB5KwAremAoe0dhETFJaV4lL1V1P1EIXQjTegA6NAAbSrwAbURCABp4LSaAwgBdAnkgA

Related Issues:

@nmain
Copy link

nmain commented Nov 5, 2019

Possible duplicate of #34924

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 5, 2019

Not a duplicate. Also shown that this PR, as it is, will not fix this issue.

#33707

#33707 (comment)

@stevehollaar
Copy link

I have another Playground link example of Promise.all inferring types incorrectly. It seems when one of the results is nullable, TS incorrectly makes all results nullable:
http://www.typescriptlang.org/play/?ssl=8&ssc=1&pln=8&pc=5#code/MYewdgzgLgBAZgUysAFgRhgXhgCgJQBcMACgE4gC2AlhAgDxgCuFARgqTAD4xMA2vAPixCylGggB0pBBBC8Abghxo8AbgBQoSLETIUAJiy5CJctVp1opKmADmAbQC6QzCLPipMuYpz2A5ACGfgA0MH4sfo5q6prg0DABRgEQAJ5gwMbCMADe6jAwWvH2pGihpPqORPZMrOycfLyhVjYOjo5JAO4BVLCi5pIB-L66qGj4oSMG+FEa+YVykrwgtjglweV46gC+6gH4qkA

@AnyhowStep
Copy link
Contributor

@stevehollaar

#33707 will fix that example.

type Awaited<T> =
    T extends undefined ?
    T :
    T extends PromiseLike<infer U> ?
    U :
    T
;
declare function all<T extends readonly any[]>(
    values: T
): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }>;

//for empty tuple
declare function betterAll (arr : []) : Promise<[]>;
//for non-empty tuple
declare function betterAll<ArrT extends readonly [any, ...any[]]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;
//for non-tuple array
declare function betterAll<ArrT extends readonly any[]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;

//https://github.com/microsoft/TypeScript/issues/34925#issuecomment-550021453
const fetch1 = (): Promise<number | null> => Promise.resolve(1);
const fetch2 = (): Promise<string[]> => Promise.resolve(['a', 'b']);

async () => {
  //Error
  const [r1, r2]: [number|null, string[]] = await Promise.all([fetch1(), fetch2()]);
  console.log(r1,r2)
}

async () => {
  //OK
  const [r1, r2]: [number|null, string[]] = await all([fetch1(), fetch2()]);
  console.log(r1,r2)
}

async () => {
  //OK
  const [r1, r2]: [number|null, string[]] = await betterAll([fetch1(), fetch2()]);
  console.log(r1,r2)
}

async () => {
  //OK
  //const r1: number | null
  //const r2: string[]
  const [r1, r2] = await all([fetch1(), fetch2()]);
  console.log(r1,r2)
}

async () => {
  //OK
  //const r1: number | null
  //const r2: string[]
  const [r1, r2] = await betterAll([fetch1(), fetch2()]);
  console.log(r1,r2)
}

Playground

@junoatwork
Copy link

Here's another example I have of Promise.all behavior changes in 3.7.2 vs 3.6. I tried testing it with the new declarations in #33707 but it has the same error result.

/* eslint-disable */
type Awaited<T> = T extends undefined ? T : T extends PromiseLike<infer U> ? U : T;

function all<T extends readonly any[]>(values: T): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }>{
  return Promise.all(values) as any;
}

class K {
  getA = (): Promise<string | undefined> => {
    return Promise.resolve(undefined);
  };
  getB = (): Promise<string> => {
    return Promise.resolve('');
  };

  getC() {
    // working in 3.6.3, error in 3.7.2
    Promise.all([this.getA(), this.getB()]).then(([a, b]) => {
      const a_: string | undefined = a;
      // ts2322, b is inferred as `string | undefined`
      const b_: string = b;

      return [a_, b_];
    });

    // attempt to test with new type in PR #33707, same result
    all([this.getA(), this.getB()]).then(([a, b]) => {
      const a_: string | undefined = a;
      // ts2322, b is inferred as `string | undefined`
      const b_: string = b;

      return [a_, b_];
    });
  }
}

@AnyhowStep
Copy link
Contributor

I'm on mobile, so I can't try it but can you try the betterAll declaration?

@thaoula
Copy link

thaoula commented Nov 6, 2019

We have quite a few Promise.all and array destructuring statements in our codebase. Most are working like the Typescript 3.6.3 version except any statement that involves promises returning any.

Whenever any promise in the Promise.all call returns any all the destructured variables are marked as any.

@AnyhowStep
Copy link
Contributor

@junoatscroll

type Awaited<T> =
    T extends undefined ?
    T :
    T extends PromiseLike<infer U> ?
    U :
    T
;
declare function all<T extends readonly any[]>(
    values: T
): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }>;

//for empty tuple
declare function betterAll (arr : []) : Promise<[]>;
//for non-empty tuple
declare function betterAll<ArrT extends readonly [any, ...any[]]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;
//for non-tuple array
declare function betterAll<ArrT extends readonly any[]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;

//https://github.com/microsoft/TypeScript/issues/34925#issuecomment-550069150
class K {
  getA = (): Promise<string | undefined> => {
    return Promise.resolve(undefined);
  };
  getB = (): Promise<string> => {
    return Promise.resolve('');
  };

  getC() {
    // working in 3.6.3, error in 3.7.2
    Promise.all([this.getA(), this.getB()]).then(([a, b]) => {
      const a_: string | undefined = a;
      // ts2322, b is inferred as `string | undefined`
      const b_: string = b;

      return [a_, b_];
    });

    // attempt to test with new type in PR #33707, same result
    all([this.getA(), this.getB()]).then(([a, b]) => {
      const a_: string | undefined = a;
      // ts2322, b is inferred as `string | undefined`
      const b_: string = b;

      return [a_, b_];
    });

    betterAll([this.getA(), this.getB()]).then(([a, b]) => {
      const a_: string | undefined = a;
      //OK!
      const b_: string = b;

      return [a_, b_];
    });
  }
}

Playground

The betterAll() type works for your use case

@AnyhowStep
Copy link
Contributor

@thaoula

type Awaited<T> =
    T extends undefined ?
    T :
    T extends PromiseLike<infer U> ?
    U :
    T
;
declare function all<T extends readonly any[]>(
    values: T
): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }>;

//for empty tuple
declare function betterAll (arr : []) : Promise<[]>;
//for non-empty tuple
declare function betterAll<ArrT extends readonly [any, ...any[]]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;
//for non-tuple array
declare function betterAll<ArrT extends readonly any[]>(
    arr: ArrT
): Promise<{ -readonly [i in keyof ArrT]: Awaited<ArrT[i]> }>;

//https://github.com/microsoft/TypeScript/issues/34925#issuecomment-550092250
//const res0: Promise<any[]>
const res0 = Promise.all([Promise.resolve(1), Promise.resolve(null as any)]);
//const res1: Promise<any[]>
const res1 = all([Promise.resolve(1), Promise.resolve(null as any)]);
//const res2: Promise<[number, any]>
const res2 = betterAll([Promise.resolve(1), Promise.resolve(null as any)]);

Playground

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Nov 8, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Nov 8, 2019
@RyanCavanaugh
Copy link
Member

@rbuckton can you investigate? Thanks!

@strelga
Copy link

strelga commented Dec 11, 2019

Hey, @RyanCavanaugh!

Is there any chance a fix of this or related (#34937, #35258, etc) issues could be squeezed into some of the next 3.7.x releases?
Seems like Promise.all typings are broken in lots of projects in 3.7.3.

@strelga
Copy link

strelga commented Dec 20, 2019

Is there any chance a fix of this or related (#34937, #35258, etc) issues could be squeezed into some of the next 3.7.x releases?

@RyanCavanaugh @rbuckton friendly ping, just in case you missed my message

@csojinb
Copy link

csojinb commented Jan 2, 2020

I've got another example here. From my tests, it looks like this occurs whenever one of the promises has a narrower type that is assignable to the type of another promise in the list. I've only been able to get around it by explicitly typing Promise.all.

Note that I'm seeing this type widening occur on earlier versions of TS than 3.7, at least in the playground.

Copying code from the playground here too:

type Thing1 = Record<string, string>;
type Thing2 = {a: string; b: string;}; // assignable to Thing1

const main = async() => {

    const thing1: Thing1 = {hello: 'world'}; // const thing1: Record<string, string>
    const thing2: Thing2 = {a: 'hello', b: 'world'}; // const thing2: Thing2

    const things = [thing1, thing2];  // const things: Record<string, string>[]

    const do1 = async (t1: Thing1) => t1;
    const do2 = async (t2: Thing2) => t2;

    const promise1 = do1(thing1);  // const promise1: Promise<Record<string, string>>
    const promise2 = do2(thing2);  // const promise2: Promise<Thing2>

    const promises = [promise1, promise2];  // const promises: Promise<Record<string, string>>[]

    // const superPromiseFromPromises: Promise<Record<string, string>[]>
    const superPromiseFromPromises = Promise.all(promises);
    
    // const a1: Record<string, string>
    // const a2: Record<string, string>
    const [a1, a2] = await superPromiseFromPromises;

    // const superPromiseFromLiteral: Promise<Record<string, string>[]>
    const superPromiseFromLiteral = Promise.all([promise1, promise2]);

    // const b1: Record<string, string>
    // const b2: Record<string, string>
    const [b1, b2] = await superPromiseFromLiteral;

    // const c1: Record<string, string>
    // const c2: Record<string, string>
    const [c1, c2] = await Promise.all([promise1, promise2]);

    // const explicitlyTypedPromises: [Promise<Record<string, string>>, Promise<Thing2>]
    const explicitlyTypedPromises: [Promise<Thing1>, Promise<Thing2>] = [promise1, promise2];

    // const d1: Record<string, string>
    // const d2: Record<string, string>
    const [d1, d2] = await Promise.all(explicitlyTypedPromises);

    // const e1: Record<string, string>
    // const e2: Thing2
    const [e1, e2] = await Promise.all<Thing1, Thing2>([promise1, promise2]);
}

@schusovskoy
Copy link

I've also found that it can be related with how readonly modifier works. Here is the playground link describing what I mean.

@adrienboulle
Copy link

@RyanCavanaugh if it can help i have this example

Here the thing that break type inferring is (oddly) the type {[key: string]: any}

@moodseller
Copy link

I have also found that using Promise.all in such way:

const [pr1, pr2] = await Promise.all([fn1(), fn2()]);

resolves pr1, pr2 variable types to type | null if any of the async functions which are present in Promise.all can alternatively return null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.