Typescript: Can't compare property value other than 2, because of previously developed "if != 2 then error" code

[ysmoradi]

Can't compare property value other than 2, because of previously developed "if != 2 then error" code

        if (vm.model.Id != '2')
            throw new Error("...");
        // some codes ...
        if (vm.model.Id != '1')
            throw new Error("...");

vm: { model : { Id : string; } }

"compilerOptions": {
    "allowUnreachableCode": true,
    "declaration": true,
    "emitDecoratorMetadata": false,
    "experimentalDecorators": true,
    "module": "none",
    "outFile": "test.js",
    "target": "es5",
    "sourceMap": true,
    "lib": [ "dom", "es2015" ]
  }

TypeScript 2.1.4 | Visual Studio 2015 Update 3

Actual behavior:
Error: Operator '!=' cannot be applied to types '"2"' and '"1"'.

Is there any config to disable this behavior? I know why this is happening, but I'm not interested in this type of flow checking in typescript.

[fedoranimus]

I do not understand why this is happening. I'm seeing the same issue and feel that it is demonstrably incorrect.

Edit: I could understand if we were testing if Id _is equal_ to a particular string; the inference could make sense. But when testing if Id _is not equal_, why would we assume that it would be that particular string we don't want it to be?

[markboyall]

You know that it must be that thing, because if it wasn't that thing, you threw an error. Since you didn't leave the scope by throwing, it must be that thing.

The compiler is quite right to error, assuming that vm and vm.model are immutable. If you started mutating them, then it's wrong..

[JoyalToTheWorld]

What is the type for vm.model.Id? If it's string then this is definitely a bug. The compiler should not coerce the property type based on a conditional check, that would be absurd.

[markboyall]

It's not only not absurd, it's completely correct if it's immutable.

[RyanCavanaugh]

We assume fields to be not changing "behind the scenes". If they actually are, you'll need to add a type assertion in the appropriate place to indicate otherwise.

[JoyalToTheWorld]

We assume fields to be not changing "behind the scenes". If they actually are, you'll need to add a type assertion in the appropriate place to indicate otherwise.

Why? Since when are javascript objects assumed to be immutable? whats the point of const then?

What "type assertion" are you referring to?

[RyanCavanaugh]

const and immutability are completely different concepts. One does not imply the other except in the case of primitives.

If we didn't assume fields to be immutable, we'd have to error on code like this:

let x: { y: number | string } = ...;
if (typeof x.y === 'string') {
  console.log("Hello world");
  console.log(x.y.substr(2)); // ERROR, x.y might be a number
}

[JoyalToTheWorld]

It's not only not absurd, it's completely correct if it's immutable.

If you assume its immutable, then why would a conditional check change the associated type???

[ysmoradi]

@RyanCavanaugh @markboyall Do you see // some codes comment?
Compiler can not and should not be responsible for such things! Let's imaging that Id is a property that returns random string on its get. Compiler can detect this?
The real codes is here:

https://github.com/bit-foundation/bit-framework/blob/master/Foundation/HtmlClient/Foundation.Test.HtmlClient/Implementations/Tests/radTests.ts

Line 58.
Before the second if, I'm, calling await uiAutomation.formViewModel.setCurrent();
I promise you that vm.model.TestModel.Id will be '1' after that.
For now, I should write vm.model.TestModel.Id.ToString() instead of vm.model.TestModel.Id
I can't believe you're saying this is working as intended. Is there any compiler on the planet other than typescript's one doing this?

[fedoranimus]

const and immutability are completely different concepts. One does not imply the other except in the case of primitives.

string is a primitive, is it not?

Take the following code, the comments are what intellisense tells me:

let name: string;

if(name != "Fred" // typeof name == "string"
    || name == "Steve") { // typeof name == "Fred"
 ...
}

Why would the compiler assume that name has to be the string literal _Fred_ in the second comparator? The compiler has no basis to say that name is _Fred_ in my mind.

[RyanCavanaugh]

Why would the compiler assume that name has to be the string literal Fred in the second comparator?

I'm not even sure how to respond to this. It "assumes" it because it is known to be true.

Why would you not want the type system to warn you about an expression that can be guaranteed to always be false? Why are you writing code, on purpose, that evaluates a conditional whose result you already know by simple inspection?

There is literally no other way to reach the name == "Steve" expression than by name being "Fred". The right operand of || is only evaluated when the left side is falsy, and the only way the expression name != "Fred" is falsy is if name === "Fred". Why are you comparing against "Steve" only in the case where you know the name to be "Fred" ?

This is like writing code like if ((x === 1) && (x === 2)) {. It simply doesn't make any sense at all.

[fedoranimus]

There is literally no other way to reach the name == "Steve" expression than by name being "Fred". The right operand of || is only evaluated when the left side is falsy, and the only way the expression name != "Fred" is falsy is if name === "Fred". Why are you comparing against "Steve" only in the case where you know the name to be "Fred" ?

I concur with this - it's not _good_ programming, but it should not be an error. It should be a warning, similar to declaring a variable that is never used. Maybe it could be harmful in other instances, where it's harmless in this instance?

This is like writing code like if ((x === 1) && (x === 2)) {. It simply doesn't make any sense at all.

I disagree. x can never be both 1 and 2. However, if name == "Eric";, then the if will be entered.

[RyanCavanaugh]

The assumption here is that you're not insane, and rather meant to write code that made sense. For example, it's likely that if the compiler flags a warning here, you meant to write && instead of ||, or === instead of !==, or one of your operands was incorrect.

This logic has found multiple bugs in existing well-tested codebases. It's a good trade-off vs just allowing nonsense code for the sake of allowing it.

[fedoranimus]

This logic has found multiple bugs in existing well-tested codebases. It's a good trade-off vs just allowing nonsense code for the sake of allowing it.

Okay, I can empathize with this position.

[ysmoradi]

class ValueHolder {
    private val: number = 0;
    public get value() : number {
        this.val++;
        return this.val;
    }
}

let valueHolder = new ValueHolder();
if (valueHolder.value != 1)
    throw new Error('!');
if (valueHolder.value.toString() == '2')
    alert('!');

I can't write valueHolder.value != 2 here! Why? you'll see that this code works if you copy and paste it to https://www.typescriptlang.org/play/

[ysmoradi]

Why we've allowUnreachableCode flag, but we've no flag for this one?
I can't even build the application that was working like a charm before!

[DouglasLivingstone]

I can't write valueHolder.value != 2 here! Why?

There's discussion about whether this should be allowed or not here: https://github.com/Microsoft/TypeScript/issues/9998

Previously the type-narrowing was only applied in cases where it was guaranteed to be correct, it has now been expanded to other cases where it is assumed that function calls never mutate values. In lots of code this is fine, since it makes type guards easier to use, but it does feel like it is crossing a line, since there are now errors produced for correct code.

[ysmoradi]

@DouglasLivingstone Thanks, but we can have a config for this. Something like allowUnreachableCode.

[ysmoradi]

Thanks to typescript's definite-assignment-assertions you can handle this.

Following won't gets compiled:

const vm = { model: { Id: "1" } };

if (vm.model.Id != "2")
    throw new Error("...");

// some codes with side effect on vm.model.Id's value, but for some reasons, typescript's code analyzer can't detect that.

if (vm.model.Id != "1") // compile error
    throw new Error("...");

Following code gets compiled:

const vm = { model: { Id: "1" } };

if (vm.model.Id! != "2")
    throw new Error("...");

// some codes with side effect on vm.model.Id's value, but for some reasons, typescript's code analyzer can't detect that.

if (vm.model.Id != "1")
    throw new Error("...");

You've to use ! after vm.model.Id in first if

if (vm.model.Id! != "2")