Simple inversion of `&&` causes types to stop being valid, even though the js execution is the same

[danon]

🔎 Search Terms

I think both of these notations should yield the same result.

function f4 (obj: object, field: 'start'|'end') {
    if (field === 'start' && field in obj) {
        obj[field]
    }
    throw new Error();
}

function f5 (obj: object, field: 'start'|'end') {
    if (field in obj && field === 'start') {
        obj[field]
    }
    throw new Error();
}

But actually one of them is marked as valid and the other as not:

Same thing happens with two nested ifs if you just invert them.

Reproduction: https://www.typescriptlang.org/play/?#code/GYVwdgxgLglg9mABMALIgFHARgKwFyLY4Cm0ANMjMQDYAmBA5AM5QCGATlAwD4PFi0GASkQBvAFCIpiGMAzAqdRAF5ViZm04NEAMh2UatGUiIiJ0i4VwBtBYYC6k6QF8nUqAAt2cAO6IwxH4Aouze7OhCANziruKgkLAIyACsGEQERKRQFHZ0jCwcXLz8gmZuMnLouUYwJri6+tUqahqFwmLlFkS2irSOFq4Wnt5+AcGhcOFRMeJAA

🕗 Version & Regression Information

• This changed between versions 4.8.4 and 4.9.5 - At 4.8.4 and before both of these were invalid.
• This changed in commit or PR _______
• This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
• I was unable to test this on prior versions because _______

⏯ Playground Link

No response

💻 Code

function f1(obj: object, field: 'start'|'end') {
    if (field === 'start' && field in obj) {
        obj[field]
    }
    throw new Error();
}

function f2(obj: object, field: 'start'|'end') {
    if (field in obj && field === 'start') {
        obj[field]
    }
    throw new Error();
}

🙁 Actual behavior

It reports as erroneous, even tough I think it's a valid code.

🙂 Expected behavior

Both of these should be accepted as valid.

Additional information about the issue

No response

[RyanCavanaugh]

Narrowing doesn't reach back into earlier in the process to re-apply new information gained later in the control flow graph, so these are actually different from the checker's perspective. It's not a constraint solver.

I don't think this is fixable without a pretty invasive restructuring of how narrowing works with some fairly serious performance / circularity implications.

[danon]

@RyanCavanaugh it doesn't need to reach earlier. The error happens inside the condition block, so in both cases it's later, not earlier.

[MartinJohns]

In f1 the value obj gets narrowed to object & Record<"start", unknown>, so an object with a property start. This is possible because field is narrowed to "start".

In f2 the value doesn't get narrowed, it remains object. That's because field is not narrowed to a specific value before the in obj check (this is what Ryan means with earlier), so the compiler can't narrow obj to a type with that specific property. There's no type it can narrow obj to.

[danon]

function f2(obj: object, field: 'start'|'end') {
    if (field === 'start' && field in obj) {
        obj[field]
    }
    throw new Error();
}

I need to write a typescript-sound function to return a field from an object or throw an error. I don't want to use as or is, because they essentially disable static-type checks. I think field in obj or possibly typeof obj[field] should be enough to guarantee the presence of the field in an object in js. How can I write that function so I could accept arbitrary string in f2()?

[MartinJohns]

Use Record<> instead of object.

[danon]

@MartinJohns how do I narrow unknown to Record<> using type guards? (I don't want to use type assertions such as as or is, because they aren't checked by ts.

[MartinJohns]

You don't. You use a type assertion, that's what they're there for.

[danon]

You don't. You use a type assertion, that's what they're there for.

I want to check the type dynamically, so that if the unknown happens to contain an incompatible type, I need to throw new Error().

If I use as without actually checking the type, typescript will accept it and the invalid value will be propagated further to the system, and I will discover it somewhere else in the stack, milion functions later where I'll see something Cannot call function substr() on number or something. I will think "how on earth number got here?" and I will trace it to this as and I'll have wasted 30 minutes doing that.

[danon]

I managed to get it to work with a single field:

function f4 (obj: object, field: 'start') {
    if (field in obj) {
        obj[field]
    }
    throw new Error();
}

Notice that in that example I don't need type assertions such as as or in.

(Truth be told the word "assertion" is kinda misleading, because it suggests that invalid type will be rejected (as in other languages). But what in fact happens is the invalid type is silently ignored, and it's just being passed on. So they should be called "type silencers" :D(

[MartinJohns]

You use a type assertion and still perform your actual runtime checks. That's what type assertions are there for, when you reach the limits of the automatic type narrowing or you have very dynamically typed code.

I managed to get it to work with a single field:

That works for the same reason I outlined in my first comment.

Type assertion is not misleading. You assert to the compiler that something is of a specific type.

But the issue is getting out of hand, this repository is not a suitable place for basic language support questions. I'm out, peace.

[danon]

@MartinJohns I was able to get it slightly to work by doing something like that:

function cast(value: unknown, field: 'start'|'end'): string {
  if (typeof value === 'object' && value !== null) {
    return castField(value, field);
  }
  throw new Error();
}

function castField(value: object, field: 'start'|'end'): string {
  const object: {start?: unknown, end?: unknown} = value;
  if (field in object) {
    return castString(object[field]);
  }
  throw new Error();
}

function castString(value: unknown): string {
  if (typeof value === 'string') {
    return value;
  }
  throw new Error();
}

Notice I didn't have to use as and in, I have a sound typescript and dynamic checking. The only problem I have left with that is that 'string' and 'end' are known at compile time. I'm trying to update it so they can be general string.

Type assertion is not misleading. You assert to the compiler that something is of a specific type.

But if you make a mistake, and that value is not of that type, then that is completely unchecked. I don't want to use that, I need to find a different solutio. In other languages like Python or Java, if you assert a type like that, you get an error in runtime. In typescript it's silently passed on.

[mkantor]

Like Martin said, this isn't the place for general language discussion. We're not really commenting on the issue anymore. If you want to talk about stuff like this, consider joining the TypeScript Community Chat.

Against my better judgement, I'll engage anyway (but will resist the urge after this comment):

I'm trying to update it so they can be general string.

You can generalize it like so:

function cast(value: unknown, field: PropertyKey): string {
  if (value !== null && value !== undefined) {
    const possiblyHasField: Partial<Record<PropertyKey, unknown>> = value;
    return castString(possiblyHasField[field]);
  }
  throw new Error();
}

I'd suggest using one of the many battle-tested validation libraries if you need to do this kind of thing often (Zod, Valibot, Superstruct, io-ts, TypeBox, ArkType, etc).

Type assertion is not misleading. You assert to the compiler that something is of a specific type.

But if you make a mistake, and that value is not of that type, then that is completely unchecked.

That's why you encapsulate it behind a safe API. I agree that as, is, etc should be treated with caution and avoided when possible, but that's not the same as "never use them". There are other unsafe things in TypeScript's type system and you're not avoiding all of them (e.g. optional properties are inherently risky).

[danon]

You can generalize it like so:

function cast(value: unknown, field: PropertyKey): string {
  if (value !== null && value !== undefined) {
    const possiblyHasField: Partial<Record<PropertyKey, unknown>> = value;
    return castString(possiblyHasField[field]);
  }
  throw new Error();
}

cast("string", "field"); // returns undefined, instead of throwing