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

Array.isArray problem with narrowed type #48

Open
plehnen opened this issue Feb 22, 2023 · 9 comments
Open

Array.isArray problem with narrowed type #48

plehnen opened this issue Feb 22, 2023 · 9 comments

Comments

@plehnen
Copy link

plehnen commented Feb 22, 2023

Hi, I recently saw your YT video and wanted to add ts-reset to our project. But there is an issue which I find difficult to overcome:

Basically the problem is that isArray is always adding the type unknown[] to my variable, which I know is of type T | T[].
So when I check whether it's an array, I should know it's T[], but TS now thinks it's T[] & unknown[] which is a problem.

I tried to replicate it here:
https://www.typescriptlang.org/play?ts=4.9.5#code/JYOwLgpgTgZghgYwgAgIJSnAngYQPYgDOYUArgmHlMgN4BQyywh6mWAFHFAOYBcycEFgCU-LtyaFkpEAGsQeAO4gA2gF0A3HQC+dOmCwAHFAFUQXNgB4AKgD5kAXmTXkEAB6QQAEymtsl0BhoZBN7AH4Q5H5rLToYGQpgAmRIYhtbdgA3OAAbUgho4Vo6AEhShAJiJhAQYKdLAA1XDwhvKTMLfzsMzP4G9SKHe3oSkoB6MeRS3TKS4Bhkdj8sADpmZazc-OEikbma6E28iGEtEt1tIA

Any advice maybe? :)

mattpocock added a commit that referenced this issue Feb 22, 2023
@mattpocock
Copy link
Collaborator

Getting somewhere in #51, but it's nasty. Need a bit of help here.

@plehnen Test looks correct to me, well-caught. This is likely why the definitions themselves return any[].

@DeepDoge
Copy link

DeepDoge commented Feb 22, 2023

@mattpocock Unarray is creating a new array from a type.
but the generic T is extending array, its might not only be an array.

So T can be for example number[] & { foo: string }
Which in that case inner would expect number[], which would not work. Playground

Array.isArray(Object.assign([], { foo: "bar" })) would return true
type _ = number[] & { foo: string } extends number[] ? true : false would also give true

@mattpocock
Copy link
Collaborator

@DeepDoge Your meaning is unclear from the comment above. I don't see anything wrong with OP's issue.

@DeepDoge
Copy link

DeepDoge commented Feb 22, 2023

@mattpocock well then this fixes OP's issue:

isArray<T>(arg: T | T[]): arg is T[] extends T ? T[] : any[];

it takes advantage of these returning true

type _ = any[] extends any ? true : false
type _ = unknown[] extends unknown ? true : false 

while these returns false

type _ = number[] extends number ? true : false
type _ = string[] extends string ? true : false 
type _ = {}[] extends {} ? true : false 

It passes all test.
If the input is any which is what a generic type by default is, it will return any[]
If the input it unknown it will return unknown[]

@mattpocock
Copy link
Collaborator

@DeepDoge Amazing work! Truly, thanks for your patience and skill in figuring this out. Feel free to PR (I'd love your name on the contributors list) and we can ship this.

@guillaumebrunerie
Copy link

I'm confused, if Array.isArray returns true on a variable of type T | T[], it should not narrow the type of the variable to T[] but to unknown[], because it can very well be the case that T itself is unknown[].
For instance if T happens to be number[] and we apply Array.isArray to an array of numbers, narrowing its type to T[] (that is number[][]) is just wrong.

@DeepDoge
Copy link

DeepDoge commented Feb 23, 2023

@guillaumebrunerie
When you do .isArray(number[]) or .isArray(number), T is a number
number doesn't extend number[] so arg is any[] not T[], which is the default behavior, makes the final type number[]

if input is unknown or unknown[], T is a unknown, unknown[] extends unknown, so arg is T[] which is unknown[]
Similarly:
if input is any or any[], T is a any, any[] extends any, so arg is T[] which is any[]

Basically, all this is doing is if the input is unknown or unknown[] it gives unknown[] and not any[], that's all, everything else is the same default behavior.

if you have a function like:

 foo<T>(foo: T | T[]): T

when you give it number or number[] it returns number

@plehnen
Copy link
Author

plehnen commented Feb 28, 2023

So can the PR #56 be merged now?
I'd love to get those changes and start using ts-reset :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants