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 narrowing with union types create impossible case with else statement #58541

Closed
spsquared opened this issue May 15, 2024 · 7 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@spsquared
Copy link

spsquared commented May 15, 2024

🔎 Search Terms

else
type narrowing

🕗 Version & Regression Information

  • This is the behavior in every version available on the typescript playground, and the FAQ doesn't seem to mention it
  • EDIT: it only seems to happen when there are more type unions within the type union

⏯ Playground Link

EDIT: this is a bad example
https://www.typescriptlang.org/play/?ts=5.4.5#code/C4TwDgpgBAQgrgcygXigbwFBW1AhgLigHIAeZIgGixwCNCA7OAWxogCcMBfKAH3WuwFiAPnJUcUAMYNmrDpwwYA9EqgATAPYQAzvSLAoAdw1sA1gH4MkjfW0HgO4IXhJUmCUNJiBUOlABMXBgAlgBmUAAUDnYAdLgoqF5EAJT8Eiq+UBAAHsF22nj0alJSuPT0GsA+1rYaADYQMXUaCFGOMTTJ1Tba9Y3NrdHAMZJd3BB12tDuOBnB9FC9TNCSuFMFwAAW7NB5eFAARNZMYLhseTZ4YJBnBXD08w70wME2uHUHWWxsJlBhUABJKDANggYEaKQaE5naBDOI+DL0CDBLbsEq-GhZXJ2KDbNjQCAANwgCy2GkQm32TDgONYInIhWKqPxoRMKyg1JxOTyVQkNV6DSaLTasU63VqgoGIuGoyCylUNDg9k2e2MZm0GFC90kL0uQ38UWciFSM2w-yicQSxDIKTSEhw-L6QsGHS6EnGk2mPj5PSdUplbpwCk4QA

💻 Code

EDIT: still a bad example, assume test is not known

type Bug = {
    a: '<=',
    b: number
} | {
    a: '>=',
    c: number
}

// doesn't work?
const test: Bug = {
    a: '<=',
    b: 2
}
if (test.a == '<=') {
    // b exists and c cannot
    console.log(test.b)
    console.log(test.c)
} else {
    // in some cases there is a "comparison appears unintentional" error if I try to compare test.a
    // neither c or b exist here even though a must be '>=' and therefore c must exist
    console.log(test.b)
    console.log(test.c)
}

// but this works
function test2(t: Bug) {
    if (t.a == '<=') {
        console.log(t.b)
    } else {
        console.log(t.c)
    }
}

🙁 Actual behavior

else statement does not type narrow properly in some cases.

In my code it's impossible to get around this as the type narrowing DOES work properly in further if statements, which prevents me from simply checking with the same comparison again with a return statement.

image

The example given uses the value of a to determine whether to use b or c, but the narrowing doesn't allow any code in the else to use either.

🙂 Expected behavior

The type narrowing checks for the if condition inside the else block should also apply to other code in the else block.

Additional information about the issue

@fatcerberus
Copy link

Not a bug. Types are narrowed on assignment, so TS knows it's impossible for that else to execute and test gets narrowed to never. Hence the error message

Property 'c' does not exist on type 'never'.

@spsquared
Copy link
Author

Oh, I've provided a bad example here. It still happens when the value is not known (in a function) for me in some places

It seems to happen when there are nested type unions?

https://www.typescriptlang.org/play/?#code/C4TwDgpgBAKgFgSwHYHMA8MB8UC8UDeAUFCVAPZgBcUA5AIY1QA+tARjcaQG7UyEC+zApxIVqNAMaMWNACYdSULgEZeIpQCY1gljCEwA2gF0A3IUIAzAK5IJwBGSRRgEAM7AAFBLIAbKwFskV2p8JTo-CF5EVDQkANYIACchd0TkFGwWG1kIC2QIWSE4nx8ofmMASmouMgRCokULMmSvR3cCMIiy8gsobz9A1wrhRUUEXo8ucKtoZHc6WwgyXoBBRMS6EGGG0d2Aej3nRFcoBBO8pAh1RUEIH1dZidBIZc6Z3DwaMlYAKwg7RgAMkBb2gAEI8MUfNtrrtThMphEAHQUD60KTMFiImYosBouQ0GFw4lQA5HM6nc75WEk0G4mkk7EQJEqBnEpksjRs0i3e7QHa0xRk4AnADWSDIAHcThzURIFuQkD4QFAErQGEIaOxubtxlBJtNmaicJ8MUwsYbcfj5MMmTrRmTWFZgEpKVB3AgSlAJS6chcCvbFByuPb+AzeQ94fqmVAId6rCUiYKyal0kV4kl7XbiWGbgIgA

type Thing<T> = {
    op: 'a' | 'b'
    v: T
} | {
    op: 'c' | 'd'
    v1: T
    v2: T
} | T | T[];

function test(columns: { value: Thing<number | string> | undefined | null }[]): void {
    for (const { value } of columns) {
        if (value instanceof Array) {
            // this is fine
        } else if (typeof value == 'object' && value != null) {
            if (value.op == 'c' || value.op == 'd') {
                // this is fine
                value.op
                value.v1
                value.v2
            } else {
                // ts knows value.op can only be 'a' | 'b'
                if (value.op == 'c' || value.op == 'd') value
                // but v is still not defined
                value.v
            }
        } else if (value != null) {
            // string | number
            value
        }
    }
}

@fatcerberus
Copy link

fatcerberus commented May 15, 2024

That's a much more instructive example, thanks. I'd recommend replacing the example in the OP with it since it perfectly illustrates the actual problem you're facing.

It seems like the addition of extra union members causes TS to no longer treat Thing as a discriminated union: from the example it's easy to see that TS narrows value.op to "a" | "b" (hence the first error) but not value itself (hence the second).

edit: That second paragraph is probably wrong. See Ryan's reply below.

@RyanCavanaugh
Copy link
Member

The simplest form looks like this:

type Thing<T> =
    | { op: 'a', a: T }
    | { op: 'c' | 'd', cd: T };

function test(value: Thing<number | string>): void {
    if (value.op === 'c' || value.op === 'd') {
        // this is fine
        value.op
        value.cd
    } else {
        // ts knows value.op can only be 'a'
        value.op
        //    ^?
        // but no a property
        value.a
    }
}

This is a duplicate of #31404 but I'll ping some folks to take another bite at the apple; maybe it's more tractable these days

@fatcerberus
Copy link

fatcerberus commented May 15, 2024

Ah, so it's just the union-typed discriminant that's throwing it off? I figured it was because value was initially typed as <discriminated union> | T | T[] | null | undefined that it wasn't triggering the discriminated union checks.

@RyanCavanaugh
Copy link
Member

The TL;DR is that from the control flow analyzer's perspective, the code looks like this

type Thing<T> =
    | { op: 'a', a: T }
    | { op: 'c' | 'd', cd: T };

function test(value: Thing<number | string>): void {
    if (value.op === 'c') {
        // this is fine
        value.op
        value.cd
    } else if (value.op === 'd') {
        // this is fine
        value.op
        value.cd
    } else {
        // ts knows value.op can only be 'a'
        value.op
        //    ^?
        // but no a property
        value.a
    }
}

By exclusion, value.op went from the total possible union 'a' | 'c' | 'd' to a, but the object union itself couldn't undergo any narrowing because there's never a distinct point at which we can remove { op: 'c' | 'd', cd: T } from the list of possible inhabitants of value

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jun 6, 2024
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants