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

Adds 'Awaited' type alias and updates to Promise.all/race/allSettled/any #45350

Merged
merged 4 commits into from
Sep 10, 2021

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Aug 7, 2021

This adds an Awaited<T> type alias that supports the following capabilities:

  • Recursive unwrap
  • Resolves promise-like "thenables" without relying on PromiseLike (to avoid issues with augmentation/assignability)
  • Non promise "thenables" resolve to never
  • Supports null and undefined in non-strictNullChecks mode
  • Relies on recursion limiters to provide errors for unresolvable promises.

This also adds overloads to Promise.all, Promise.race, Promise.allSettled, and Promise.any to leverage Awaited<T>.

Supersedes #33707, thanks @jablko for your prior work on this.

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

@rbuckton
Copy link
Member Author

@typescript-bot perf test
@typescript-bot run dt
@typescript-bot test this
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 25, 2021

Heya @rbuckton, I've started to run the parallelized Definitely Typed test suite on this PR at 46ce88c. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 25, 2021

Heya @rbuckton, I've started to run the perf test suite on this PR at 46ce88c. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 25, 2021

Heya @rbuckton, I've started to run the parallelized community code test suite on this PR at 46ce88c. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 25, 2021

Heya @rbuckton, I've started to run the extended test suite on this PR at 46ce88c. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@rbuckton
Copy link
Member Author

Interesting outcomes from the rwc tests:

  • firebase.log: Cannot find module 'firebase' or its corresponding type declarations. - Probably not due to this change...
  • chrome-devtools-frontend.log: 4 errors go away, one error changes from:
    node_modules/chrome-devtools-frontend/front_end/elements/ComputedStyleWidget.js(91,34): 
      error TS2339: Property 'spread' does not exist on type 'Promise<[any, any, any, any, any, any, any, any, any, any]>'.
    
    to
    node_modules/chrome-devtools-frontend/front_end/elements/ComputedStyleWidget.js(91,34): 
      error TS2339: Property 'spread' does not exist on type 'Promise<(CSSMatchedStyles | ComputedStyle)[]>'.
    
  • prettier.log: OOM. Not sure if this is because of this change or something else, I'll investigate.

@typescript-bot
Copy link
Collaborator

@rbuckton
The results of the perf run you requested are in!

Here they are:

Comparison Report - main..45350

Metric main 45350 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 351,106k (± 0.02%) 351,322k (± 0.02%) +216k (+ 0.06%) 351,198k 351,601k
Parse Time 1.92s (± 1.88%) 1.92s (± 1.43%) -0.00s (- 0.10%) 1.88s 2.02s
Bind Time 0.87s (± 0.89%) 0.85s (± 0.47%) -0.02s (- 2.30%) 0.84s 0.86s
Check Time 5.40s (± 0.48%) 5.38s (± 0.36%) -0.02s (- 0.33%) 5.33s 5.42s
Emit Time 5.86s (± 0.50%) 5.84s (± 0.77%) -0.01s (- 0.20%) 5.77s 5.99s
Total Time 14.04s (± 0.40%) 13.99s (± 0.38%) -0.05s (- 0.38%) 13.91s 14.11s
Compiler-Unions - node (v10.16.3, x64)
Memory used 203,794k (± 0.39%) 203,415k (± 0.04%) -379k (- 0.19%) 203,242k 203,540k
Parse Time 0.78s (± 1.04%) 0.79s (± 0.74%) +0.00s (+ 0.51%) 0.78s 0.80s
Bind Time 0.53s (± 1.14%) 0.53s (± 0.94%) +0.00s (+ 0.19%) 0.52s 0.54s
Check Time 7.84s (± 0.57%) 7.78s (± 0.40%) -0.06s (- 0.75%) 7.71s 7.84s
Emit Time 2.44s (± 0.39%) 2.43s (± 0.92%) -0.01s (- 0.25%) 2.39s 2.49s
Total Time 11.59s (± 0.41%) 11.52s (± 0.32%) -0.07s (- 0.59%) 11.41s 11.58s
Monaco - node (v10.16.3, x64)
Memory used 340,673k (± 0.02%) 340,553k (± 0.02%) -120k (- 0.04%) 340,442k 340,717k
Parse Time 1.46s (± 1.20%) 1.45s (± 1.83%) -0.01s (- 0.34%) 1.40s 1.54s
Bind Time 0.74s (± 0.95%) 0.75s (± 0.80%) +0.00s (+ 0.67%) 0.74s 0.76s
Check Time 5.42s (± 0.46%) 5.38s (± 0.47%) -0.04s (- 0.72%) 5.30s 5.42s
Emit Time 3.13s (± 0.66%) 3.14s (± 0.93%) +0.01s (+ 0.29%) 3.09s 3.23s
Total Time 10.75s (± 0.42%) 10.71s (± 0.38%) -0.03s (- 0.31%) 10.60s 10.82s
TFS - node (v10.16.3, x64)
Memory used 304,064k (± 0.03%) 303,921k (± 0.02%) -144k (- 0.05%) 303,768k 304,064k
Parse Time 1.19s (± 0.97%) 1.19s (± 1.22%) 0.00s ( 0.00%) 1.17s 1.24s
Bind Time 0.71s (± 0.56%) 0.71s (± 1.13%) -0.00s (- 0.42%) 0.69s 0.72s
Check Time 4.89s (± 0.45%) 4.92s (± 0.42%) +0.02s (+ 0.47%) 4.85s 4.95s
Emit Time 3.32s (± 1.40%) 3.35s (± 1.44%) +0.03s (+ 0.81%) 3.24s 3.43s
Total Time 10.11s (± 0.46%) 10.16s (± 0.58%) +0.05s (+ 0.47%) 10.01s 10.26s
material-ui - node (v10.16.3, x64)
Memory used 469,701k (± 0.01%) 469,821k (± 0.01%) +119k (+ 0.03%) 469,691k 469,935k
Parse Time 1.74s (± 2.11%) 1.74s (± 2.12%) +0.00s (+ 0.23%) 1.71s 1.89s
Bind Time 0.67s (± 0.60%) 0.66s (± 0.79%) -0.01s (- 1.49%) 0.65s 0.67s
Check Time 14.13s (± 0.35%) 14.17s (± 0.90%) +0.04s (+ 0.30%) 13.97s 14.62s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.54s (± 0.31%) 16.57s (± 0.80%) +0.03s (+ 0.21%) 16.33s 17.02s
Angular - node (v12.1.0, x64)
Memory used 329,162k (± 0.03%) 329,233k (± 0.03%) +70k (+ 0.02%) 328,933k 329,428k
Parse Time 1.90s (± 0.65%) 1.88s (± 0.35%) -0.02s (- 0.89%) 1.87s 1.90s
Bind Time 0.86s (± 0.43%) 0.84s (± 0.95%) -0.02s (- 2.66%) 0.83s 0.87s
Check Time 5.28s (± 0.50%) 5.21s (± 0.52%) -0.07s (- 1.25%) 5.16s 5.25s
Emit Time 6.14s (± 0.84%) 6.08s (± 0.74%) -0.05s (- 0.86%) 6.01s 6.23s
Total Time 14.18s (± 0.58%) 14.02s (± 0.33%) -0.16s (- 1.14%) 13.93s 14.12s
Compiler-Unions - node (v12.1.0, x64)
Memory used 190,778k (± 0.15%) 190,835k (± 0.08%) +57k (+ 0.03%) 190,279k 191,083k
Parse Time 0.78s (± 0.57%) 0.78s (± 1.00%) -0.00s (- 0.38%) 0.77s 0.80s
Bind Time 0.54s (± 0.68%) 0.54s (± 0.68%) +0.00s (+ 0.00%) 0.53s 0.54s
Check Time 7.37s (± 0.83%) 7.31s (± 0.54%) -0.07s (- 0.90%) 7.22s 7.38s
Emit Time 2.45s (± 0.77%) 2.43s (± 1.10%) -0.02s (- 0.61%) 2.40s 2.52s
Total Time 11.14s (± 0.62%) 11.05s (± 0.56%) -0.08s (- 0.75%) 10.96s 11.23s
Monaco - node (v12.1.0, x64)
Memory used 323,773k (± 0.01%) 323,740k (± 0.02%) -33k (- 0.01%) 323,615k 323,923k
Parse Time 1.43s (± 0.66%) 1.42s (± 0.65%) -0.01s (- 0.56%) 1.40s 1.44s
Bind Time 0.73s (± 0.91%) 0.72s (± 0.92%) -0.01s (- 1.37%) 0.71s 0.74s
Check Time 5.30s (± 0.51%) 5.29s (± 0.51%) -0.01s (- 0.21%) 5.22s 5.34s
Emit Time 3.21s (± 0.49%) 3.21s (± 0.98%) +0.00s (+ 0.09%) 3.17s 3.33s
Total Time 10.67s (± 0.39%) 10.64s (± 0.45%) -0.03s (- 0.24%) 10.52s 10.74s
TFS - node (v12.1.0, x64)
Memory used 288,800k (± 0.02%) 288,745k (± 0.02%) -54k (- 0.02%) 288,639k 288,842k
Parse Time 1.21s (± 0.90%) 1.20s (± 1.05%) -0.01s (- 0.99%) 1.17s 1.23s
Bind Time 0.70s (± 1.07%) 0.70s (± 0.68%) 0.00s ( 0.00%) 0.69s 0.71s
Check Time 4.85s (± 0.45%) 4.84s (± 0.69%) -0.01s (- 0.21%) 4.75s 4.91s
Emit Time 3.37s (± 1.20%) 3.34s (± 1.12%) -0.03s (- 0.80%) 3.25s 3.44s
Total Time 10.13s (± 0.51%) 10.08s (± 0.65%) -0.05s (- 0.51%) 9.87s 10.23s
material-ui - node (v12.1.0, x64)
Memory used 448,530k (± 0.02%) 448,571k (± 0.01%) +41k (+ 0.01%) 448,412k 448,653k
Parse Time 1.74s (± 0.51%) 1.72s (± 0.35%) -0.02s (- 1.21%) 1.70s 1.73s
Bind Time 0.65s (± 1.16%) 0.65s (± 0.95%) -0.00s (- 0.46%) 0.64s 0.66s
Check Time 12.91s (± 0.47%) 12.77s (± 0.56%) -0.14s (- 1.09%) 12.62s 12.92s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.30s (± 0.36%) 15.13s (± 0.49%) -0.16s (- 1.08%) 14.98s 15.30s
Angular - node (v14.15.1, x64)
Memory used 327,782k (± 0.00%) 327,831k (± 0.01%) +50k (+ 0.02%) 327,780k 327,867k
Parse Time 1.92s (± 0.51%) 1.89s (± 0.37%) -0.03s (- 1.61%) 1.88s 1.91s
Bind Time 0.91s (± 0.77%) 0.87s (± 0.60%) 🟩-0.04s (- 4.39%) 0.86s 0.88s
Check Time 5.33s (± 0.48%) 5.26s (± 0.53%) -0.07s (- 1.37%) 5.21s 5.31s
Emit Time 6.26s (± 0.63%) 6.14s (± 0.81%) -0.11s (- 1.81%) 6.06s 6.28s
Total Time 14.42s (± 0.51%) 14.17s (± 0.52%) -0.25s (- 1.75%) 14.06s 14.33s
Compiler-Unions - node (v14.15.1, x64)
Memory used 190,432k (± 0.51%) 191,428k (± 0.60%) +996k (+ 0.52%) 189,517k 192,746k
Parse Time 0.81s (± 0.61%) 0.81s (± 0.74%) -0.00s (- 0.12%) 0.80s 0.82s
Bind Time 0.57s (± 0.60%) 0.56s (± 0.59%) -0.01s (- 1.23%) 0.55s 0.57s
Check Time 7.55s (± 0.86%) 7.47s (± 0.58%) -0.07s (- 0.98%) 7.38s 7.55s
Emit Time 2.44s (± 0.75%) 2.42s (± 0.56%) -0.02s (- 0.90%) 2.39s 2.44s
Total Time 11.36s (± 0.58%) 11.26s (± 0.36%) -0.10s (- 0.90%) 11.18s 11.34s
Monaco - node (v14.15.1, x64)
Memory used 322,558k (± 0.00%) 322,512k (± 0.00%) -46k (- 0.01%) 322,473k 322,541k
Parse Time 1.49s (± 0.45%) 1.49s (± 0.69%) -0.00s (- 0.27%) 1.47s 1.51s
Bind Time 0.75s (± 0.65%) 0.75s (± 0.63%) -0.00s (- 0.13%) 0.75s 0.77s
Check Time 5.21s (± 0.44%) 5.22s (± 0.50%) +0.01s (+ 0.15%) 5.17s 5.28s
Emit Time 3.25s (± 0.73%) 3.21s (± 0.72%) -0.03s (- 1.05%) 3.15s 3.26s
Total Time 10.71s (± 0.28%) 10.68s (± 0.35%) -0.03s (- 0.30%) 10.60s 10.77s
TFS - node (v14.15.1, x64)
Memory used 287,718k (± 0.01%) 287,650k (± 0.00%) -68k (- 0.02%) 287,619k 287,673k
Parse Time 1.25s (± 1.84%) 1.25s (± 1.51%) -0.00s (- 0.32%) 1.21s 1.29s
Bind Time 0.74s (± 4.38%) 0.73s (± 3.53%) -0.01s (- 1.89%) 0.71s 0.83s
Check Time 4.88s (± 0.60%) 4.85s (± 0.47%) -0.03s (- 0.53%) 4.81s 4.90s
Emit Time 3.47s (± 0.77%) 3.47s (± 0.83%) -0.00s (- 0.03%) 3.41s 3.55s
Total Time 10.34s (± 0.57%) 10.29s (± 0.49%) -0.05s (- 0.44%) 10.21s 10.48s
material-ui - node (v14.15.1, x64)
Memory used 446,826k (± 0.03%) 446,952k (± 0.00%) +125k (+ 0.03%) 446,915k 447,000k
Parse Time 1.76s (± 0.40%) 1.76s (± 0.61%) 0.00s ( 0.00%) 1.73s 1.78s
Bind Time 0.69s (± 0.43%) 0.69s (± 0.83%) +0.00s (+ 0.29%) 0.68s 0.70s
Check Time 12.92s (± 0.33%) 12.94s (± 0.59%) +0.02s (+ 0.16%) 12.79s 13.11s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.37s (± 0.30%) 15.39s (± 0.48%) +0.03s (+ 0.17%) 15.24s 15.55s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory11 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
Benchmark Name Iterations
Current 45350 10
Baseline main 10

Developer Information:

Download Benchmark

@rbuckton
Copy link
Member Author

The OOM in Prettier doesn't seem to be related to this change, as I'm seeing it in other unrelated PRs as well.

@rbuckton rbuckton requested a review from andrewbranch August 30, 2021 18:36
@rbuckton
Copy link
Member Author

rbuckton commented Aug 30, 2021

Notes from design meeting:

  • Need to see what this breaks.
    • Nothing seems broken in the RWC and User tests as is.
  • Need to see if we can remove overloads.
    • I'm concerned that removing overloads for all will be a breaking change for anyone who has previously used
      explicit type arguments to overcome inference issues that this will ostensibly solve. The question is whether
      this is an acceptable breaking change.
  • Maybe see if this can be leveraged by await?
    • I am exploring this in a separate PR that I will post shortly. This is likely to be a very breaking change in higher-order, since await PT where PT is a Promise<T> and T is a type variable will now produce an Awaited<T> instead of T. While this is arguably more correct (which was the case in the awaited T PR), it is also likely to be a breaking change.

@rbuckton
Copy link
Member Author

rbuckton commented Sep 3, 2021

I investigated whether having await return Awaited<T> would be a major breaking change in #45701 and it doesn't seem like that's the case, so I will merge the changes into this PR.

Comment on lines +1453 to +1454
T : // argument was not an object
T; // non-thenable
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that these comments are swapped.

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently so. I will push up a small fix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed by #45918

@frigus02
Copy link
Contributor

If anyone in the community heavily uses await with generics and/or Promise.all, please give the above build a try and let me know your experience.

I did a quick search for Promise.all usages in the Google code base internally and only found a handful (~10) of usages with more than 1 generic parameter. We should be fine here.

To check usages of await with generics we will have to actually upgrade TypeScript and see where the builds break, I think. We'll probably report that together with other findings.

@julienavert
Copy link

Hello everyone,

Not sure if this is the correct place to bring this up but the Awaited<T> type as-is broke our build after upgrading from typescript 4.4.3 to 4.5.2.

For reference, here is what Awaited<T> looks like:

/**
 * Recursively unwraps the "awaited type" of a type. Non-promise "thenables" should resolve to `never`. This emulates the behavior of `await`.
 */
type Awaited<T> =
    T extends null | undefined ? T : // special case for `null | undefined` when not in `--strictNullChecks` mode
        T extends object & { then(onfulfilled: infer F): any } ? // `await` only unwraps object types with a callable `then`. Non-object types are not unwrapped
            F extends ((value: infer V) => any) ? // if the argument to `then` is callable, extracts the argument
                Awaited<V> : // recursively unwrap the value
                never : // the argument to `then` was not callable
        T; // non-object or non-thenable

The problem is that we have a custom Promise class (AbortablePromise) and its then function accepts a onfulfilled callback that do not exactly match the condition F extends ((value: infer V) => any).

Our then is declared like this:

  public then<TResult1 = T, TResult2 = never>(
    onfulfilled?: ((value: T, abortController: AbortController) => TResult1 | PromiseLike<TResult1>) | undefined | null,
    onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null
  ): AbortablePromise<TResult1 | TResult2>;

So onfulfilled in our case is (value: T, abortController: AbortController) => TResult1 | PromiseLike<TResult1>) which does not match (value: infer V) => any

Hence, Awaited<T> where T is an AbortablePromise always returns never.

Would it be possible to make the type condition more flexible in Awaited<T> to enable these promise extensions and still have the type be compatible with Promise.all, Promise.race, etc?

@wbt
Copy link

wbt commented Aug 10, 2022

It would be helpful to add some documentation about this on the Utility types page, not just in the 4.5 release notes that aren't part of the docs search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment