Skip to content

T[] still inferred as (T | undefined)[] even after filtering out undefined entries explicitly #45097

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

Closed
y-nk opened this issue Jul 19, 2021 · 13 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@y-nk
Copy link

y-nk commented Jul 19, 2021

Bug Report

πŸ”Ž Search Terms

filter Boolean undefined

πŸ•— Version & Regression Information

  • Latest version

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type Model = { id: string }

const models: Model[] = [{ id: '0' }, { id: '1' }, { id: '2' }]

const ids = ['1', '3']

const values = ids
    .map(id => models.find(model => model.id === id))
    .filter(model => model !== undefined) // values type should be Model[]

πŸ™ Actual behavior

values is of type (Model | undefined)[] even though undefined values are excluded thanks to the filter function

πŸ™‚ Expected behavior

values should be of type Model[]

@y-nk y-nk changed the title Array type still includes T | undefined even after filtering out undefined entries explicitly Array T[] still inferred as (T | undefined)[] even after filtering out undefined entries explicitly Jul 19, 2021
@y-nk y-nk changed the title Array T[] still inferred as (T | undefined)[] even after filtering out undefined entries explicitly T[] still inferred as (T | undefined)[] even after filtering out undefined entries explicitly Jul 19, 2021
@MartinJohns
Copy link
Contributor

This is working as intended. Your arrow function passed to filter is not a type-guard and is not used to narrow the type. It's just a function returning a boolean.

You can make the function a type-guard by adding an explicit type annotation: .filter((model): model is Model => model !== undefined)

You're also interested in #38390.

@y-nk
Copy link
Author

y-nk commented Jul 19, 2021

@MartinJohns I'm sorry but... how's that acceptable to be "working as intented"?

As I understand Typescript should work on top of Javascript, and obviously it isn't. The typing of the filter's predicate function is obviously the culprit here, instead of having the return value to be unknown it should be boolean (or more precisely, as stated in the specs, "a value that coerces to true to keep the element, or to false otherwise").

You're asking me to do:

const isModel = (model: any) model is Model => model !== undefined
array.filter(isModel)

but actually there are 2 issues with this:

  1. Testing something is undefined doesn't mean it's of a particular shape (Model). As far as I know, isModel({ iAmNotAModel: "nope" }) will return a Model which is invalid, so this looks like a mis-use of the type guard feature to me.

  2. If a simple type guard fixes the typing issue, why then the filter's predicate function isn't typed similarly than a type guard natively? Why is the work on userland?

@MartinJohns
Copy link
Contributor

MartinJohns commented Jul 19, 2021

how's that acceptable to be "working as intented"?

I'd say it's actually more a design limitation. But the last word lies with the TypeScript team.

As I understand Typescript should work on top of Javascript, and obviously it isn't.

I'm not sure what your point is. It is working on top, your issue is purely related to TypeScripts type-system. This is no different than trying to assign a string to a number variable.

The typing of the filter's predicate function is obviously the culprit here, instead of having the return value to be unknown it should be boolean (or more precisely, as stated in the specs, "a value that coerces to true to keep the element, or to false otherwise").

The typing of the predicate function is fine. If you pass a type-guard, then it will narrow the type accordingly. But just assuming any returned boolean means the type can't be undefined is a wrong assumption. A returned boolean could mean anything, and can't be used to narrow the types.

You're asking me to do:
const isModel = (model: any) model is Model => model !== undefined

This is not what I suggested. In your example the model variable is typed any, but in my suggestion the variable is typed Model | undefined. So the value can either be Model, or it can be undefined, and we're just excluding the undefined case.

Testing something is undefined doesn't mean it's of a particular shape (Model). As far as I know, isModel({ iAmNotAModel: "nope" }) will return a Model which is invalid, so this looks like a mis-use of the type guard feature to me.

I commonly have a isDefined function that I just use, which just excludes the undefined from the type. It's just excluding undefined from a union type. It can't be used to wrongly infer the type a of a model.

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

// Then use it:
.filter(isDefined)

If a simple type guard fixes the typing issue, why then the filter's predicate function isn't typed similarly than a type guard natively? Why is the work on userland?

See above. It is working if you pass a type predicate, but a function returning boolean is not a type predicate. You can't make safe assumptions regarding the type-system from just a returned boolean value.


I'd like to point you again to the issue #38390. This feature requests asks for TypeScript to automatically infer simple arrow functions to type-predicates instead of functions returning boolean.

@y-nk
Copy link
Author

y-nk commented Jul 19, 2021

I'd say it's actually more a design limitation.

But it's not working as intended...

