3

Trying to reach peak TypeScript performance I'm currently getting into some of the niche areas of the language, and there's something I don't get.

With strict-null-checks and strict mode and that stuff enabled, I don't understand why this code gives me an error:

function filterNullable<T>(arr: T[]): NonNullable<T>[] {
    const ret: NonNullable<T>[] = [];
    arr.forEach(el => {
        if (el !== null && el !== undefined) {
            ret.push(el); // TS2345: Argument of type 'T' is not assignable to parameter of type 'NonNullable '.
        }
    })
    return ret;
}

Clearly, even if T is a type like string | number | undefined, by the time I'm trying to push the filtered value into the array, it's pretty damn clear that it can't be undefined. Is TypeScript not smart enough to follow me here (I don't think so, because similar things work just fine), am I missing a crucial detail, or is the NonNullable type simply not as powerful as one might expect?

(I'm aware that there are more concise methods of doing what I'm doing here, however I need code with a similar logic to do more complicated things, so the Array.prototype.filter style method is not quite applicable)

-------Edit: I kind of get the problem now, however, instead of using type assertions in the push method, I'd then rather rewrite the whole thing to something like

function filterNullable2<T>(arr: Array<T | undefined | null>): T[] {
    const ret: T[] = [];
    arr.forEach(el => {
        if (el !== null && el !== undefined) {
            ret.push(el);
        }
    })
    return ret;
}

which seems to do a better job. But it needs more writing.

What I'm really trying to do is find a better way to write a utility for NGRX, as this is really annoying (thank god there isn't a third nullish type)

import {MemoizedSelector, Store} from "@ngrx/store";
import {filterNullable} from "../shared/operators";
import {Observable} from "rxjs";

export function selectDefined<T, S> (store: Store<T>, selector: MemoizedSelector<T, NonNullable<S>>): Observable<S>;
export function selectDefined<T, S> (store: Store<T>, selector: MemoizedSelector<T, NonNullable<S> | null>): Observable<S>;
export function selectDefined<T, S> (store: Store<T>, selector: MemoizedSelector<T, NonNullable<S> | undefined>): Observable<S>;
export function selectDefined<T, S> (store: Store<T>, selector: MemoizedSelector<T, NonNullable<S> | undefined | null>): Observable<S> {
  return store.select(selector).pipe(filterNullable());
}

I'd hope to find a way to write this like

export function selectDefined<T, S> (store: Store<T>, selector: MemoizedSelector<T, S>): Observable<NonNullable<S>> {
  return store.select(selector).pipe(filterNullable());
// TS2322: Type 'Observable<S>' is not assignable to type 'Observable<NonNullable<S>>'.   Type 'S' is not assignable to type 'NonNullable<S>'.
}

which, unfortunately, does not work

NoBullsh1t
  • 483
  • 4
  • 14
  • what do you mean with "But it needs more writing"? Is there any issue with your second code snippet? – Tobias S. Apr 11 '22 at 14:13
  • See the filterNullable() operator in the last snipped. It's wirtten "like" filterNullable2. What I really want is not having to overload the selectDefined function. I'll add how I'd like to write that, which doesn't work... – NoBullsh1t Apr 11 '22 at 14:16
  • It sounds like you want to know why this is happening and not how to work around it (e.g., assert, user-defined type guard, complete refactor). If so, then I can write up an answer pointing to https://github.com/microsoft/TypeScript/issues/48048 and hence https://github.com/microsoft/TypeScript/pull/22348; essentially, the compiler does not introduce generic conditional types like `NonNullable` in control flow because it makes things too complicated and breaks other things. – jcalz Apr 11 '22 at 14:24
  • @jcalz well, yeah. I'd like to understand why this doesn't work (which I think I do now, though it's not satisfactory), to really fix the oveloading issue of the selectDefined function. I get that this is idealistic, since I already have a solution there as well, but, as mentioned, such constructs lead into combinatory hell damn quickly if there were more types to exclude... but if you just tell me straight that there's no way around this, that's also okay ;) – NoBullsh1t Apr 11 '22 at 14:30
  • So then I'll write up an answer explaining the unfortunate reality when I get a chance, and leave it to you to deal with how to work around it. Note that I think the explanation in the existing answer here is incorrect ("you could still assign `null` of `undefined` so that's why it doesn't narrow from `T` to `NonNullable`"), which is why I'm bothering to intervene here. – jcalz Apr 11 '22 at 14:36
  • That'd be great, since this is the second time I'm looking into this and haven't really found a good explanation out there :) – NoBullsh1t Apr 11 '22 at 14:39

2 Answers2

2

This is essentially a design limitation of TypeScript. Inside the implementation of a generic function, it is difficult for the compiler to reason about types that depend on unspecified type parameters like T. See microsoft/TypeScript#48048 for an authoritative answer.


For a value of a specific type like string | undefined, the compiler can use control flow analyasis to filter the type of the value to either string or undefined. Indeed, your code works just fine if we replace the generic T with the specific string | undefined:

function filterNullable(arr: (string | undefined)[]): NonNullable<string | undefined>[] {
  const ret: NonNullable<string | undefined>[] = [];
  arr.forEach(el => {
    if (el !== null && el !== undefined) {
      ret.push(el); // okay
    }
  })
  return ret;
}

Unfortunately, for a value of a generic type like T, there is no specific filtering to be applied. If the compiler knew that T were constrained to a union of types, then the null check could possible narrow el from T to some specific subtype of the constraint, like this:

function filterNullable<T extends string | undefined>(arr: T[]): NonNullable<T>[] {
  const ret: NonNullable<T>[] = [];
  arr.forEach(el => {
    if (el !== null && el !== undefined) {
      el.toUpperCase() // this works
      ret.push(el); // this still doesn't
    }
  })
  return ret;
}

See how since T extends string | undefined, the compiler realizes that el must be assignable to string after the null check. That's great, and support for that was added in TypeScript 4.3 as contextual narrowing for generics. But that's not good enough for you, since you need el to be narrowed not to the specific type string but to the generic type NonNullable<T> (which is surely some subtype of string but might be narrower, if, say, T is "foo" | "bar" | undefined).


And here we're stuck. The compiler simply does not synthesize the type NonNullable<T> in response to a null check. The NonNullable<T> utility type is implemented as a conditional type, and conditional types are not synthesized by the compiler as a result of control flow analysis.

At one point there was a pull request at microsoft/TypeScript#22348 which would have enabled such narrowings. But it was never merged:

This has inspired many a conversation over the years, however as-is we know we have no shot of merging this. Conditional types have too many drawbacks for it to be OK to introduce them in control flow (like being very hard to relate correctly).

So for now it's not possible.


There are, of course, workarounds, the easiest of which is just to use a type assertion. I won't go into other possibilities here since it's mostly out of scope for the question.

Playground link to code

jcalz
  • 264,269
  • 27
  • 359
  • 360
0

After the null and undefined checks the variable el remains of type T, as you could still assign null or undefined to the variable inside the if statement.

To solve this problem just tell the compiler that you are pushing an NonNullable<T>:

ret.push(el as NonNullable<T>)

To make your code a little bit more concise:

function filterNullable<T>(arr: T[]): NonNullable<T>[] {
    return arr.filter((s) : s is NonNullable<T> => !!s)
}
Tobias S.
  • 21,159
  • 4
  • 27
  • 45
  • 1
    I don't see how the first sentence of the answer really applies; can you explain how it differs from [this](https://tsplay.dev/wRRXXw)? Control flow analysis is what allows you to assume something isn't null after you check it; if you do end up assigning null to it then control flow narrowing is reset. The issue has to do with generics and conditional types, not with the possibility of re-introducing nullishness. – jcalz Apr 11 '22 at 14:28
  • Oh wow, okay, thanks for clarifying this @jcalz – NoBullsh1t Apr 11 '22 at 14:35