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

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

Closed
Andarist opened this issue Feb 1, 2022 · 2 comments Β· Fixed by #51196
Assignees
Labels
Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Milestone

Comments

@Andarist
Copy link
Contributor

Andarist commented Feb 1, 2022

Bug Report

πŸ”Ž Search Terms

contextual typing, generic, async, return type, union

πŸ•— Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ

⏯ Playground Link

TS playground (latest)
TS playground (nightly)

πŸ’» Code

declare class StateMachine<T> {
  onDone: (a: T) => void;
}

declare function createMachine<T>(implementations: {
  services: Record<string, () => Promise<T> | StateMachine<T>>;
}): void;

// this is close to my original issue found in the real-world in XState
createMachine<{ count: number }>({
  services: {
    test: async () => Promise.reject("some err"),
    async test2() {
      return Promise.reject("some err");
    },
  },
});

// this is an additional test case I've figured out that behaves in a similar way.
function fn1(): () => Promise<{ count: number }> | StateMachine<{ count: number }> {
  return async () => Promise.reject('some err')
}

πŸ™ Actual behavior

Both tests fail to compile and the reported error looks bizzare(-ish):

Type '() => Promise<{ count: number; } | StateMachine<{ count: number; }>>' is not assignable to type '() => Promise<{ count: number; }> | StateMachine<{ count: number; }>'.
  Type 'Promise<{ count: number; } | StateMachine<{ count: number; }>>' is not assignable to type 'Promise<{ count: number; }> | StateMachine<{ count: number; }>'.
    Type 'Promise<{ count: number; } | StateMachine<{ count: number; }>>' is not assignable to type 'Promise<{ count: number; }>'

For some reason, the return type here didn't narrow the contextual union to include only Promise(-like?) members and dragged the whole Awaited<Union> to the computed contextual type. And this has caused an assignability problem.

Note that this "only" happens with Promise.reject, it's not reproducible with Promise.resolve and other similar types. My guess is that it's because Promise.reject contains a generic only at the return type position and this is what makes this issue manifest somehow.

πŸ™‚ Expected behavior

No error should be reported because this is valid code and it looks like sufficient information has been provided in the context for this to be narrowed down properly~.


While looking into this problem I've poked around the internals of TS and figured out that this might be a potential fix for this issue: #47683

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 1, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Feb 1, 2022
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 11, 2022
@JoostK
Copy link
Contributor

JoostK commented Jul 4, 2022

This is also causing errors in Angular codebases as reported in angular/angular#46697.

Minimum example:

declare function load(): Promise<boolean>;

type LoadCallback = () => Promise<boolean> | string;

const cb1: LoadCallback = async () => load().then(m => m); // errors
const cb2: LoadCallback = async () => load();              // works
const cb3: LoadCallback = () => load().then(m => m);       // works

Playground link (nightly)

@Andarist
Copy link
Contributor Author

Andarist commented Jul 5, 2022

@JoostK I've confirmed that my PR fixes your case too and pushed out your minimal repro as an additional test case there: ce8c012

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.
Projects
None yet
5 participants