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

Generic type narrowing for truthy check has inconsistent / incorrect behavior #59717

Open
brandonryan opened this issue Aug 22, 2024 · 6 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@brandonryan
Copy link

brandonryan commented Aug 22, 2024

πŸ”Ž Search Terms

generic truthy narrowing generic index

πŸ•— Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about narrowing truthy generic types

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.7.0-dev.20240821&ssl=29&ssc=2&pln=1&pc=1#code/MYewdgzgLgBATgU1HAJgLhgJSSVAeaOASzAHMAaGAVzAGswQB3MAPhgF4YBvAXwG4AsAChhAMxrAoRcDFEgQeACowEADygIwKCDEIlSMAD6yAhgBsICIzDBUzZlgAoAbuYyKAlN2ExfMIqIwLuZeXD5+Ea5mgkIREQD08QB6APzhcTAARiZwwWYeMRm+iMgoANpRALocMACshX48KhZWYbFFUelxialdPML9IkLiYJLSYLLyAEx5GHpk1qLmlta29qFdAUFRG+0ZUQ0ZPWl7cdm5O4dxJbjlVTX1fc0rbUUwnacJyScRg4NiEikMnOs10UGIZF2EUQUCocAmBwGQA

πŸ’» Code

const record: Record<string, unknown> = {};

function foo<T extends string | false | null>(val: T) {
    if (val) {
        val;
        //^?
        bar(val);
        record[val] = 5;
    } else {
        val
        //^?
    }
}

function foo2(val: string | false | null) {
    if (val) {
        val;
        //^?
        bar(val);
        record[val] = 5;
    } else {
        val
        //^?
    }
}

function bar(val: string) {
    return val;
}

πŸ™ Actual behavior

Error on line 8, cannot use val to index object type with string key.
Val appears to be narrowed to NonNullable<T> yet I can use it as string when calling functions?
Note: foo2 does not use generics and has more expected bahavior.

πŸ™‚ Expected behavior

type Falsy = false | null | undefined | 0 | "";
(might remove 0 | "" from this because that gets into negated types... false seems like a common enough one to matter though)

On line 5 inside of the if check, I expect val to be narrowed to Exclude<T, Falsy> but instead, is incorrectly (imo) narrowed to NonNullable<T> which does not exclude false.

What is exceptionally strange, is that the type narrowing behavior seems to differ depending on how val is used. I can pass it to the function that takes a string parameter, but when I try to use it to index an object on line 8, I get an error.

Additional information about the issue

This issue was discussed in detail by others more experienced than me in this typescript discord thread:
https://discord.com/channels/508357248330760243/1275942751724372110

Invite to server if you aren't a member:
https://discord.com/invite/typescript

@Andarist
Copy link
Contributor

Relates to #15576 and #43183 , only expressions in the element access expressions are subject to constraint-based narrowing (you'd like to narrow argument expressions here). We can find this comment in the current implementation of isConstraintPosition:

        // In an element access obj[x], we consider obj to be in a constraint position, except when obj is of
        // a generic type without a nullable constraint and x is a generic type. This is because when both obj
        // and x are of generic types T and K, we want the resulting type to be T[K].

@brandonryan
Copy link
Author

@Andarist not sure if you are meaning this is intended behavior, or a bug.
Is there anything else I need to do to prevent this from getting buried in the 5K+ other issues?

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Aug 30, 2024
@Lordfirespeed

This comment has been minimized.

@brandonryan

This comment was marked as off-topic.

@Lordfirespeed

This comment has been minimized.

@brandonryan
Copy link
Author

brandonryan commented Sep 10, 2024

@Lordfirespeed
Think this is what youre looking for
#50652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

5 participants