- Notifications
You must be signed in to change notification settings - Fork 13.2k
Better typings for Promise, like #31117#33707
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jablko commented Oct 1, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
jablko commented Oct 1, 2019
@typescript-bot test this |
9e9dd9c to 497d816Compare| * @returns A new Promise. | ||
| */ | ||
| race<T>(values: readonlyT[]): Promise<TextendsPromiseLike<infer U> ? U : T>; | ||
| all<Textendsreadonlyany[]>(values: T): Promise<{-readonly[PinkeyofT]: Awaited<T[P]>}>; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!
7224336 to ba2ffbcCompareorta commented Oct 15, 2019
@typescript-bot test this |
typescript-bot commented Oct 15, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
jablko commented Oct 15, 2019
@typescript-bot user test this |
RyanCavanaugh commented Oct 17, 2019
@rbuckton can you please review and merge if this is good to go? |
500160b to 16ded29Comparejablko 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>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was race removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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>).
There was a problem hiding this comment.
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.
jablkoNov 21, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️ Done.
57ad578 to 9cdcb89CompareYou can include es2015.promise.d.ts without es2015.iterable.d.ts. It was moved to es2015.promise.d.ts in microsoft#31117
Luxcium commented Mar 24, 2020 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
as in my issue comment in #37526 UpdateMy 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
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 fixedI have a similar problem and I don't know if I should open a different issue ...
// 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»exportclassMyMaybeList<T=any>{privateconstructor(values: T[]){this.values=values;}privatevalues: T[];// get =======================================================-| fork() |-====publicgetfork(): T[]{returnthis.values!=null ? this.values.slice() : [].slice();}// public =====================================================-| map() |-====publicmap<R=any>(fn: (val: T,index: number,array: T[])=>R): MyMaybeList<R>{returnMyMaybeList.of(...this.values.map(fn));}// static ======================================================-| of() |-====publicstaticof<TVal>(...val: TVal[]): MyMaybeList<TVal>{if(val==null||!val.length)returnnewMyMaybeList<TVal>([]);returnnewMyMaybeList<TVal>([...val]);}// async =====================================================-| will() |-====publicasyncwill/* <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);constwillThen=Promise.all(this.fork);constthenWill=awaitwillThen;returnMyMaybeList.of(...thenWill);}}constoneToTen=MyMaybeList.of(1,2,3,4,5,6,7,8,9,10);constpowersOneToTen=oneToTen.map(asyncval=>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>}constawaitedPowersOneToTen=powersOneToTen.will/* <unnknow> */();awaitedPowersOneToTen.then(awaitedList=>console.log(awaitedList));// Promise{<pending>} will resolve into ->console.log(awaitedPowersOneToTen);/* // 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 commented Mar 27, 2020
Now that #37610 has reverted the
|
ValentinH commented May 7, 2020
Sorry to be that guy but what is the status of this PR? The regression in #33752 is super annoying. 😞 |
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 commented May 7, 2020
@strelga thank you really much, this is good news. Still I found the new definition for |
jakebailey commented Jan 24, 2022
All of the issues this PR links are closed; I think that this all was handled in 4.5 with I'm going to close this, but am happy to reopen if I'm mistaken. |



Roll up #33055,
#33062,#33074,#33103and#33105Fixes
#27711Fixes#22469
Fixes#28427
Fixes
#30390Fixes#31722
Fixes#33559
Fixes#33562
Fixes#33752
Fixes
#34924Fixes#34937
Fixes
#35136Fixes
#35258