TS doesn't narrow generic parameter T to null, despite an explicit comparison

[arafathusayn]

🔎 Search Terms

T to null

🕗 Version & Regression Information

• This is a type narrowing issue
• This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.5.0-pr-57465-110#code/GYVwdgxgLglg9mABDAzgORAG0wHgCoB8AFAG4CGmIApgFyJ4CUiA3gFAD07i3P3AegH5W3GMESkK1RAF5ZiMFkxM2vcpSrdO9ADSIUACzhYAJogBGGhdmGIAvqxsAnKlBCOkaqbOnzFAblZ7IA

💻 Code

function isNull<T>(value: T) {
  if (value === null) {
    value  // T, should be null
  }

  return value === null;
}

🙁 Actual behavior

value is T inside the if block of value === null (explicit comparison)

🙂 Expected behavior

value should be of type null inside the if block of value === null

Additional information about the issue

No response

[jcalz]

This is sort of a duplicate of #13995 except that that is closed because it's largely fixed.

Note that the fact that value is still of type T isn't itself a problem. It's that value is not also constrained by null. That is, the real problem is in the const v: null = value line in:

function isNullBad<T>(value: T) {  
  if (value === null) {
    value 
    // ^? (parameter) value: T // this is okay
    const v: null = value; // error, this is not
  }
  return value === null;
}

#13995 was fixed by #43183 and specifically only for the case where the generic type parameter is constrained to a union. So if you want to work around it today you could constrain T to {} | null | undefined, which is a near-synonym of unknown, the implicit constraint of an unconstrained type parameter:

function isNull<T extends null | {} | undefined>(value: T) {  
  if (value === null) {
    value // still T
    // ^? (parameter) value: T extends null | {} | undefined // this is still okay
    const v: null = value; // okay now also
  }
  return value === null;
}

Playground link

[Andarist]

The negated branch gets nicely narrowed down. The observed behavior is quite surprising here (TS playground):

function acceptNull(arg: null) {}
function acceptNonNull(arg: {} | undefined) {}

function test<T>(value: T) {
  if (value === null) {
    value;
    // ^? (parameter) value: T
    acceptNull(value) // error
  } else {
    value;
    // ^? (parameter) value: T & ({} | undefined)
    acceptNonNull(value) // ok
  }
}

[Andarist]

There are 2 mechanisms at play here and in related cases.

getAdjustedTypeWithFacts focuses today on narrowing negated cases (TypeFacts.NEUndefined, TypeFacts.NENull, etc). That's why the other branch above works just fine. It's somewhat special because this kind of logic often can't be expressed easily by intersecting the current type with something else or by discriminating the original constraints etc. You need to handle those cases in a special way here - when something is not null then it has to be either {} or undefined. This has those kind of rules here.

Fixing it here, in a somewhat trivial way, has almost not downsides:

git diff

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index c8eefa7778..620f0ffe49 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -26986,6 +26986,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         const reduced = recombineUnknownType(getTypeWithFacts(strictNullChecks && type.flags & TypeFlags.Unknown ? unknownUnionType : type, facts));
         if (strictNullChecks) {
             switch (facts) {
+                case TypeFacts.EQUndefined:
+                    return mapType(reduced, t => getIntersectionType([t, undefinedType]));
+                case TypeFacts.EQNull:
+                    return mapType(reduced, t => getIntersectionType([t, nullType]));
                 case TypeFacts.NEUndefined:
                     return removeNullableByIntersection(reduced, TypeFacts.EQUndefined, TypeFacts.EQNull, TypeFacts.IsNull, nullType);
                 case TypeFacts.NENull:
diff --git a/tests/baselines/reference/unknownControlFlow.types b/tests/baselines/reference/unknownControlFlow.types
index 7b320964b8..43cef8a6a2 100644
--- a/tests/baselines/reference/unknownControlFlow.types
+++ b/tests/baselines/reference/unknownControlFlow.types
@@ -214,56 +214,56 @@ function f21<T>(x: T) {
     }
     else {
         x;  // T
->x : T
+>x : T & undefined
     }
     if (x !== null) {
 >x !== null : boolean
->x : T
+>x : (T & ({} | null)) | (T & undefined)
 
         x;  // T & ({} | undefined)
->x : T & ({} | undefined)
+>x : (T & {}) | (T & undefined)
     }
     else {
         x;  // T
->x : T
+>x : T & null
     }
     if (x !== undefined && x !== null) {
 >x !== undefined && x !== null : boolean
 >x !== undefined : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
 >undefined : undefined
 >x !== null : boolean
->x : T & ({} | null)
+>x : (T & null) | (T & {})
 
         x;  // T & {}
 >x : T & {}
     }
     else {
         x;  // T
->x : T
+>x : (T & null) | (T & undefined)
     }
     if (x != undefined) {
 >x != undefined : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
 >undefined : undefined
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
     else {
         x;  // T
->x : T
+>x : (T & null) | (T & undefined)
     }
     if (x != null) {
 >x != null : boolean
->x : T
+>x : (T & null) | (T & {}) | (T & undefined)
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
     else {
         x;  // T
->x : T
+>x : (T & null) | (T & undefined)
     }
 }
 
@@ -281,14 +281,14 @@ function f22<T extends {} | undefined>(x: T) {
     }
     else {
         x;  // T
->x : T
+>x : T & undefined
     }
     if (x !== null) {
 >x !== null : boolean
->x : T
+>x : (T & {}) | (T & undefined)
 
         x;  // T
->x : T
+>x : (T & {}) | (T & undefined)
     }
     else {
         x;  // T
@@ -297,7 +297,7 @@ function f22<T extends {} | undefined>(x: T) {
     if (x !== undefined && x !== null) {
 >x !== undefined && x !== null : boolean
 >x !== undefined : boolean
->x : T
+>x : (T & {}) | (T & undefined)
 >undefined : undefined
 >x !== null : boolean
 >x : T & {}
@@ -307,30 +307,30 @@ function f22<T extends {} | undefined>(x: T) {
     }
     else {
         x;  // T
->x : T
+>x : T & undefined
     }
     if (x != undefined) {
 >x != undefined : boolean
->x : T
+>x : (T & {}) | (T & undefined)
 >undefined : undefined
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
     else {
         x;  // T
->x : T
+>x : T & undefined
     }
     if (x != null) {
 >x != null : boolean
->x : T
+>x : (T & {}) | (T & undefined)
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
     else {
         x;  // T
->x : T
+>x : T & undefined
     }
 }
 
@@ -348,25 +348,25 @@ function f23<T>(x: T | undefined | null) {
     }
     if (x !== null) {
 >x !== null : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
 
         x;  // T & {} | undefined
 >x : (T & {}) | undefined
     }
     if (x != undefined) {
 >x != undefined : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
 >undefined : undefined
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
     if (x != null) {
 >x != null : boolean
->x : T | null | undefined
+>x : (T & {}) | null | undefined
 
         x;  // T & {}
->x : NonNullable<T>
+>x : T & {}
     }
 }
 
@@ -957,7 +957,7 @@ function doSomething1<T extends unknown>(value: T): T {
 >undefined : undefined
 
         return value;
->value : T
+>value : T & undefined
     }
     if (value === 42) {
 >value === 42 : boolean

However, it kinda creates some extra type identities and the control flow isn't able to "merge" those types back to just T when they both branches join etc.

The other mechanism - the one that handles the case when T extends {} | null | undefined - is getNarrowableTypeForReference. It avoid the above problem by substituting constraints in constraints positions. Treating an unconstrained type parameter as constrained to unknown (or rather to {} | null | undefined aka unknownUnionType) could fix this here. However, I found that this has some - IMHO - negative effects on printed errors. So at the very least, some extra work around that would be done here. I also had other problem when following this route but I didn't yet have the time to analyze them.

[RyanCavanaugh]

the control flow isn't able to "merge" those types back to just T when they both branches join etc.

This is a big deal, actually. If this invariant doesn't hold, then you get "spooky action at a distance" bugs where adding an if statement causes effects in later code outside the CFA path of the if

Any fix here needs to not have downsides. I'd really prefer there be more concrete upsides too; at the point you've narrowed a value to a unit type, it's not clear why you'd write the identifier over the value literal (i.e. why not just write acceptsNull(null); ?)

[danvk]

This issue results in some odd behavior with inferred type predicates:

// ✅ returns x is T & ({} | undefined); good!
function isNonNull<T>(x: T) {
    return x !== null;
}

// 😔 returns boolean; would prefer x is T & null
function isNull<T>(x: T) {
    return x === null;
}

// 🤔 returns x is T & null
function isNull2<T>(x: T) {
    return (typeof x === "object" && x === null);
}

in particular see this tweet for the source of this issue.

I'd guess that checking for non-null/undefined is the more common case, so it's nice that type predicates are at least inferred in that direction.