Skip to content

Possible Regression in v4.9: Conditionals using Unlisted Property Narrowing with the in Operator  #52812

Closed
@mattkrick

Description

@mattkrick

Bug Report

πŸ”Ž Search Terms

v4.9 conditionals unlisted property narrowing in operator #50666

πŸ•— Version & Regression Information

v4.9+, including nightly
working in v4.8.4

  • This changed between versions 4.8.4 and 4.9

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type HandledResult = {data: any, errors?: any}
type UnhandledResult = {message: string} 
const result = null as any as HandledResult | UnhandledResult

const noWork = (): HandledResult => {    
    if ('data' in result || 'errors' in result) return result // when checking an optional property second, I get an error 
    return {data: null, errors: [result.message]}
}

const work = (): HandledResult => {
    if ('errors' in result || 'data' in result ) return result // reversing the order fixes the problem
    return {data: null, errors: [result.message]}
}

πŸ™ Actual behavior

In the noWork function, typescript could not narrow down the type to being HandledResult. When the or clause is switched, it works. Since Boolean(A || B) === Boolean(B || A) this seems like an error.

πŸ™‚ Expected behavior

The ordering of the conditionals should not impact the narrowing of the type.

Activity

RyanCavanaugh

RyanCavanaugh commented on Feb 17, 2023

@RyanCavanaugh
Member

This code is confusing to reason about because it is written in a way that presupposes an invariant violation occurring somewhere along the way.

Our logic effectively thinks of it this way:

In "noWork", it goes like this:

  • If "data" in result, then it's clearly a HandledResult -> OK
  • If not "data" in result, then it's clearly not a HandledResult, since that's a mandatory property, so it must be an UnhandledResult
  • If errors is present, then it's must be a UnhandledResult & { errors: unknown }; that's the only thing it can be. That's not a valid return type, so the program is correctly rejected

In "work", it goes like this:

  • If "errors" in result then it's clearly a HandledResult, since that's one of its listed optional properties
  • If "data" in result then it's clearly a HandledResult, since that's one of its listed required properties
  • Either way this code is fine

The functions don't make any sense if you assume the types are closed, which is the only operative condition in which using in makes any sense in the first place. I don't really see a way to change this without breaking more reasonable code that would expect not "data" in result to produce an UnhandledResult

fatcerberus

fatcerberus commented on Feb 18, 2023

@fatcerberus

In "work", it goes like this: ...

It's interesting to me that in the "working" case, TS never considers the possibility that "data" in result might be false, like it does in the "non-working" case. It seems like it should, since || is commutative (short-circuiting of side effects notwithstanding).

mattkrick

mattkrick commented on Feb 21, 2023

@mattkrick
Author

thank you so much for your thoughtful explanation. πŸ™
it took a few re-reads, but it eventually makes sense why it works the way it does! I was treating them like open types where {data?: any, errors: any} could possibly exist & it prevented that.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Not a DefectThis behavior is one of several equally-correct options

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @fatcerberus@mattkrick@RyanCavanaugh

        Issue actions

          Possible Regression in v4.9: Conditionals using Unlisted Property Narrowing with the in Operator Β· Issue #52812 Β· microsoft/TypeScript