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

Better typings for Promise, like #31117 #33707

Closed
wants to merge 4 commits into from
Closed

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Oct 1, 2019

Roll up #33055, #33062, #33074, #33103 and #33105

Fixes #27711
Fixes #22469
Fixes #28427
Fixes #30390
Fixes #31722
Fixes #33559
Fixes #33562
Fixes #33752
Fixes #34924
Fixes #34937
Fixes #35136
Fixes #35258

@jablko jablko marked this pull request as ready for review October 1, 2019 20:56
@jablko
Copy link
Contributor Author

jablko commented Oct 1, 2019

@typescript-bot test this

* @returns A new Promise.
*/
race<T>(values: readonly T[]): Promise<T extends PromiseLike<infer U> ? U : T>;
all<T extends readonly any[]>(values: T): Promise<{ -readonly [P in keyof T]: Awaited<T[P]> }>;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this overload, are the tuple-based overloads even necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! I've now updated this PR accordingly. Thank you!

@jablko jablko force-pushed the patch-14 branch 8 times, most recently from 7224336 to ba2ffbc Compare October 9, 2019 16:37
@orta
Copy link
Contributor

orta commented Oct 15, 2019

@typescript-bot test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 15, 2019

Heya @orta, I've started to run the extended test suite on this PR at ba2ffbc. You can monitor the build here. It should now contribute to this PR's status checks.

@jablko
Copy link
Contributor Author

jablko commented Oct 15, 2019

@typescript-bot user test this

@RyanCavanaugh
Copy link
Member

@rbuckton can you please review and merge if this is good to go?

@jablko jablko force-pushed the patch-14 branch 2 times, most recently from 500160b to 16ded29 Compare October 17, 2019 19:45
@jablko
Copy link
Contributor Author

jablko commented Oct 17, 2019

@rbuckton or @RyanCavanaugh Can you please trigger @typescript-bot test this, run dt and user test this for me?

* @param values An array of Promises.
* @returns A new Promise.
*/
race<T>(values: Iterable<T | PromiseLike<T>>): Promise<T>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was race removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4841984 it was moved to es2015.promise.d.ts in #31117

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It existed here so that you could include es2015.promise without es2015.iterable, depending on what shims/polyfills are used down-level, although it looks like a definition including Iterable was incorrectly added to that file at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to fix that but just wondering, is it better to be able to include es2015.promise without es2015.iterable, or es2015.iterable without es2015.promise? The latter is more obvious (all Promise.all() and Promise.race() definitions located together) and has fewer overloads (since readonly T[] extends Iterable<T>).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I get it now: es2015.iterable doesn't depend on PromiseConstructor, it augments it. I'll move that Iterable overload back to es2015.iterable.

Copy link
Contributor Author

@jablko jablko Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️ Done.

@sandersn sandersn added this to Needs review in PR Backlog Mar 9, 2020
@sandersn sandersn changed the title Better typings for Promise.all(), like #31117 Better typings for Promise, like #31117 Mar 9, 2020
@sandersn sandersn removed this from Curio in Pall Mall Mar 9, 2020
@Luxcium
Copy link

Luxcium commented Mar 24, 2020

as in my issue comment in #37526

Update

My problem is solved with the nightly version of typescript but I will leave it there in case it can be useful for something or someone

"typescript": "3.9.0-dev.20200324"

I have placed a copy of this resolved problem in #33055 and in #33707 hoping it can be helpful and not too spammy ...

Issue now fixed

I have a similar problem and I don't know if I should open a different issue ...

"typescript": "3.8.3"

    // Promise.all(this.fork) take a T1[] where T1 is a Promise<number>
    // and return a Promise<T2[]> where T2 is a number
    // therefore T1 and T2 are not both «T»

class MyMaybeList<T = any>, le 2020-03-24 à 03 45 35 EST

export class MyMaybeList<T = any> {
  private constructor(values: T[]) {
    this.values = values;
  }

  private values: T[];

  // get =======================================================-| fork() |-====

  public get fork(): T[] {
    return this.values != null ? this.values.slice() : [].slice();
  }

  // public =====================================================-| map() |-====

  public map<R = any>(
    fn: (val: T, index: number, array: T[]) => R
  ): MyMaybeList<R> {
    return MyMaybeList.of(...this.values.map(fn));
  }

