Skip to content

Infer type guard => array.filter(x => !!x) should refine Array<T|null> to Array<T> #16069

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
danvk opened this issue May 24, 2017 · 61 comments · Fixed by #57465
Closed

Infer type guard => array.filter(x => !!x) should refine Array<T|null> to Array<T> #16069

danvk opened this issue May 24, 2017 · 61 comments · Fixed by #57465
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@danvk
Copy link
Contributor

danvk commented May 24, 2017

TypeScript Version: 2.3

Code

const evenSquares: number[] =
    [1, 2, 3, 4]
        .map(x => x % 2 === 0 ? x * x : null)
        .filter(x => !!x);

with strictNullChecks enabled.

Expected behavior:

This should type check. The type of evenSquares should be number[].

Actual behavior:

The type of evenSquares is deduced as (number|null)[], despite the null values being removed via the call to .filter.

@mhegazy
Copy link
Contributor

mhegazy commented May 24, 2017

For filter specifically, #7657 would be something we can do here. but that still requires you to write an explicit type guard annotation somewhere, (x: number| null): x is number somewhere.
the other part is the compiler figuring out that type guard automatically from the function body, but that is not a trivial, #5101 tracks some aspects of that.

@mhegazy mhegazy added the Duplicate An existing issue was already created label May 24, 2017
@danvk danvk closed this as completed May 24, 2017
@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript and removed Duplicate An existing issue was already created labels Jun 29, 2017
@RyanCavanaugh
Copy link
Member

Reopening to track this since both other issues now target lib.d.ts changes

@dgreene1
Copy link

Subscribing since this is a common pattern I use after a map

@AlexGalays
Copy link

Just chiming in to say that this shouldn't be fixed just for Arrays.
This is very useful for all custom (container or not) types too.

@dgreene1
Copy link

dgreene1 commented Feb 28, 2018

Here's a workaround:

// I wish I could just do the following:
let result = ["", 3,null, undefined].filter(x => x != null);

Instead, you can do this:

// Create this helper function
function isNotNullOrUndefined<T extends Object>(input: null | undefined | T): input is T {
    return input != null;
}
// This actually determines that result is of type (string|number)[]
let result = ["", 3,null, undefined].filter(isNotNullOrUndefined);

I believe that the first snippet doesn't work due to TS not being able to automatically infer that the callback function inside the filter is a type guard. So by explicitly defining the function's return type as input is T then it allows filter to utilize the control flow features of the type checking to discriminate the union.

###################################################

That being said, I hope that we can get this type guard inference into the language. :)

@mhegazy mhegazy changed the title array.filter(x => !!x) should refine Array<T|null> to Array<T> Infer type guard => array.filter(x => !!x) should refine Array<T|null> to Array<T> May 2, 2018
@benschulz
Copy link

As I said in #10734 (comment), I'm very eager to see type guards inferred and have already implemented it prototypically for a subset of arrow expressions. If you get around to specifying the workings of type guard inference (i.e. the hard part), I'd gladly get involved in the implementation (the easier part).

@scott-ho
Copy link

scott-ho commented May 25, 2018

@dgreene1 I trid to write a simple operator to make the guard simpler to use, but failed. Do you have any suggestion?

import { Observable } from 'rxjs';
import { filter } from 'rxjs/operators';

export function isNotNullOrUndefined<T>(input: null | undefined | T): input is T {
  return input != null;
}

export function skipEmpty<T>() {
  return function skipEmptyOperator(source$: Observable<T>) {
    return source$.pipe(filter(isNotNullOrUndefined));
  };
}

@Andy-MS @mhegazy could you help to improve the types above?

@Hotell
Copy link

Hotell commented May 25, 2018

@scott-ho https://twitter.com/martin_hotell/status/999707821980647424
you're welcome :) 💃

@dgreene1
Copy link

@scott-ho, I'd check out the approach @Hotell shared. I wish I could help you more, but I'm not familiar with rxJs yet. So I'm not really sure what pipe() and filter() are sending you. Since the filter I know is the filter that exists on arrays, and since the snippet above doesn't have filter on an array, then I don't think it's the filter that pertains to this TypeScript issue. I.e. it's not the standard ES5 filter, it's rxJs' filter.

@scott-ho
Copy link

@Hotell Thanks for your guidance. It seems your solution only works in v2.8 or above.

And I finally make it works

import { Observable } from 'rxjs';
import { filter } from 'rxjs/operators';

export function isNotNullOrUndefined<T>(input: null | undefined | T): input is T {
  return input != null;
}

export function skipEmpty<T>() {
  return function skipEmptyOperator(source$: Observable<null | undefined | T>) {
    return source$.pipe(filter(isNotNullOrUndefined));
  };
}

@pie6k
Copy link

pie6k commented Aug 7, 2019

I thought I'll share my solution

export type Empty = null | undefined;

export function isEmpty(value: any): value is Empty {
  return [null, undefined].includes(value);
}

export function removeEmptyElementsFromArray<T>(array: Array<T | Empty>): T[] {
  return array.filter((value) => !isEmpty(value)) as T[];
}

example:

const nums = [2, 3, 4, null] // type is (number | null)[]
const numsWithoutEmptyValues = removeEmptyElementsFromArray(nums); // type is number[]

@samhh
Copy link

samhh commented Aug 7, 2019

@pie6k As far as I can tell that's not a solution, that's merely your assertion (as T[]) unsafely narrowing.

@MartinJohns
Copy link
Contributor

@pie6k That [null, undefined].includes(..) will always create a new array, won't it?

Here's a cleaned up version that has no need for type assertions:

function hasValue<T>(value: T | undefined | null): value is T {
    return value !== undefined && value !== null;
}

function removeEmptyElementsFromArray<T>(array: Array<T | undefined | null>): T[] {
    return array.filter(hasValue);
}

@pedrodurek
Copy link

@robertmassaioli, hasPresentKey it's not exactly the same as supporting in
operator narrowing, hasPresentKey relies on type predicate.

export function hasPresentKey<K extends string | number | symbol>(k: K) {
  return function <T, V>(
    a: T & { [k in K]?: V | null }
  ): a is T & { [k in K]: V } {
    return a[k] !== undefined && a[k] !== null;
  };
}

@graphemecluster
Copy link
Contributor

graphemecluster commented Sep 1, 2022

FWIW, I have seen somebody using flatMap in favor of filter all the time just to avoid writing type guard. I believe the reason behind is that writing a type guard is danger, you will misspell words.

interface Foo {
    type: "foo";
}
interface Bar {
    type: "bar";
}
declare const fooBar: (Foo | Bar)[];

const foos = fooBar.flatMap(v => v.type === "foo" ? [v] : []); // inferred as Foo[]
const bars = fooBar.flatMap(v => v.type !== "foo" ? [v] : []); // inferred as Bar[]

Basically all you need to do is to append ? [v] : [] to the function, and TypeScript narrows the type for us out of the box. Maybe this idea would help in writing an implementation too.

(Edit: Expanding the comments, this is already mentioned here.)

@infacto
Copy link

infacto commented Nov 25, 2022

Yes this is very annoying when chaning methods and/or check for more than just nullish.

type MyObj = { data?: string };
type MyArray = { list?: MyObj[] }[];
const myArray: MyArray = [];

const result = myArray
  .map((arr) => arr.list)
  .filter((arr) => arr && arr.length)
  .map((arr) => arr
//              ^^^ Object is possibly 'undefined'.
    .filter((obj) => obj && obj.data)
    .map(obj => JSON.parse(obj.data))
//                         ^^^^^^^^ Type 'undefined' is not assignable to type 'string'.
  );

Playground

The nullish type guard can handle it if directly set as function. But not in the case above.

const isNotNullish = <T>(input: T | null | undefined): input is T => {
  return input !== null && input !== undefined
}

// Work
array.filter(isNotNullish);

// Not work
array.filter(item => isNotNullish(item) && item.length)
array.filter(item => item?.length)

Now I could write a custom type guard like isNotNullish. Something like isArrayFilled to also check for length. Same to test property on object (in this case the "data"). I could also use the not-null assertion ! here (in my case disable ESLint rule).
Or I cast everytime to the same type again and again and again and ... It would be great if find a better solution. My example is maybe not the best. And I know why this happens. But I have faith in the TypeScript developers.

@MartinJohns
Copy link
Contributor

@infacto You could also just add a type annotation.

@infacto
Copy link

infacto commented Nov 28, 2022

@MartinJohns All variables are well typed (maybe some in context in the example above). Do you mean casting or use not-null assertion? Then I would either break the chain and move the first result in a new not-null variable. Otherwise I have to do it (cast, assert) everywhere again and again. I just wanted another example for this topic here (Infer type guard from filter). But I understand the problem. Not sure if TypeScript can fix that. Because the callback must explicitly set the return type like a type-guard does. And TS does not know that's inside the callback. But the TS lang devs know it better for sure. Idk. Spontaneous ideas (not tested): Wrap filter fn in a type-guard fn or use an own custom function to filter array with condition fn which returns type guarded type (input is T from T | undefined | null or unknown). Something like arr.filter(notNull((value) => doStuff())) or notNull as standalone filter. ...

@temoncher
Copy link

@infacto I think maybe @MartinJohns meant typing filter predicate as a typeguard like this
First example playground

type MyObj = { data?: string };
type MyArray = { list?: MyObj[] }[];
const myArray: MyArray = [];

const result = myArray
  .map((arr) => arr.list)
  .filter((arr): arr is MyObj[] => !!(arr && arr.length))
  .map((arr) => arr
    .filter((obj): obj is Required<MyObj> => !!(obj && obj.data))
    .map(obj => JSON.parse(obj.data))
  );

Second example

array.filter((item): item is WhateverTheTypeOfArrayItemIs[] => item?.length)

But I want to admit I don't like this option

@temoncher
Copy link

temoncher commented Nov 28, 2022

@infacto Actually we can make first example work with current state of things using some more complex typeguards Playground

type MyObj = { data?: string };
type MyArray = { list?: MyObj[] }[];
const myArray: MyArray = [];

const isNotNullish = <T>(input: T | null | undefined): input is T => input != null
const isNotEmpty = <T>(input: T[]) => input.length !== 0;

function both<I, O1 extends I>(
  predicate1: (input: I) => boolean,
  predicate2: (input: I) => input is O1,
): (input: I) => input is O1;
function both<I, O1 extends I>(
  predicate1: (input: I) => input is O1,
  predicate2: (input: O1) => boolean,
): (input: I) => input is O1;
function both<I, O1 extends I, O2 extends O1>(
  predicate1: (input: I) => input is O1,
  predicate2: (input: O1) => input is O2,
): (input: I) => input is O2;
function both<I>(
  predicate1: (input: I) => boolean,
  predicate2: (input: I) => boolean,
) {
  return (input: I) => predicate1(input) && predicate2(input)
}

type HasNonNullishPredicate<P extends string> = <T extends { [K in P]?: unknown }>(obj: T) => obj is T & { [K in P]: Exclude<T[K], undefined | null> };
function hasNonNullish<P extends string>(prop: P): HasNonNullishPredicate<P>;
function hasNonNullish(prop: string) {
  return (input: object) => prop in input && (input as any).prop != null; // this `as any` cast will be unnecessary in TSv4.9
}

const result = myArray
  .map((arr) => arr.list)
  .filter(both(isNotNullish, isNotEmpty))
  .map((arr) => arr
    .filter(hasNonNullish('data'))
    .map(obj => JSON.parse(obj.data))
  );

@infacto
Copy link

infacto commented Nov 29, 2022

@temoncher Thanks for your input. Helpful stuff. Somehow I hadn't thought of the return value of the arrow function in the filter method. 🤦‍♂️ Only indirect. Yes, makes sense. ^^ But anyway, it would be great if TypeScript could just infer that for less and cleaner code.

@infacto
Copy link

infacto commented Dec 2, 2022

Sorry if I disturb you. I'm facing this problem again, where infer type guard for filter function would be awesome. Just another contribution post or thinking out loud.

To the example above:

context.filter((obj): obj is Required<MyObj> => !!obj?.data)

This guards the objects to have all properties required. In this example, it's ok. But for more complex object with several (optional) properties should rather do that.

context.filter((obj): obj is MyObj & Required<Pick<MyObj, 'data'>> => !!obj?.data)

Pick only the tested properties. Otherwise you may obfuscate nullish properties. It's a really long johnny. Infer type would simply look like:

context.filter((obj) => !!obj?.data)

Now I craft many type guards to avoid such long complex type defs. I mean this is just a simple example. The real world code type def would be 2 miles of code horizontal scoll. I thinking of a chain guard-type function like rxjs pipes. But it's either exhausting or unsafe (e.g. simple cast from unknown by assuming types). I don't want to rule out that I'm thinking too complicated and that there is a much simpler way. I just want to fix strict type issues. In best case without blind casting.

@robertmassaioli
Copy link

@infacto in pretty sure that's what isPresentKey does from ts-is-present

@mhmo91
Copy link

mhmo91 commented Apr 20, 2023

This is pretty annoying issue that happens quite often. Can we raise priority on this one?

@TheAlexLichter
Copy link

Leaving https://github.com/total-typescript/ts-reset/ here as a drop-in "fix" until this lands in TS itself.

@btoo
Copy link

btoo commented Apr 20, 2023

i hope anyone advocating for using is knows that's still inherently type unsafe

function isPresent<T>(input: null | undefined | T): input is T {
    return !input;
}

const arr = ["string", 8, null, undefined].filter(isPresent);

// expects ["string", 8]
// but is instead [null, undefined]
console.log(arr)

is is no different from as, so i still like using flatMap, especially with the benchmark in #16069 (comment) revealing that (the JIT compiler is allowing that) the performance cost is smaller than linear

@ptitjes
Copy link

ptitjes commented Jul 12, 2023

Hi all,

When will TypeScript's type inference be fixed to handle this simple example ?

// Types as `(o: string | undefined) => o is string`
const myGuard = (o: string | undefined): o is string => !o;

// Types as `(o: string | undefined) => boolean` but should type as `(o: string | undefined) => o is string`
const mySecondGuard = (o: string | undefined) => myGuard(o);

@JustinGrote
Copy link

Thank you @danvk and @RyanCavanaugh!

@ethanresnick
Copy link
Contributor

@ptitjes The issue with your example is that the first guard is not correct:

const myGuard = (o: string | undefined): o is string => !o;

const emptyString = '';

if(!myGuard(emptyString)) {
  // according to the "if and only if" nature of type predicates,
  // the fact that the predicate failed means that `emptyString` 
  // _must not_ be a string, but actually it is.
}

@b-fett
Copy link

b-fett commented Jul 2, 2024

i use such a patch:

diff --git a/node_modules/typescript/lib/lib.es5.d.ts b/node_modules/typescript/lib/lib.es5.d.ts
index a88d3b6..f091ecb 100644
--- a/node_modules/typescript/lib/lib.es5.d.ts
+++ b/node_modules/typescript/lib/lib.es5.d.ts
@@ -1268,6 +1268,7 @@ interface ReadonlyArray<T> {
      * @param thisArg An object to which the this keyword can refer in the predicate function. If thisArg is omitted, undefined is used as the this value.
      */
     filter<S extends T>(predicate: (value: T, index: number, array: readonly T[]) => value is S, thisArg?: any): S[];
+    filter<F extends BooleanConstructor>(predicate: F, thisArg?: any): Exclude<T, false | null | undefined | '' | 0>[];
     /**
      * Returns the elements of an array that meet the condition specified in a callback function.
      * @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array.
@@ -1459,6 +1460,7 @@ interface Array<T> {
      * @param thisArg An object to which the this keyword can refer in the predicate function. If thisArg is omitted, undefined is used as the this value.
      */
     filter<S extends T>(predicate: (value: T, index: number, array: T[]) => value is S, thisArg?: any): S[];
+    filter<F extends BooleanConstructor>(predicate: F, thisArg?: any): Exclude<T, false | null | undefined | '' | 0>[];
     /**
      * Returns the elements of an array that meet the condition specified in a callback function.
      * @param predicate A function that accepts up to three arguments. The filter method calls the predicate function one time for each element in the array.

with

[].filter(Boolean)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.