Skip to content

Fixed contextual async return type with unions in context that contain a mix of promise and non-promise types#47683

Closed
Andarist wants to merge 4 commits intomicrosoft:mainfrom
Andarist:contextual-async-return-type-with-unions
Closed

Fixed contextual async return type with unions in context that contain a mix of promise and non-promise types#47683
Andarist wants to merge 4 commits intomicrosoft:mainfrom
Andarist:contextual-async-return-type-with-unions

Conversation

@Andarist
Copy link
Contributor

@Andarist Andarist commented Feb 1, 2022

Please verify that:

  • There is an associated issue in the Backlog milestone (required)
  • Code is up-to-date with the main branch
  • You've successfully run gulp runtests locally
  • There are new or updated unit tests validating the change

This potentially fixes #47682

Commits:

  • first commit just add a test case to showcase the weird behavior
  • second commit fixes the thing in one-line change
  • third commit just refactors the code to avoid fallthrough and to make all paths more distinct and easier to follow (in my opinion), this is a subjective matter and I can revert this if needed

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 1, 2022
Andarist
Andarist commented Feb 1, 2022
View reviewed changes
src/compiler/checker.ts
const contextualAwaitedType = mapType(contextualReturnType, getAwaitedTypeNoAlias);
return contextualAwaitedType && getUnionType([contextualAwaitedType, createPromiseLikeType(contextualAwaitedType)]);
if (functionFlags & FunctionFlags.Async) {
const contextualTypeOfPromise = mapType(contextualReturnType, getAwaitedTypeOfPromise);
Copy link
Contributor Author

@Andarist Andarist Feb 1, 2022

Choose a reason for hiding this comment

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

The main and only change here is that for async non-generator functions I use mapType with getAwaitedTypeOfPromise. This ensures that I only unwrap promise types instead of unwrapping any types with Awaited<T>.

It IMHO conceptually makes sense because if we deal with unions we can only return a Promise type that includes Promise-wrapped types. If a member of a union is of a different type then async method just won't be able to match it as async always returns a Promise.

@Andarist Andarist mentioned this pull request Feb 1, 2022
Closed
@sandersn sandersn added the For Backlog Bug PRs that fix a backlog bug label Feb 11, 2022
@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Backlog Bug PRs that fix a backlog bug For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 11, 2022
@sandersn sandersn added For Backlog Bug PRs that fix a backlog bug and removed For Milestone Bug PRs that fix a bug with a specific milestone labels Feb 11, 2022
@typescript-bot typescript-bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Backlog Bug PRs that fix a backlog bug labels Feb 11, 2022
@Andarist
Copy link
Contributor Author

Andarist commented Apr 4, 2022

@rbuckton friendly 🏓 would love to hear out your thoughts about this PR :)

@Azarattum
Copy link

Azarattum commented May 3, 2022

@Andarist would this fix #48927?

@Andarist
Copy link
Contributor Author

Andarist commented May 3, 2022

@Azarattum I’ve checked and those are unrelated

@Andarist
Copy link
Contributor Author

Andarist commented Jul 3, 2022

@rbuckton friendly 🏓

@Andarist Andarist force-pushed the contextual-async-return-type-with-unions branch from 0ea6851 to f4b2ec7 Compare July 5, 2022 08:17
@Andarist Andarist mentioned this pull request Oct 17, 2022
Merged
@sandersn
Copy link
Member

sandersn commented Oct 27, 2022

since #51196 supersedes this PR, do you want to close it?

@Andarist
Copy link
Contributor Author

Andarist commented Oct 27, 2022

Ye, I think that this other PR is a better one. Let's close this one.

@Andarist Andarist closed this Oct 27, 2022
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rbuckton rbuckton Awaiting requested review from rbuckton

Assignees

@rbuckton rbuckton

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Contextual generic return type of an async function includes union members from the context that it shouldn't

5 participants

@Andarist @Azarattum @sandersn @rbuckton @typescript-bot