Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

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?

@manniL
Copy link

manniL commented Apr 20, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.