...especially as per Javascript's filter function definition in the original spec. From Javascript's PoV, a type guard is just any other function and the fact that we need to arrange our code to make it work with Typescript breaks the promise of "All valid JavaScript code is also TypeScript code." which is written on the Typescript's website home page.

Screenshot 2021-07-19 at 17 02 48 png

If anything, this workaround (type guards being built in JavaScript) is just coincidental and just masks the fact that we can't make use of javascript as we could naturally do in standard javascript. I'm basing this reflection on the obligation that, when filling for a feature request, we are to tick boxes such as:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code

...and to be fair passing a type guard function in the .filter does not change the runtime behavior of existing JavaScript code, although not being able to pass an anonymous function does limit the scope of what's possible with that filter function, therefore changing the runtime behavior of JavaScript in its entirety.

This is not what I suggested.

I'm genuinely sorry for overseeing this one, but still doesn't make a solid point for why the TS team should not solve the issue. I understand you guys may have bigger/more interesting problems at heart, but following the ecma spec should be a first priority which isn't followed at the moment (imho)

See above. It is working if you pass a type predicate, but a function returning boolean is not a type predicate. You can't make safe assumptions regarding the type-system from just a returned boolean value.
[...]
I'd like to point you again to the issue #38390. This feature requests asks for TypeScript to automatically infer simple arrow functions to type-predicates instead of functions returning boolean.

Thank you for pointing me to this. I understand how you can see it related but to me it's just besides the point.

Point being that the filter function's definition from the ecma spec is clear: the predicate should return a value which can be coerced as a boolean, and that's all about it. There's no concept of type guards, so while i can understand that model is Model is somehow coercible to a boolean value, you also have to admit that true, 1, 0, undefined and null are also valid boolean coercible candidates, and returning these does break valid Javascript TS-decorated code.

The issue i'm trying to raise is about the fact that the predicate function's possible returned type should be a boolean (or coercible boolean), and that the filter's return type should be expanded according to the type of the array itself and the predicate function's body.

To be more in the solution-making mindset, as a TS user I'd expect that if Typescript is smart enough to narrow down types in code blocks based on inline expressions such as (this)
:

const foo: string | undefined = "test"

if (foo !== undefined) {
  // type of foo is "string", not "string | undefined"
  foo.toLowerCase() // no need for `?`
}

...then this same logic could be applied to compute the returned type of the array as a result from the filter function, ultimately understanding that if an array of type (string | undefined)[] is filtered by a item !== undefined expression, the resulting type should be of string[].

@MartinJohns
Copy link
Contributor

MartinJohns commented Jul 19, 2021

...especially as per Javascript's filter function definition in the original spec. From Javascript's PoV, a type guard is just any other function and the fact that we need to arrange our code to make it work with Typescript breaks the promise of "All valid JavaScript code is also TypeScript code." which is written on the Typescript's website home page.

... It helps if you continue reading: "You might get type-errors, [...]" - This is what you get here: A type error. I don't understand why you continue to cite JavaScript or the ECMA spec when this is purely a TypeScript issue. Your code will compile (assuming noEmitOnError is turned off) and run as you expect it.

See also this answer on StackOverflow by Ryan.

Point being that the filter function's definition from the ecma spec is clear: the predicate should return a value which can be coerced as a boolean, and that's all about it. There's no concept of type guards, so while i can understand that model is Model is somehow coercible to a boolean value, you also have to admit that true, 1, 0, undefined and null are also valid boolean coercible candidates, and returning these does break valid Javascript TS-decorated code.

Of course there is no mention of a type predicate, because that's a TypeScript construct. And what you cite here is still completely unrelated to the issue at hand.

Change your code to this:

.filter(model => false)

Should the type now still be Model[]? You're still returning a boolean, after all. Just as you're doing now, no difference.

The point that still stands is that TypeScript can't make type-assumptions from a function returning a boolean. Instead TypeScript has an additional construct: type-guard functions. This is the solution I've been suggesting you. Using a type-guard (or often called type predicate) you instruct TypeScripts type-system that your function ensures a variable matches a specific type. This is purely compile-time for the TypeScript compiler.

By providing a type-guard that says "this value is never undefined" the compiler can safely eliminate the undefined type from your union. But just providing a function that returns a boolean doesn't help the compiler at all. The boolean could represent anything.

...then this same logic could be applied to compute the returned type of the array as a result from the filter function, ultimately understanding that if an array of type (string | undefined)[] is filtered by a item !== undefined expression, the resulting type should be of string[].

... Which is exactly what the issue I linked covers. Currently model => model !== undefined is inferred to be a function returning a boolean, with no further type-information. As a result filter() has no relevant information about the types, so the type can not be narrowed. With the linked issue your function would be inferred to be typed (model: Model | undefined): model is Model, which would instruct the TypeScript compiler that the type is narrowed and can't be undefined. The filter() function has an overload that accepts these type-guard functions and narrows the resulting type accordingly.

@nicu-chiciuc
Copy link

For cases like these we have a predefined generic type-guard:

export function notEmpty<TValue>(value: TValue | null | undefined): value is TValue {
  return value !== null && value !== undefined;
}

then you can just

something.filter(notEmpty)

which seems like a better approach than ad-hoc type-guards.

@MartinJohns
Copy link
Contributor

@nicu-chiciuc Just as I suggested earlier already, but it didn't satisfy him. :-)

@y-nk
Copy link
Author

y-nk commented Jul 20, 2021

@MartinJohns @nicu-chiciuc

I'm probably lost in deep technical concepts of the TS language/compiler, so please excuse my following questions if they seem stupid πŸ™ Please trust that I'm not hostile and i genuinely appreciate the time you spend in answering, but if i keep going if mainly because i don't understand why you're pushing back in "use a type guard" when it should be fixed on TS's side. To be clear it's not my intent to troll this thread (in case you might think so πŸ™ ). I'm just overall puzzled on why this is not working as it should from the start ; why do TS users have to bare creating a spare function as a workaround for something that works fine in pure JS? (and why do i need to waste time to explain to my more junior devs "yeah we do like this because... it's a TS thing πŸ€·β€β™‚οΈ" for which they responded a equally puzzled "ah... ok... πŸ‘€")


I don't understand why you continue to cite JavaScript or the ECMA spec when this is purely a TypeScript issue.

I do understand your point being about this being Typescript issue (and overall i do agree, it is a Typescript issue otherwise i wouldn't report it in this repo) ; I keep bringing it because the Typescript issue you're describing brings a breaking change compared to what's naturally possible in JS, so i think it's necessary for the conversation to bring in the JS spec to explain why it should be fixed (it's context at this point). Saying "no biggie use a type guard" isn't good enough imho. It is a Typescript issue which breaks what we're normally able to do in JS, and therefore it should be addressed (especially .filter being a method of array, which a basic very used data structure) and so reminding of the spec helps to remind what is the natural expectations of the language.


.filter(model => false)
Should the type now still be Model[]? You're still returning a boolean, after all. Just as you're doing now, no difference.

If there's no type narrowing produced by the filter function i don't see why the type of the array should be changed. A const model: Model[] = [] is still an array of type Model[] no matter how big/small it is populated, but a const model: (Model | undefined)[] = [] with .filter(model => model !== undefined) is reasonably expected to return a Model[] or am i not understanding what a filter function does?


The point that still stands is that TypeScript can't make type-assumptions from a function returning a boolean.

I understand that, and i understand that's what Type guards are supposed to do, but in the end if TS can do type inference based on an expression in in an inline if statement, why it's not possible to do the same in a function? Is #38390 proposing to reuse what's currently use for if blocks type inference as a broader scope (because by reading it seems similar but different by nature)?


Thank you overall.

@gabrielricardourbina
Copy link

@y-nk A bit ago I implemented a typeguard, isDefined, as a part of a libray, that checks that a given value is not undefined

  const isDefined = (value: unknown): value is {} | null => value !== undefined;

if curious if it works you can check the tests

@y-nk
Copy link
Author

y-nk commented Jul 20, 2021

@GabrielUrbina thanks for this :) it has been suggested by @MartinJohns and @nicu-chiciuc before ; my following comments are about understanding why this bug should be fixed in userland and not implemented by default in Typescript since it's a thing we can do natively in JavaScript

@gabrielricardourbina
Copy link

I share your opinion, that it would be great to have that inference in the filter method, however, I see it as a missing feature, due to design limitation, rather than a defect.

This limitation is why I created a library for making it easier to handle.

On the bright side, I think we are moving to a direction where this will be possible, in TS 4.4 we can have these alieased conditions on variables

@fatcerberus
Copy link

fatcerberus commented Jul 20, 2021

The specific design limitation here is that, even if TS could infer type predicates, model => model !== undefined can't be automatically inferred as a type predicate because that would require negated types (i.e. model is NOT undefined). The isDefined() workaround uses a generic function to accomplish this; inferring the type parameters of a generic function is whole other can of worms from inferring the return type of a regular function and TS simply doesn't support that kind of higher-level reasoning.

When you do this using an if, the narrowing is done directly by control flow analysis, but doing it through filter requires it to be done at the type level. They are completely separate mechanisms.

@andrewbranch andrewbranch added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jul 21, 2021
@y-nk
Copy link
Author

y-nk commented Jul 22, 2021

@fatcerberus Thanks for explaining the if part ; i don't doubt it's a tedious issue ; i hope to see it coming soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants