-
Notifications
You must be signed in to change notification settings - Fork 124
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
Fixes to isArray, handled when input is any
#56
Conversation
src/entrypoints/is-array.d.ts
Outdated
@@ -1,3 +1,12 @@ | |||
type AsArray<T> = T[] extends T ? T[] : any[] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you inline this? Otherwise it'll be entered into the global scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!
@mattpocock Found a better solution that returns less complex type for interface ArrayConstructor {
// @ts-ignore
isArray<T>(arg: T[] extends T ? T | T[] : never): arg is unknown[];
} Basically, this ^ only works when the input is I have one question, when the input is Or I can change it to the example below: interface ArrayConstructor {
// @ts-ignore
isArray<T>(arg: T[] extends T ? T | T[] : never): arg is T[];
} This ^ would return:
|
@DeepDoge I'm not sure that we can have Do you have a test case that doesn't pass with the current implementation? |
i mean first solution was passing the tests, down side was. On this test: doNotExecute(() => {
function test<T>(value: T) {
type Unarray<T> = T extends Array<infer U> ? U : T;
const inner = <X extends Unarray<T>>(v: X[]) => {};
if (Array.isArray(value)) {
inner(value);
}
}
});
|
Gotcha, I don't mind too much because especially inside a generic function, it's fairly important to see how |
@mattpocock Wasn't expecting to solve it this quick. Got rid of Current behavior:
Can be merged now. |
isArray(arg: T[] extends T ? T | T[] : never): arg is unknown[]; |
I was messing around with this, and i came up with this one interface ArrayConstructor {
isArray<T>(
arg: T
): T extends Array<unknown> | ReadonlyArray<unknown> ? true : false
}
let array = Array.isArray(123) // false
array = Array.isArray([1, 2, 3]) // true |
@mjxl This PR is for narrowing the type correctly. But, if you ask me if the type of the argument is already known to be not an array, it shouldn't even let you use it as an argument. But we can't do this because it will just fallback to the default |
I'm not sure I totally understand the use case here, but did want to chime in and argue against adding a type parameter to |
@DeepDoge Would it be possible to make this handle readonly arrays as well? doNotExecute(() => {
const arrOrString = [] as readonly string[] | string;
if (Array.isArray(arrOrString)) {
type tests = [Expect<Equal<typeof arrOrString, readonly string[]>>];
}
});
doNotExecute(() => {
const arrOrString = [] as readonly string[] | string;
if (Array.isArray(arrOrString)) return;
type tests = [Expect<Equal<typeof arrOrString, string>>];
}); I have been testing a bit but can't quite figure it out. interface ArrayConstructor {
isArray<T>(arg: T): arg is T extends readonly any[] | any[] ? T : T[] extends T ? T[] : never;
} these tests fail doNotExecute(() => {
const maybeArr = [1, 2, 3] as any;
if (Array.isArray(maybeArr)) {
type tests = [Expect<Equal<typeof maybeArr, any[]>>];
}
});
doNotExecute(() => {
function test<T>(value: T) {
type Unarray<T> = T extends Array<infer U> ? U : T;
const inner = <X extends Unarray<T>>(v: X[]) => {};
if (Array.isArray(value)) {
inner(value);
}
}
}); and with interface ArrayConstructor {
isArray<T>(arg: T[] extends T ? T | T[] : never): arg is T[] extends T ? T[] : never;
isArray<T>(arg: T): arg is T extends readonly any[] | any[] ? T : T[] extends T ? T[] : never;
} only doNotExecute(() => {
function test<T>(value: T) {
type Unarray<T> = T extends Array<infer U> ? U : T;
const inner = <X extends Unarray<T>>(v: X[]) => {};
if (Array.isArray(value)) {
inner(value);
}
}
}); fails but I can't figure out how to make all of them pass. |
Hey @DeepDoge , not sure if this is related or not, but if you're trying to add support for readonly arrays, the easiest way to do that is to treat all arrays like readonly arrays. That's because ReadonlyArray is the supertype -- which is kinda backwards from what most people expect. Here's a sandbox about it: https://codesandbox.io/s/arrays-vs-readonlyarray-in-typescript-xdgsfj?file=/src/array-stuff.ts That's something I wish someone had told me, because I found this behavior very confusing until I figured out why it worked that way. Hopefully that's helpful! |
Although Matt told me to open this PR, I don't think this is gonna get merged, it has been months. I don't wanna deal with this PR anymore. So closing it and deleting the fork. For future reference, here's the code: interface ArrayConstructor {
isArray<T>(arg: T[] extends T ? T | T[] : never): arg is T[] extends T ? T[] : never;
} And tests: import { doNotExecute, Equal, Expect } from "./utils";
doNotExecute(() => {
const maybeArr = [1, 2, 3] as unknown;
if (Array.isArray(maybeArr)) {
type tests = [Expect<Equal<typeof maybeArr, unknown[]>>];
}
});
doNotExecute(() => {
const maybeArr = [1, 2, 3] as unknown[];
if (Array.isArray(maybeArr)) {
type tests = [Expect<Equal<typeof maybeArr, unknown[]>>];
}
});
doNotExecute(() => {
const maybeArr = [1, 2, 3] as any;
if (Array.isArray(maybeArr)) {
type tests = [Expect<Equal<typeof maybeArr, any[]>>];
}
});
doNotExecute(() => {
const maybeArr = [1, 2, 3] as any[];
if (Array.isArray(maybeArr)) {
type tests = [Expect<Equal<typeof maybeArr, any[]>>];
}
});
doNotExecute(() => {
const arrOrString = [] as string[] | string;
if (Array.isArray(arrOrString)) {
type tests = [Expect<Equal<typeof arrOrString, string[]>>];
}
});
doNotExecute(() => {
const arrOrString = [] as string[] | string;
if (Array.isArray(arrOrString)) return;
type tests = [Expect<Equal<typeof arrOrString, string>>];
});
doNotExecute(() => {
let arrOrString = "" as string | [1, 2];
if (Array.isArray(arrOrString)) {
type tests = [Expect<Equal<typeof arrOrString, [1, 2]>>];
}
});
doNotExecute(() => {
let path = [] as string | string[];
const paths = Array.isArray(path) ? path : [path];
type tests = [Expect<Equal<typeof paths, string[]>>];
});
doNotExecute(() => {
function test<T>(value: T) {
type Unarray<T> = T extends Array<infer U> ? U : T;
const inner = <X extends Unarray<T>>(v: X[]) => {};
if (Array.isArray(value)) {
inner(value);
}
}
}); |
Handled the cases when the input is
any[]
orany
and notunknown
orunknown[]
#48 (comment)
You might wanna check if the test are correct.