Skip to content

"Condition will always return false": declared type not being used? #42702

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

Closed
5 tasks done
cakoose opened this issue Feb 9, 2021 · 7 comments
Closed
5 tasks done

"Condition will always return false": declared type not being used? #42702

cakoose opened this issue Feb 9, 2021 · 7 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@cakoose
Copy link

cakoose commented Feb 9, 2021

Suggestion

πŸ” Search Terms

union condition always return false inferred

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code [I think it would only make more code accepted, but I'm not sure.]
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Playground link

const x: 'a' | 'b' = 'a';
if (x === 'b') { // TS2367: This condition will always return 'false' since the types '"a"' and '"b"' have no overlap.
    console.log('b');
}

Since I've declared x to have type 'a' | 'b', I was surprised that TypeScript didn't respect my declared type. But maybe TS does this a lot and I just haven't noticed?

I think this is a useful error to report, but it feels like something you only want when you're ready to commit, like the "unused variable" warning. When I'm debugging/experimenting and moving a code around, I don't want this to prevent compilation.

πŸ“ƒ Motivating Example

See above.

πŸ’» Use Cases

See above.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 9, 2021
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 9, 2021

So let's say you wrote something like this

declare function getCarOrPlaneOrBoat(): Car | Boat | Plane;
type CoolThing = Car | Boat | Plane | undefined;

const p: CoolThing = getCarOrPlaneOrBoat();
p.moveFast();
// ^ OK?

Should this line error because p might be undefined? No, of course not -- we know it's not undefined, because you just initialized it with something that's not undefined.

The fact that that isn't an error is the exact reason that the OP is an error -- TS sees what you initialized the const with, and narrows appropriately.

@RyanCavanaugh
Copy link
Member

You can write const x = "a" as "a" | "b" to get different behavior

@cakoose
Copy link
Author

cakoose commented Feb 9, 2021

Should this line error because p might be undefined? No, of course not -- we know it's not undefined, because you just initialized it with something that's not undefined.

Couldn't the same thing be said for the as operator?

const p = "hello" as string | undefined;
console.log(p.length);

Why is one case an "of course not"?

And TS doesn't narrow in other cases:

const x: string = "a";
if (x === "b") { // no error
    console.log("b");
}

const y: unknown = 1;
if (y > 0) { // error
    console.log('positive')
}

Even though those seems similar to this:

function g(x: string, y: unknown) {
    if (x === "a") {
        if (x === "b") { // error
            console.log("b");
        }
    }

    if (y === 1) {
        if (y > 0) { // no error
            console.log('2');
        }
    }
}

I don't think narrowing "through" a declared type is fundamentally wrong. I'm just wondering if it's more useful to not do that.

For one, the behavior was surprising to me and my coworkers. Maybe it's because the : T syntax everywhere else is relatively "strong", or maybe it's because narrowing doesn't happen in other similar cases. (We initially thought it was a bug with how unions worked.)

Also, I think it's common to use type declarations as a convenient "firewall" when messing around with code. For example, when debugging something, you might replace an expression with a constant, assuming that the type declaration keeps things modular. This is especially true when trying to figure out how to get code to type-check.

I could retrain myself to use as for this, but I'd need to be more vigilant since it's less sound. I could create a cast helper function (e.g. <T>(v: T): T => v) but that's more verbose.

@whzx5byb
Copy link

whzx5byb commented Feb 9, 2021

And TS doesn't narrow in other cases:

const x: string = "a";
if (x === "b") { // no error
    console.log("b");
}

const y: unknown = 1;
if (y > 0) { // error
    console.log('positive')
}

Non-union types such as string and unknown are not narrowed on assignment. See #27706

@cakoose
Copy link
Author

cakoose commented Feb 9, 2021

Yeah, good point. I think the lack of narrowing contributed to the confusion I had, but it's sort of orthogonal to the core issue.

The example in that issue also reminded me that I should distinguish between const and let. Though I think narrowing beyond than the declared type is a net negative for const initialization, the pros/cons might come out differently for let assignments.

@jgbpercy
Copy link

jgbpercy commented Feb 9, 2021

Couldn't the same thing be said for the as operator?

I think a good mental model for as is "Hey TypeScript, I know better than you! In your view this might be be unsound (or otherwise cover up things that you think are errors), but take my word for it". So when you use as, you're overruling what TypeScript knows about your code, and in this case widening a type in order to hide an issue that TypeScript can otherwise statically guarantee is an error.

This isn't the case with a simple type annotation on a declaration - there TypeScript is free to use what it knows statically about your code in order to narrow the type.

I feel pretty strongly (and I think the TS team do as well) that the default behaviour from an annotation is generally the most useful, and as is there as an escape hatch if you really want the behaviour you're describing.

@RyanCavanaugh
Copy link
Member

Initialization used to not act like narrowing and people complained because of the cases like the one I demonstrated. Moreover, it's very confusing if

let x: T = e;

behaves differently from

let x: T;
x = e;

Ultimately the code in the repro is indistinguishable from a logic error, there are many cases where you do want narrowing on initialization, and it's more consistent with assignment. It's a hard sell to make this less precise on purpose. We thought about this behavior before implementing it and are reasonably certain that this is the best available behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants