typeof discards previous non-null checks

[mgol]

TypeScript Version: 3.2.0-dev.20181025

Search Terms:
unknown typeof object null

Code

Run the following code via tsc --noEmit --strict test.ts

declare const x: unknown;
x !== null && typeof x === 'object' && 'field' in x;

Expected behavior:
It should pass the type check as the first condition ensures x is not null and the second one narrows the type to object | null. Together with the previous non-null check it should result in the object type.

Actual behavior:

test.ts:2:42 - error TS2531: Object is possibly 'null'.

2 x && typeof x === 'object' && 'field' in x;
                                           ~

A workaround is to switch the order of typeof and the x truthiness check:

declare const x: unknown;
typeof x === 'object' && x !== null && 'field' in x;

Playground Link: Note: you need to enable strictNullChecks manually!
https://www.typescriptlang.org/play/#src=declare%20const%20x%3A%20unknown%3B%0D%0Ax%20!%3D%3D%20null%20%26%26%20typeof%20x%20%3D%3D%3D%20'object'%20%26%26%20'field'%20in%20x%3B%0D%0A

Related Issues: #27180

[ahejlsberg]

This is a design limitation in the control flow analyzer. Control flow analysis fundamentally is concerned with narrowing union types, so when the current control flow type of x is unknown, the expression x !== null doesn't narrow x because unknown is a non-union type and undivisible, so to speak. However, the check typeof x === 'object' changes the control flow type of x to object | null and following that it is possible to narrow away the null. So, it works if when you write it in the opposite order. Would be nice to do better, but it is not a trivial task.

[voltrevo]

@ahejlsberg I hit what I think is the same issue today, but it doesn't seem to fit that explanation:

type Processing = { t: 'processing' };

type Item = (
  {
    found: true,
    summary: Processing | string;
  } |
  {
    found: false,
    summary: '(not found)',
  } |
  never
);

export function getProcessedString(item: Item): string | null {
  return (
    typeof item.summary === 'string' &&
    item.found &&
    item.summary.toUpperCase()
  ) || null;
}

typescript@3.2.0-dev.20181101 output:

bug.ts:19:18 - error TS2339: Property 'toUpperCase' does not exist on type 'string | Processing'.
  Property 'toUpperCase' does not exist on type 'Processing'.

19     item.summary.toUpperCase()
                    ~~~~~~~~~~~

This one can also be resolved by switching the two checks around.

Maybe it doesn't fit the explanation because it's not actually the same issue. Should I file a new one?

[RyanCavanaugh]

@lll000111 how much slower are you willing to make builds in order to allow that to happen?

[lll000111]

Orig. comment:

If this is a "design limitation" this needs some serious documentation. This is completely unexpected. See my tiny examples in #32301 — how would anyone guess what's going on there?

I think that it should not be the implementation that is the focus, but the result and the impact. What is easier: Get all TS developers to know that in TS the order of OR-connected expressions in an if() used for refinement matters, sometimes, or to change the implementation? Does the sky turn around us in the center, or is it the earth that's turning?

---

@RyanCavanaugh I refer to my comment. What a strange question. Also, I don't see how this connection you make is a natural law. If it was this whole issue would not be so surprising because we would be used to it. Facebook's Flow does not have this problem. If you want to explain to me something about the internals of TS &,dash; well, see my previous comment. I mean, IMO this is quite a stunning revelation, this particular issue, and I've been reading the issues for a long time (both Flow and here) and accept a lot of the limitations of both tools because I do understand the enormous complexity and hate the hype of the fanboys "why doesn't ecmascript adopt types", people who are not aware of the huge amount of issues, many of them unsolvable, when you write a tool that tries to make hard statements about people's code.

And as I said, irregardless of how feasible it is, or that you decide it is, to change the implementation, this is so unexpected that it should be heavily documented. I mean, which developer would think to themselves "I may have to flip the order of my if() condition expressions"? This is not exactly a common thought that one would come up with, nor is there some magic word that would immediately let one google the correct issue/discussion thread that explains the behavior. Let's also keep in mind that only an incredibly tiny minority of developers will file an issue here, so you won't see anything close to the true impact of the problem by counting opened issues.

[ethanresnick]

Will #29317 (negated types) address this, or at least make a solution possible?

[sylann]

Not sure if related but in case:

I wanted to write a function that ensures a value has an expected type at runtime and also infers the type at compile time but typeof emits a type that doesn't make sense:

type ValueType = string | string[] | number | boolean

function checkType<T extends ValueType>(value: unknown, fallbackValue: T): T {
  if (value === undefined) return fallbackValue;
  const type = typeof value;
  const validType = typeof fallbackValue;
  if (type === validType) return value;
  throw new Error(`Unexpected value type: ${type}. Should be ${validType}.`);
}
// usage:
checkType<string>(value, '')

Emitted type for validType is "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function" even though it should be "string" since fallbackValue (and therefore T) is of type string.
Consequently, value doesn't have the correct type and the function does not compile.

I have to cast the return value as follows return value as T for it to compile.
But this effectively makes the whole type checking useless since I could have whatever and it would work.

Am I missing something?

[ahejlsberg]

I'm changing this from a design limitation to a bug. We keep seeing the issue reported and I have a fix that will properly address the issue. PR coming soon.