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

Types that depend on constants are not narrowing correctly #56417

Open
djsavvy opened this issue Nov 15, 2023 · 10 comments
Open

Types that depend on constants are not narrowing correctly #56417

djsavvy opened this issue Nov 15, 2023 · 10 comments
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@djsavvy
Copy link

djsavvy commented Nov 15, 2023

πŸ”Ž Search Terms

"ternary narrowing", "ternary constant", "constant narrowing", "constant dependent"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried (including nightly), and I reviewed the FAQ.

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.2.2#code/MYewdgzgLgBAQgeQQGRgXhlATgVwKYwCGEMokUAUBVAJ4AOBAKgPqO4EYDeMAHgFwxoWAJZgA5gBoYjAUNFiYAX2r0mzAGKEANhA4xu-QdnlT1s4+KVVaDaehgAKG3hAAzeEmQBKGHh5Q8MAATEmx8GAB+aVZ2GAEWTR08KgpXHDBgKGFwGFcHL3j9ChgS0nBoGAA3QiwZaLZwrl4BAHJq2papOpaxEBAglqtSsvIqmrNoxN17A1b29U6YCZaQAGtCGkHlYtKsPCgcLDAPFEix2rjz9QpFIA

πŸ’» Code

const BOOL = true as const

type T_True = { x: string, T: string }
type T_False = { x: string, F: string }

type T = (typeof BOOL) extends true ? T_True : T_False

function f(): T {
    const varT: T_True = { x: 'varT', T: 'good' }
    const varF: T_False = { x: 'varF', F: 'okay' }

    return BOOL ? varT : varF
}

πŸ™ Actual behavior

Typechecking fails with the error Type 'T_True | T_False' is not assignable to type 'T_True'. Property 'T' is missing in type 'T_False' but required in type 'T_True'. on the return statement.

πŸ™‚ Expected behavior

Typechecking should pass, since the const BOOL = true as const should mark the false ternary case as unreachable.

I also tried making T generic with respect to BOOL, and splitting the ternary operator into an if/else, but the same error persisted regardless.

Additional information about the issue

No response

@fatcerberus
Copy link

Is this a real-world issue? I'd estimate that approximately nobody writes a ternary if they already know at compile time what the result is going to be.

@MartinJohns
Copy link
Contributor

Sounds like a duplicate of #33912 to me.

@djsavvy
Copy link
Author

djsavvy commented Nov 15, 2023

This is in fact a real-world issue (the example code is boiled down from our codebase).

Our use case is that we are switching between two different versions of an API that return data via slightly different property paths. The objects returned by the API are used pretty deep in the codebase, and most of the property accesses will stay the same but a few will change.

The BOOL in this case is used as a feature flag, and we'd like to switch between the true and false values rapidly during testing over the next few months. If we can get typechecks to pass with both values enabled, we'll be much more confident in the API transition.

@djsavvy
Copy link
Author

djsavvy commented Nov 15, 2023

Sounds like a duplicate of #33912 to me.

IMO that issue seems vastly more general than this one, and also it seems to be stuck in an unresolved (unresolvable?) debate about the correctness of that generalized implementation.

However, I will defer to the Typescript maintainers, since y'all definitely know the codebase the best.

@fatcerberus
Copy link

Yeah this isn’t #33912 at all - that’s about unresolved (i.e. generic) conditional types. The conditional type here is fully resolved, TS just doesn’t realize the ternary expression is always true.

@MartinJohns
Copy link
Contributor

You're right, the general use case is more like #3538. The expectation regarding the ternary operator is probably #26914, the "never true" part could narrow to never.

For now your best option is to use a union type.

@fatcerberus
Copy link

fatcerberus commented Nov 16, 2023

I should note that, today, if you write

let foo;
if (true)
    foo = 42;
else
    foo = "fooey";

Then foo gets correctly typed as number, but you get an unreachable code error in the else branch (which can be @ts-ignore'd if you so choose). It seems likely to me that any change made here would involve adding a similar error for the unreachable half of the ternary expression. So maybe not ideal for your use case.

@gabritto
Copy link
Member

Note for implementers: as pointed out in #56417 (comment), if instead of a ternary you write this as an if (true)-else, you get an unreachable code error. You don't get the same behavior in ternaries because, first of all, we don't create control flow graph nodes for the ternary branches as we do for if-elses.
And furthermore we also don't detect unreachable code if you have if (x) where x has type true, we only do that if what's written is literally if (true).

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Nov 29, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Nov 29, 2023
@RyanCavanaugh
Copy link
Member

I'm not sure if this is going to be well-received if we "fix" it. The counterpart to this is people who have something like

// Gets regex'd to 'false' in release builds
const DEBUG = true;

const foo: string = DEBUG ? something : something_else

This introduces a danger if something_else is number -- we'll say "Ah, that never happens, it's safe to ignore" and then the program becomes wrong under release builds.

@djsavvy
Copy link
Author

djsavvy commented Nov 30, 2023

@RyanCavanaugh interesting point. Could we have the original proposed behavior only if DEBUG in your example is defined as const DEBUG = true as const?

I defer to your expertise, but my understanding is that defining a constant with as const makes Typescript treat it as narrowly as possible but dropping the as const makes Typescript treat it as the full possible type. So in your example code, defining DEBUG without the as const would internally type it as a boolean, which means that both branches could be taken and so neither path is safe to ignore. But with the as const, DEBUG would be typed as true directly, meaning that the false branch would be safe to "ignore". Then, my use case could be supported by defining with as const, while the existing behavior could be supported by defining without.

Also, note: "ignoring" the un-taken branch is not a necessary condition for my original code example to work. We don't need to ignore the universe where BOOL is false, we just need to not assume that BOOL is ever true and false at the same time. The reason my original code fails typechecking is that the compiler thinks we could return varF when BOOL is true, and vice versa.

@djsavvy djsavvy changed the title Types dependent on constants not narrowing correctly Types that depend on constants are not narrowing correctly Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

5 participants