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

Inconsistent type resolution when resolving a Promise in a generic context #46543

Closed
ysfaran opened this issue Oct 27, 2021 · 2 comments · Fixed by #53068
Closed

Inconsistent type resolution when resolving a Promise in a generic context #46543

ysfaran opened this issue Oct 27, 2021 · 2 comments · Fixed by #53068
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@ysfaran
Copy link

ysfaran commented Oct 27, 2021

Bug Report

🔎 Search Terms

promise generic return type resolution wrapper await resolve

🕗 Version & Regression Information

  • This is a type resolution issue (i guess)
  • I tested it on all Playground versions (3.3.3 - 4.5.0-beta and Nightly)
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about inference, generic, promise, return type

⏯ Playground Link

Playground link with relevant code

💻 Code

// library land (relevant and simplified part from external library)
type SelectAndInclude = {
  select: any;
  include: any;
};
type HasSelect = {
  select: any;
};
type HasInclude = {
  include: any;
};

type CheckSelect<T, S, U> = T extends SelectAndInclude
  ? "Please either choose `select` or `include`"
  : T extends HasSelect
  ? U
  : T extends HasInclude
  ? U
  : S;
  
declare function findMany<T extends {select?: string, include?: string}>(args: T): CheckSelect<T, Promise<1>, Promise<2>>;

// user land
function wrapperWorking<T extends {select?: string, include?: string}>(args: T){
  return findMany(args);
}

async function wrapperNotWorking<T extends {select?: string, include?: string}>(args: T){
  const result = await findMany(args)
  return result;
}

async function main() {
  const isErrorText = await wrapperWorking({select:"foo",include: "bar"})
  const is1 = await wrapperWorking({})
  const is2 = await wrapperWorking({select: "foo"})
  const is2Too = await wrapperWorking({include: "bar"})

  const shouldBeErrorTextButIs1 = await wrapperNotWorking({select:"foo",include: "bar"})
  const is1Too = await wrapperNotWorking({})
  const shouldBe2ButIs1 = await wrapperNotWorking({select: "foo"})
  const shouldBe2TButIs1Too = await wrapperNotWorking({include: "bar"})
}

🙁 Actual behavior

I use a library which exports generic functions in shapes like findMany (see code sample). Generally these functions accept one argument and map to some return type depending on the arguments type. This works fine and is no issue at all.

My aim was to add a wrapper arround such a generic function to do some extra work before or after calling it e.g.:

async function wrapFindMany<T extends {select?: string, include?: string}>(args: T){
  await doSomethingBefore(args); // (e.g. async validation, sanitizing)
  const result = await findMany(args);
  await doSomethingAfterwards(result); // (e.g. writing to a DB)
  return result;
}

When you look at the code sample you can see two wrappers: wrapperWorking and wrapperNotWorking. The only difference between them is that wrapperNotWorking is an async function that awaits the result of findMany first before passing it to the return.

Although these functions are essentially the same the typings behave differently. To be precise the unexpected behaviour is in this line:

const result = await findMany(args)

result resolves immediatly to the value 1, but args is generic and it should not resolve to a definite value here. For this reason wrapperNotWorking is always returning Promise<1>, while wrapperWorking returns the correct type. (see main for examples and differences)

🙂 Expected behavior

The type of result should be:

"Please either choose `select` or `include`" | 1 | 2

and wrapperNotWorking should return Promise's accordingly.
Otherwise it's not conistent and confusing.

@jcalz
Copy link
Contributor

jcalz commented Oct 28, 2021

If #35284 were merged then result might become type Awaited<CheckSelect<T, Promise<1>, Promise<2>>>, although then return result will probably end up eagerly resolving to Promise<1> again instead of the correct-but-nightmarish Promise<Awaited<CheckSelect<T, Promise<1>, Promise<2>>> (at least if this Playground Link is any indication). Not sure if that would be fixable with something else.

@ysfaran
Copy link
Author

ysfaran commented Oct 29, 2021

Interesting that it still resolves to Promise<1> even after the type assertion here:

const result = (await findMany(args)) as Awaited<CheckSelect<T, Promise<1>, Promise<2>>>

For me that's still a bug then. The types shouldn't behave differently.

@andrewbranch andrewbranch added the Needs Investigation This issue needs a team member to investigate its status. label Nov 1, 2021
@andrewbranch andrewbranch added this to the TypeScript 4.6.0 milestone Nov 1, 2021
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label May 13, 2022
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants