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

.filter should narrow down output type #20812

Closed
ArashMotamedi opened this issue Dec 20, 2017 · 13 comments
Closed

.filter should narrow down output type #20812

ArashMotamedi opened this issue Dec 20, 2017 · 13 comments
Labels
Duplicate An existing issue was already created

Comments

@ArashMotamedi
Copy link

TypeScript Version: 2.6.2

The output of filter can be a further constrained/narrowed-down type. For example, you can use filter to find items in an array that have a certain property. TS is great at inferring these constraints within the filter predicate, but should also carry the inference forward into filter results. See usage of filter and map in the example below:

Code

interface A { text?: string }
let array: A[] = [{}, {text: "hello"}, {}, {text: "world"}];
let lengths = array.filter(e => e.text).map(e => e.text.length);
                                                 ^^^^^^
// [ts] Object is possibly 'undefined'.
// (property) A.text: string | undefined

Expected behavior:
The output type of filter should be {text: string} because TS correctly understands the filter predicate: only elements with text property are returned.

Actual behavior:
The type output of filter is still {text?: string}.

@studds
Copy link

studds commented Dec 20, 2017

If you use a user defined type guard in combination with explicit types, you can do this. I also find it useful to use discriminated unions. I feel like it ought to be possible to cut this example down a bit further, but this is what I could put together quickly

interface A1 { }
interface A2 { text: string }
type A = A1 | A2;
const array: A[] = [{}, { text: "hello" }, {}, { text: "world" }];
const a2s: A2[] = array
    .filter((e): e is A2 => e.hasOwnProperty('text'));
const lengths = a2s
    .map(e => e.text.length);

// no compile errors

@ghost
Copy link

ghost commented Dec 20, 2017

Duplicate of #5101 -- we won't infer that e => e.text is a type predicate without an annotation (e): e is { text: string } => e.text.

It may be easier to define a mapDefined helper like so:

function mapDefined<T, U>(array: ReadonlyArray<T>, mapFn: (x: T, i: number) => U | undefined): U[] {
    const result: U[] = [];
    for (let i = 0; i < array.length; i++) {
        const mapped = mapFn(array[i], i);
        if (mapped !== undefined) {
            result.push(mapped);
        }
    }
    return result;
}

interface A { text?: string }
let array: A[] = [{}, {text: "hello"}, {}, {text: "world"}];
let lengths = mapDefined(array, e => e.text === undefined ? undefined : e.text.length);

then there is only a single closure and type guards will work.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Dec 20, 2017
@JannesMeyer
Copy link

JannesMeyer commented Dec 26, 2017

Shouldn't it be possible to leave off the explicit type on the result of .filter(), though?

interface A1 { }
interface A2 { text: string }
type A = A1 | A2;
const array: A[] = [{}, { text: "hello" }, {}, { text: "world" }];
const a2s = array
    .filter((e): e is A2 => e.hasOwnProperty('text'));
const lengths = a2s
    .map(e => e.text.length);

// The type of a2s is {}[]

It should be automatically typed like this:

const a2s: A2[];

@ghost
Copy link

ghost commented Jan 2, 2018

@JannesMeyer In typescript@next it is.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@seanpoulter
Copy link

Where should I add that this explicit type guard is required in the docs? We really shouldn't have to turn to GitHub to figure something like this out ...

Duplicate of #5101 -- we won't infer that e => e.text is a type predicate without an annotation (e): e is { text: string } => e.text.

@dfee
Copy link

dfee commented May 17, 2019

Hello friends (who've come here before me and discovered how to do this!). Like @seanpoulter, I feel this should be documented somewhere.

For anyone coming after me, here's a practical example of how you should use something like filter:

const getNumbers = (v: Record<string, number | string>): number[] =>
  Object.keys(v)
    .map<number | string>(k => v[k])
    .filter<number>((i: number | string): i is number => typeof i === "number");

Now I'm not clear on why this is the case. Perhaps someone could enlighten us?

[edit]
I guess you could techinically drop the explicit types on map and filter:

const getNumbers = (v: Record<string, number | string>): number[] =>
  Object.keys(v)
    .map(k => v[k])
    .filter((i): i is number => typeof i === "number");

@seanpoulter
Copy link

I'd say we should add a note on the Type Inference section of the TypeScript Handbook. Let's ask the contributors of that section if they'd welcome a PR. Would you review a PR @DanielRosenwasser, @arthanzel, @RyanCavanaugh, @vomvoru, and @karptonite? Would you rather it go in Type Inference or Advanced Types?

@narkowicz
Copy link

@dfee No claim to enlightenment, but I think the core problem is summed up quite well in the original issue description and in #10734 - type narrowing doesn't flow through scopes.

The issue is unrelated to the implementation of filter which, as in your example, correctly narrows types when a type predicate is declared as the return type.

The problem is that functions never infer a type predicate as a return type. So the below isNumber function infers a return type of boolean rather than a is number and can't be used for type narrowing (consumers of isNumber correctly do not inspect the implementation, only the signature).

function isNumber(a: any) {
  return typeof a === 'number';
}

The situation is the same for inline arrow functions commonly used with filter.

What we're effectively asking for is that the type narrowing provided by an if statement (which can be quite complex) be flowed through as an inferred predicate return type when a function returns that same statement (which otherwise equates to a boolean). I can't imagine it's trivial to implement, but it would be very handy.

In such a world, the below isCardinalNumber function would also infer a return type of a is number and support type narrowing if used with filter.

function isCardinalNumber(a: any) {
  return isNumber(a) && a >= 0 && a%1 === 0;
}

@JamesMcMahon
Copy link

For those not wanting to track through multiple closed duplicate issues, this issue/discussion appears to be alive at #16069.

@SmileJayden
Copy link

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[].

@naivefun
Copy link

in latest ts document there is the solution:
https://www.typescriptlang.org/docs/handbook/advanced-types.html

image

Offroaders123 added a commit to Offroaders123/Librar.io that referenced this issue Dec 27, 2022
Well, kinda. It's JSDoc JavaScript, haha. Now that I have editor type-safety, I redid a buch of the code where it was starting to stink XD

Found some confusing recursive function calls along the way, and having type checking made it much easier to tell what the kinds of data I was working with. Some examples of this were the interactions between `readdir()` and `dirtree()`, which had some extra handling and recursive calls that weren't needed anymore. I removed the `Array.prototype.flat(Infinity)` calls alongside that too, and that helped slim things down. I wasn't using the recursive option anywhere in the codebase, so it didn't have to be around anymore. It was making those two functions more complicated than they had to be, too, and now it's more straightforward as to what they each do.

I also simplified the `DirTree` interface, now it only has the `value` property, rather than two optional properties, `value` and `handle`, which would appear in separate situations. Now it's simply just a union type!

I love it! Opening up this codebase again is really cool, I've learned a lot about a ton of different things since I started making this, so now I'm thinking of how I can build this app in a different way now, in a more TypeScript-y way or something, haha.

This was my first ESM-based web app structure, so I had only scratched the surface of breaking up the different app components into modules. Now I have a much better idea of what should be grouped together, and what shouldn't. It also makes way more sense of how to hook differnt things together, and how you should break them apart. This was a big slow down for STE I think, looking back at it now. I would write big functions that do a lot of things, and then call each of those from the top level. If the stuff the function did had to be used somewhere else, I had to add a different access point into the function. Now I try to build my things more modularly, so then you can use the smaller parts in other places, rather than only on the inside of the implementation. Hopefully that makes sense? I think that's one of the best things I've learned with ESM actually, how to break up your code into recyclable pieces, rather than having to tape together the outside so it can fit into tons of different locations. Something like that! (Thanks, Marco)

Learned about how to narrow types with `Array.prototype.filter()` and TypeScript/JSDoc! It's not too complex on the TypeScript side, since you have Generic parameters, but I couldn't figure out how to pass a type param to a `.filter()` call using JSDoc. These links present a very similar way to narrow the type returned by the `.filter()` call, it's extra smart!

microsoft/TypeScript#20812 (comment)
https://www.typescriptlang.org/docs/handbook/advanced-types.html#using-type-predicates (Just the TS docs for Type Predicates)
https://stackoverflow.com/questions/43118692/typescript-filter-out-nulls-from-an-array/51577579#51577579

I was trying to do the same thing with something like `(fileHandle: FileSystemHandle) => fileHandle is not null`, but `is not` isn't a thing you can do. The inverse works just the same, and it's actually more explicit! `(fileHandle: FileSystemHandle) => fileHandle is FileSystemHandle` Yay!
@mahmoudmoravej
Copy link

After es2019, you can use flatMap.

Although flatMap fixes the issue, but it is using a wrong tool for a specific problem: We fix typecheck error, with using an unneeded call to flat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests