-
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
Improvment to Array.isArray
#151
base: main
Are you sure you want to change the base?
Conversation
@mattpocock @DeepDoge @phenomnomnominal I have not been able to make it pass the final test. If you have any suggestions I'd be happy for the help. |
I have been able to get either or of 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);
}
}
});
doNotExecute(async () => {
function makeArray<Item>(input: Item | ReadonlyArray<Item> | Array<Item>) {
if (Array.isArray(input)) {
return input;
}
return [input];
}
const [first] = makeArray([{ a: "1" }, { a: "2" }, { a: "3" }] as const);
// No error!
first.a;
}); to pass but not both at the same time. With interface ArrayConstructor {
isArray<T>(arg: true extends TSReset.IsAny<T> ? never : T): arg is true extends TSReset.IsAny<T> ? never : T extends unknown[] | readonly unknown[] ? T : T[] extends T ? T[] : never;
} the first passes and with interface ArrayConstructor {
isArray<T>(arg: true extends TSReset.IsAny<T> ? never : T): arg is true extends TSReset.IsAny<T> ? never : T extends unknown[] | readonly unknown[] ? T : T[] extends T ? T[] : never;
isArray<T>(arg: T[] extends T ? T | T[] : never): arg is T[] extends T ? T[] : never;
isArray<T>(arg: T): arg is T extends unknown[] | readonly unknown[] ? T : T[] extends T ? T[] : never;
} the second passes. Any help to get both to pass would be greatly appreciated. |
doNotExecute(async () => {
function makeArray<Item>(input: Item) {
if (Array.isArray(input)) {
return input;
}
return [input];
}
const [first] = makeArray([{ a: "1" }, { a: "2" }, { a: "3" }] as const);
// No error!
first.a;
}); In the test above, you get a value, and if that value is an array, you just return it, if its not an array you place it inside an array and return it. So as far as typescript knows that Normally to make it give the right type you would define the function makeArray<Item>(input: Item):
Item extends Array<any> ? Item :
Item extends ReadonlyArray<any> ? Item :
[Item]
{
if (Array.isArray(input)) {
return input as any;
}
return [input] as any;
} So, basically you have to repeat your runtime logic with types. It's a typescript issue, not an issue with As far as I know, but a second opinion won't hurt, maybe I'm missing something. |
Thanks @DeepDoge for your answer. If @mattpocock agrees I could just remove that test and it would be ready from my side. |
src/entrypoints/is-array.d.ts
Outdated
@@ -1,3 +1,3 @@ | |||
interface ArrayConstructor { | |||
isArray(arg: any): arg is unknown[]; | |||
isArray<T>(arg: 0 extends 1 & T ? never : T): arg is 0 extends 1 & T ? never : T extends unknown[] | readonly unknown[] ? T : T[] extends T ? T[] : never; |
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.
Not sure if you saw my comment here -- you can simplify this by removing the check for unknown[]
entirely.
isArray<T>(arg: 0 extends 1 & T ? never : T): arg is 0 extends 1 & T ? never : T extends unknown[] | readonly unknown[] ? T : T[] extends T ? T[] : never; | |
isArray<T>(arg: 0 extends 1 & T ? never : T): arg is 0 extends 1 & T ? never : T extends readonly unknown[] ? T : T[] extends T ? T[] : never; |
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.
@ahrjarrett Thanks, I saw your comment but it slipped my mind while I was sitting with the code
I've tried this and it didn't work for my case. const token: string | readonly string[];
if (Array.isArray(token)) throw new Error("Token is an array");
console.log(token); // token is `string`, not `string | readonly string[]` here (as expected) |
@mattpocock I removed the failing test based on this comment. This PR is now redo to be merged |
interface ArrayConstructor { | ||
isArray(arg: any): arg is unknown[]; | ||
isArray<T>(arg: true extends TSReset.IsAny<T> ? never : T): arg is true extends TSReset.IsAny<T> ? never : T extends ReadonlyArray<unknown> ? T : Array<T> extends T ? Array<T> : never; | ||
} |
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.
Not sure I agree with the decision to disallow any
, since narrowing a value from any
to unknown[]
seems like a valid thing to want to do.
Setting that aside, here's another way you could accomplish the same thing:
interface ArrayConstructor { | |
isArray(arg: any): arg is unknown[]; | |
isArray<T>(arg: true extends TSReset.IsAny<T> ? never : T): arg is true extends TSReset.IsAny<T> ? never : T extends ReadonlyArray<unknown> ? T : Array<T> extends T ? Array<T> : never; | |
} | |
interface ArrayConstructor { | |
isArray<T>(arg: NotAny<T>): arg is Extract<NotAny<T>, readonly unknown[]>; | |
} | |
type NotAny<T> = 0 extends 1 & T ? never : T |
@mattpocock Are the changes in this PR something that will be considered? |
This PR improves the type inference of
Array.isArray
.It builds on the changes made in #56 and #23 to solve #48.