feat(lib/es2020): Add Promise.allSettled(…)#34065
feat(lib/es2020): Add Promise.allSettled(…)#34065DanielRosenwasser merged 7 commits intomicrosoft:masterfrom
Promise.allSettled(…)#34065Conversation
|
LGTM |
|
I guess this code can be simplified similarly to #33707 ? |
|
Nope, typings for node still need to support 2.1 until further notice. |
|
I first need #33707 to be merged, so that I can get access to the |
| * @param values An array of Promises. | ||
| * @returns A new Promise. | ||
| */ | ||
| allSettled<T extends readonly any[] | readonly [any]>(values: T): Promise<{ -readonly [P in keyof T]: PromiseSettledResult<T[P] extends PromiseLike<infer U> ? U : T[P]> }>; |
There was a problem hiding this comment.
Should we make these readonly unknown[] | readonly [unknown]? (@RyanCavanaugh @rbuckton)
| @@ -0,0 +1,29 @@ | |||
| interface PromiseResolvedResult<T> { | |||
There was a problem hiding this comment.
Hi. Drive-by code review here. Shouldn’t this be named something like PromiseFulfilledResult to adhere to the states and fates terminology? I think the reason calling it PromiseResolvedResult is incorrect here is that while all fulfilled promises are resolved, not all resolved promises are fulfilled. A promise can be resolved to a pending promise, in which case it has not settled, and it can also be resolved to a rejected promise, in which case it has not fulfilled.
If my interpretation of states and fates terminology is correct, shouldn’t the correct name of this interface be PromiseFulfilledResult?
ec3628c to
2929aad
Compare
|
Thank you! |
|
Still feel like it should've used |
|
LGTM |
Depends on #33707(I’ve now inlined theAwaited<T>type)Fixes #31083
Closes #31085
review?(@Kingwl)