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

Type guard affects type of variable in surprising way #50916

Closed
MichaelMitchell-at opened this issue Sep 23, 2022 · 8 comments Β· Fixed by #52282
Closed

Type guard affects type of variable in surprising way #50916

MichaelMitchell-at opened this issue Sep 23, 2022 · 8 comments Β· Fixed by #52282
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@MichaelMitchell-at
Copy link

Bug Report

πŸ”Ž Search Terms

type guard / fall through / narrow / change / lazy / evaluate

πŸ•— Version & Regression Information

  • This changed between versions v4.7.4 and v4.8.2

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type Identity<T> = {[K in keyof T]: T[K]};

type Self<T> = T extends unknown ? Identity<T> : never;

function is<T>(value: T): value is Self<T> {
    return true;
}

type Union =  {a: number} | {b: number} | {c: number};

function example(x: Union) {
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}
    if (is(x)) {}

    return x;
    //     ^?
}

πŸ™ Actual behavior

The type of x is "narrowed" to Identity<Identity<Identity<Identity<Identity<Identity<Identity<Identity<{a: number;}>>>>>>>> | Identity<Identity<Identity<Identity<Identity<Identity<Identity<Identity<{b: number;}>>>>>>>> | Identity<Identity<Identity<Identity<Identity<Identity<Identity<Identity<{c: number;}>>>>>>>>

It's as if a variable gets narrowed to the union of the types of both sides of the type predicate, e.g.

if (isA(aOrB)) {
  // `aOrB` gets narrowed to `A`
} else {
  // `aOrB` gets narrowed to `Exclude<typeof aOrB, A>`
}

// `aOrB` gets narrowed to `A | Exclude<typeof aOrB, A>` but it should just be left alone

πŸ™‚ Expected behavior

The type of x doesn't change.

@ahejlsberg
Copy link
Member

This is an effect of #50044. The issue here is that the argument type and the asserted type are subtypes of each other, and therefore appear interchangeable in control flow analysis. From the PR:

Note that one issue with favoring the asserted type is that CFA continues with that type after the conditional block. Though undesired, that's an effect of how CFA works in the face of mutual subtypes. This behavior was already present for singleton types, but now also extends to union types.

So this is effectively a design limitation, but we'll continue to think of ways in which to improve it.

@ahejlsberg ahejlsberg self-assigned this Sep 23, 2022
@ahejlsberg ahejlsberg added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Sep 23, 2022
@MichaelMitchell-at
Copy link
Author

Would it be possible to have TS more eagerly resolve Identity<Identity<Identity<Identity<Identity<... down to Union? A problem that we encountered is that given a more complex type and enough if blocks, the narrowed type becomes complex enough that it both slows down tsserver to a crawl and we start getting "type instantiation is excessively deep" errors.

@ahejlsberg
Copy link
Member

ahejlsberg commented Sep 23, 2022

The core issue is that Union and Identity<Union> are both subtypes of each other. If you change Identity<T> to always be a subtype of T, for example by intersecting with a tag type, things work as expected in your scenario:

type Identity<T> = {[K in keyof T]: T[K]} & { __tag__: void };

Fundamentally, when the argument and asserted types are subtypes of each other, the reason we favor the asserted type is that you must have written the assertion for some reason. I'm not sure what the reason is in your example, but presumably there is some difference in behavior?

@MichaelMitchell-at
Copy link
Author

MichaelMitchell-at commented Sep 23, 2022

Here's an example that is a lot closer (though still simplified) to the actual situation that we had:
Playground Link

@ahejlsberg
Copy link
Member

I that last example, I'd recommend re-writing the Require<T, K> type to only narrow when necessary. This way the returned type is either T or a subtype of T, and therefore types property revert back as expected.

export type Require<T, K extends keyof T> =
    Pick<T, K> extends Required<Pick<T, K>> ? T : Omit<T, K> & Required<Pick<T, K>>;

@MichaelMitchell-at
Copy link
Author

I that last example, I'd recommend re-writing the Require<T, K> type to only narrow when necessary. This way the returned type is either T or a subtype of T, and therefore types property revert back as expected.

export type Require<T, K extends keyof T> =
    Pick<T, K> extends Required<Pick<T, K>> ? T : Omit<T, K> & Required<Pick<T, K>>;

That's what we pretty much ended up doing (the playground link in the description actually has an example at the bottom of the code) and we added this optimization to our other utility types, but it was certainly an unexpected footgun as this was the first time we've ever had to optimize our types in this particular way.

@ahejlsberg
Copy link
Member

Agreed about the footgun. We'll continue to think about ways to improve this.

@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Feb 6, 2023
@ahejlsberg ahejlsberg added this to the TypeScript 5.0.1 milestone Feb 6, 2023
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 14, 2023
@MichaelMitchell-at
Copy link
Author

Thanks @ahejlsberg !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants