microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.37k stars 12.41k forks source link

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

Closed danvk closed 6 months ago

danvk commented 7 years ago

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 commented 7 years ago

For filter specifically, https://github.com/Microsoft/TypeScript/issues/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, https://github.com/Microsoft/TypeScript/issues/5101 tracks some aspects of that.

RyanCavanaugh commented 7 years ago

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

dgreene1 commented 6 years ago

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

AlexGalays commented 6 years ago

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 commented 6 years ago

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. :)

benschulz commented 6 years ago

As I said in https://github.com/Microsoft/TypeScript/issues/10734#issuecomment-351570480, 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 commented 6 years ago

@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 commented 6 years ago

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

dgreene1 commented 6 years ago

@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 commented 6 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

MartinJohns commented 5 years ago

@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);
}
pie6k commented 5 years ago

@SamHH it's narrowing, but it's doing it safely as we're removing empty values before

samhh commented 5 years ago

@pie6k It's not safe. You can prove this like so:

export function removeEmptyElementsFromArray<T>(array: Array<T | Empty>): T[] {
  return array as T[];
}

This still compiles just as yours did. It's the assertion that's pleasing the compiler; you can remove the isEmpty function's type guard as it's not doing anything. Your safety is not coming from the type system, but it should do.

I don't think there's any way to generically type guard a negative, which is what your example needs to drop the assertion.

benschulz commented 5 years ago

I believe you're talking past one another. removeEmptyElementsFromArray as posted by @pie6k and @MartinJohns are sound. However, small changes to the code may break soundness without warning because the compiler is blinded by explicitly added type assertions. That's what @SamHH is getting at.

This code issue is about inferring type guards and therefore I have to agree with @SamHH that the code above is not a solution: You're still manually adding type annotations.

mikeyhew commented 4 years ago

Referencing https://github.com/microsoft/TypeScript/issues/18122, because this is what is required for the example code there to work, and I can't comment there because it's closed locked.

kasperpeulen commented 4 years ago

This is quite a clean solution for this problem, using type predicates: http://www.typescriptlang.org/docs/handbook/advanced-types.html#using-type-predicates

function isNotNull<T>(it: T): it is NonNullable<T> {
  return it != null;
}

const nullableNumbers = [1, null];
const nonNullableNumbers = nullableNumbers.filter(isNotNull);
eduter commented 4 years ago

@kasperpeulen, if you make this mistake, it still type-checks:

function isNotNull<T>(it: T): it is NonNullable<T> {
  return it === null; // oops...
}

Using a callback with a type predicate is not much safer than using a type assertion on the return value.

And using an arrow function does not look clean at all:

nullableItems.filter((it): it is NonNullable<typeof it> => it !== null)
AlexGalays commented 4 years ago

This is quite a clean solution for this problem, using type predicates: http://www.typescriptlang.org/docs/handbook/advanced-types.html#using-type-predicates

function isNotNull<T>(it: T): it is NonNullable<T> {
  return it != null;
}

const nullableNumbers = [1, null];
const nonNullableNumbers = nullableNumbers.filter(isNotNull);

This is not a solution to this problem since this problem is all about inferring the type guard.

kasperpeulen commented 4 years ago

I wrote this for people that wonder how they can filter all nulls from an array in a way that typescript understand. If you use this isNotNull as a utility function, then I think just writing nullableNumbers.filter(isNotNull) is quite clean.

However, I totally agree that it would be awesome if typescript can infer any type predicates itself.

robertmassaioli commented 4 years ago

To avoid everybody having to write the same helper functions over and over again I bundled isPresent, isDefined and isFilled into a helper library: npm

When Typescript bundles this functionality in I'll remove the package.

To use the library:

import { isPresent, isDefined, isFilled } from 'ts-is-present';

arrayWithUndefinedAndNullValues.filter(isPresent)
arrayWithUndefinedValues.filter(isDefined)
arrayWithNullValues.filter(isFilled)

To see why this package works please read the readme of the NPM package.

pie6k commented 4 years ago

Wow! @robertmassaioli this is cool!

image

function isPresent<T>(t: T | undefined | null | void): t is T {
  return t !== undefined && t !== null;
}

const foo: Array<number | null> = [2,3, null, 4];

const bar = foo.filter(isPresent); // number[]
jtomaszewski commented 4 years ago

Similar to isPresent from ts-is-present library, me and @Smtih just created this utility hasPresentKey fn that helps you filter items down to the ones that have defined non-null item at a[k]. Maybe you'll find it useful.

Source:

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;
  };
}

Edit: This, and other similar functions (hasKey hasDefinedKey hasValueAtKey) hopefully will get released soon in the ts-is-present library ( https://github.com/robertmassaioli/ts-is-present/pull/1 ).

robertmassaioli commented 4 years ago

@jtomaszewski That looks like a good addition to ts-is-present please raise a PR against that repository with the code changes and applicable docs and we'll get this out the door. Cheers!

Update from 21/Sep/2020: Okay, it seems that the above functions don't quite typecheck the way that we would like them to. PR in which we are investigating further here: https://github.com/robertmassaioli/ts-is-present/pull/1

jtomaszewski commented 4 years ago

Here's what bitbucket says to me when I'm trying to open a PR to your repo: image

Any chance moving it to GH?

robertmassaioli commented 4 years ago

@jtomaszewski I have moved the repository to Github. I want you to get credit in the commits so please feel free to try again there: https://github.com/robertmassaioli/ts-is-present

graup commented 3 years ago

Is this also the issue tracking that I would expect these two lines to be the same?

const values = [4, "hello"] as const;
const result1 = values.filter(val => typeof val === 'number');  // (4 | "hello")[]
const result2 = values.flatMap(val => typeof val === 'number' ? val : []);  // 4[]

I understand the issue is that predicate-like functions aren't inferred as type guards, so any type narrowing that might happen inside it isn't available to the context? In this example, of course I can write is number, but that doesn't work with more complex union types. I'd really like to use the inference (which is already happening anyway).

AlexGalays commented 3 years ago

@graup Seems like the same issue yes.

SmileJayden commented 3 years ago

After es2019, you can use flatMap.

Here is my ts-playground.

const myArr = ['hello', null, 'world', undefined];
const filteredArr = myArr.flatMap((x) => x ? x : []);

Above filteredArr is inferred as string[].

fyi: However, because this approach generates empty array whenever meets nullish value, it has critical performance issue. 😓 I recommend you guys to avoid this approach in production code because of this performance issue.

ignlg commented 3 years ago

I tried this approach to filter number keys of an object:

const isNumber = (key: unknown): key is number => 'number' === typeof key;
const options: Record<number | string, string> = { 1: 'test' };
const keys: number[] = Object.keys(options).filter(isNumber);

And TS throws:

Type 'string[]' is not assignable to type 'number[]'.
  Type 'string' is not assignable to type 'number'.ts(2322)

Even with the type guard as filter TS cannot infer the right type. Am I wrong or this is part of this issue?

acutmore commented 3 years ago

Hi @ignlg, Object.keys will never return a number, it returns string[].

In JavaScript all properties of an object are either string or symbol. When using number they are being converted into string.

let map = {};

map[1] = 'hello'; // same as map['1'] = 'hello';

console.log(map['1']); // 'hello'

If you need to use actual numbers as keys, and keep them seperate from strings then Map is a better data structure for this.

let map = new Map<string | number, string>();
map.set(1, 'hello');

console.log(map.get("1")); // 'undefined'

let keys: number[] = [...map.keys()].filter(isNumber);
AlexGalays commented 3 years ago

I tried this approach to filter number keys of an object:

const isNumber = (key: unknown): key is number => 'number' === typeof key;
const options: Record<number | string, string> = { 1: 'test' };
const keys: number[] = Object.keys(options).filter(isNumber);

And TS throws:

Type 'string[]' is not assignable to type 'number[]'.
  Type 'string' is not assignable to type 'number'.ts(2322)

Even with the type guard as filter TS cannot infer the right type. Am I wrong or this is part of this issue?

No, that's an issue with your code. Your typeguard and filter() are working as intended but Object.keys always returns an Array of Strings, never numbers, etc.

vmwxiong commented 3 years ago

Is this something that's possible to support in TS, or will we always have to use assertions/predicates?

MartinJohns commented 3 years ago

@vmwxiong Best issue to track would be #38390. But I wouldn't expect it anytime soon.

haltman-at commented 3 years ago

Hm, so now that TS 4.4 added #44730, could this happen too? They seem at least vaguely related to this non-expert...

JustinGrote commented 3 years ago

Came here from google, just noting that using flatMap is an unintuitive but effective workaround: https://github.com/microsoft/TypeScript/issues/16069#issuecomment-730710964. Return [] for items you want to filter out, and return [myobject] for items you want to keep

vmwxiong commented 3 years ago

Came here from google, just noting that using flatMap is an unintuitive but effective workaround: #16069 (comment). Return [] for items you want to filter out, and return [myobject] for items you want to keep

Interesting, how much of a perf impact do you think this has? I'd imagine generating thousands of arrays just to immediately throw them away might be a little rough.

JustinGrote commented 3 years ago

@vmwxiong it would depend on your environment, I don't know how "cheap" computationally it is in Typescript to create an array, in most languages it is extremely trivial. Benchmarks are going to be the only way to determine.

n1kk commented 3 years ago

I don't know how "cheap" computationally it is in Typescript to create an array

@JustinGrote did you mean JavaScript? TypeScript compiles down to JavaScript so in 99% the environment is NodeJS or a browser.

@vmwxiong result varies depending on the sample count and iterations number but flaptMap is always slower ofc. I've played with some variations in NodeJS and it's not a linear correlation, maybe due to some V8 optimizations.

Here's a sample benchmarking code ```ts import { benchmark } from "@thi.ng/bench"; const sampleSize = 2; const iterations = 10_000_000; const list = Array(sampleSize) .fill(0) .map((_, i) => (Math.random() > 0.5 ? `${i}` : i)); const flatMap = (list: any[]): string[] => list.flatMap(x => (x ? x : [])); const filter = (list: any[]): string[] => list.filter((value: any): value is string => typeof value === "string"); const filterResults = benchmark( () => { const result: string[] = filter(list); }, { iter: iterations } ); const flatmapResults = benchmark( () => { const result: string[] = flatMap(list); }, { iter: iterations } ); const factor = flatmapResults.mean / filterResults.mean - 1; console.log(`filter is ${factor.toFixed(2)} times faster than flatMap`); ```
Here's a result matrix for different sample and iteration sizes sample \ iterations 10 100 1_000 10_000 100_000 1_000_000 10_000_000
10 x1.46 x2.1 x1.38 x0.59 x5.96 x8.55 x8.82
100 x35.37 x31.38 x6.35 x19.43 x25.14 x32.46
1_000 x45.29 x50.64 x49.44 x49.39 x42.73
10_000 x24.6 x19.89 x19.69 x20.42
10_000 x5.72 x5.88 x5.64
MartinJohns commented 2 years ago

I think if you use filter<number[]>(x => !!x); the type of evenSquares will be number[]

No, it will not. It will result in the error:

Type 'number[]' does not satisfy the constraint 'number'.

You probably meant filter<number>, but that will result in the error:

Argument of type '(x: number | null) => boolean' is not assignable to parameter of type '(value: number | null, index: number, array: (number | null)[]) => value is number'. Signature '(x: number | null): boolean' must be a type predicate.

Please verify your suggestions actually work before suggesting them.

robertmassaioli commented 2 years ago

Just in case people are missing this, because it is getting auto-collapsed, this is the comment that you want to look at for a quick and easily solution to this problem: https://github.com/microsoft/TypeScript/issues/16069#issuecomment-565658443

The npm library is currently very popular and solves this problem well:

Screen Shot 2022-07-03 at 9 27 51 am

P.S. I'm glad I created this library, I would not have guessed in late 2019 that in mid 2022 this would still be a problem that was not solved by the TypeScript type checker. I guess it's either a hard problem or a low priority; maybe both.

pedrodurek commented 2 years ago

It'd be nice to also support the in operator for narrowing. For example:

type Foo = {
  foo: string;
}
type Bar = Foo & {
  bar: string;
}

const list: (Foo | Bar)[]  = [...]
const result = list.filter((value) => 'bar' in value);
// result type should be `Bar[]` 
robertmassaioli commented 2 years ago

@pedrodurek I believe that this is already (partially) solved by the npm library linked above with the hasPresentKey function. The docs say the following.

If you want to find all of the objects in an array that have a particular field present, you can use hasPresentKey. For example:

const filesWithUrl = files.filter(hasPresentKey("url"));
 files[0].url // TS will know that this is present

If you want to find all of the objects with a particular field set to a particular value you can use hasValueAtKey:

type File = { type: "image", imageUrl: string } | { type: "pdf", pdfUrl: string };
const files: File[] = <some data here>;

const filesWithUrl = files.filter(hasValueKey("type", "image" as const));
files[0].type // TS will now know that this is "image"

These functions are useful in filtering out objects from arrays.

It was actually @jtomaszewski that provided this function in this very thread: https://github.com/microsoft/TypeScript/issues/16069#issuecomment-666296465

pedrodurek commented 2 years ago

@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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

@infacto You could also just add a type annotation.

infacto commented 1 year ago

@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 commented 1 year ago

@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