  // static ======================================================-| of() |-====

  public static of<TVal>(...val: TVal[]): MyMaybeList<TVal> {
    if (val == null || !val.length) return new MyMaybeList<TVal>([]);
    return new MyMaybeList<TVal>([...val]);
  }

  // async =====================================================-| will() |-====

  public async will /* <R> */() /* : Promise<MyMaybeList<R>> */ {
    // Promise.all(this.fork) take a T1[] where T1 is Promise<number>
    // and return a Promise<T2[]> where T2 is a number
    // therefore T1 and T2 are not both «T»
    console.log(this.fork);

    const willThen = Promise.all(this.fork);

    const thenWill = await willThen;
    return MyMaybeList.of(...thenWill);
  }
}

Example, le 2020-03-24 à 03 46 09 EST

const oneToTen = MyMaybeList.of(1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
const powersOneToTen = oneToTen.map(async val => val * val);

// "log->" powersOneToTen: MyMaybeList<Promise<number>>
console.log(powersOneToTen);

// "log->" awaitedList: MyMaybeList<Promise<number>>
// instead of a -> MyMaybeList { Promise<number> }
// in fact is a -> Promise {  MyMaybeList<number> }
const awaitedPowersOneToTen = powersOneToTen.will /* <unnknow> */();
awaitedPowersOneToTen.then(awaitedList => console.log(awaitedList));

// Promise { <pending> } will resolve into ->
console.log(awaitedPowersOneToTen);

Console logs le 2020-03-24 à 03 46 58 EST

/*
 // Promise.all(this.fork) take a T1[] where T1 is Promise<number>
    // and return a Promise<T2[]> where T2 is a number
    // therefore T1 and T2 are not both «T»
    // console.log(this.fork); will display same as console.log(powersOneToTen);



// console.log(powersOneToTen); ->
// "log->" powersOneToTen: MyMaybeList<Promise<number>>
MyMaybeList {
  values: [
    Promise { 1 },
    Promise { 4 },
    Promise { 9 },
    Promise { 16 },
    Promise { 25 },
    Promise { 36 },
    Promise { 49 },
    Promise { 64 },
    Promise { 81 },
    Promise { 100 }
  ]
}



// console.log(awaitedPowersOneToTen); ->
// "log ->" awaitedPowersOneToTen: Promise<MyMaybeList<Promise<number>>>
Promise { <pending> }
// Promise { <pending> } will resolve into ->

// awaitedPowersOneToTen.then(awaitedList => console.log(awaitedList)); ->
// console.log(awaitedList) ->
// "log->" awaitedList: MyMaybeList<Promise<number>>
// instead of a -> MyMaybeList { Promise<number> }
// in fact is a (should be) -> Promise {  MyMaybeList<number> }
MyMaybeList {
  values: [
     1,  4,  9, 16,  25,
    36, 49, 64, 81, 100
  ]
}
*/

@Luxcium
Copy link

Luxcium commented Mar 27, 2020

Now that #37610 has reverted the awaited type I don't know what to do I have posted an example above but my real-life situation is as follow :

I was relying on this solution because I have a Promise<MaybeList<Promise<ISymbolSearchResult[]>>>
where MaybeList is an array abstraction so I am using promise.all to remove the inner Promise I don't know what to do now to avoid using as unknown as MaybeList<ISymbolSearchResult[]> inside of my async function (which should be returning the Promise<MaybeList<ISymbolSearchResult[]>> instead of the Promise<MaybeList<Promise<ISymbolSearchResult[]>>>)

@ValentinH
Copy link

Sorry to be that guy but what is the status of this PR? The regression in #33752 is super annoying. 😞

@strelga
Copy link

strelga commented May 7, 2020

@ValentinH The regression in #33752 should be now fixed with #34501, it will be released with 3.9 on May, 12.

@ValentinH
Copy link

@strelga thank you really much, this is good news.

Still I found the new definition for Promise.all() pretty interesting as it's not limited to 10 elements anymore. So I'm still interested to know the status and what is blocking?

@jakebailey
Copy link
Member

All of the issues this PR links are closed; I think that this all was handled in 4.5 with Awaited.

I'm going to close this, but am happy to reopen if I'm mistaken.

@jakebailey jakebailey closed this Jan 24, 2022
PR Backlog automation moved this from Waiting on reviewers to Done Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment