-
-
Notifications
You must be signed in to change notification settings - Fork 653
Add ArrayReverse type
#1167
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
base: main
Are you sure you want to change the base?
Add ArrayReverse type
#1167
Conversation
|
Co-authored-by: Sindre Sorhus <[email protected]>
|
Hey @sindresorhus, it's been a while. |
Co-authored-by: Sindre Sorhus <[email protected]>
Co-authored-by: Sindre Sorhus <[email protected]>
| Head extends UnknownArray = [], | ||
| Tail extends UnknownArray = [], | ||
| > = | ||
| keyof Array_ & `${number}` extends never // Is `Array_` leading a rest element or empty |
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.
Would be useful with a short code comment that describes the algorithm.
test-d/reverse.ts
Outdated
| // Edge cases | ||
| expectType<Reverse<[]>>([]); | ||
| expectType<Reverse<any>>(any); | ||
| expectType<Reverse<never>>(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.
| expectType<Reverse<never>>(never); | |
| expectType<Reverse<never>>(never); | |
| expectType<Reverse<[unknown]>>([{} as unknown] as const); |
|
#1166 (comment) was merged. You mentioned you needed it for this. |
test-d/reverse.ts
Outdated
| // Optional/undefined | ||
| expectType<Reverse<[1?, 2?, 3?]>>([3, 2, 1] as const); | ||
| expectType<Reverse<[1?, 2?, 3?], {preserveOptionalModifier: true}>>({} as [3?, 2?, 1?]); | ||
| expectType<Reverse<[1, 2?, 3?], {preserveOptionalModifier: true}>>({} as [3 | undefined, 2 | undefined, 1]); |
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.
Behaviour with optional elements is not correct, you can't return [3 | undefined, 2 | undefined, 1] for Reverse<[1, 2?, 3?]> because at runtime input could be [1, 2] and reversing it would produce [2, 1] which wouldn't conform to [3 | undefined, 2 | undefined, 1].
So, the output for cases containing optional elements needs fixing.
…scart`ReverseOptions` and update JsDoc
I’ve managed to:
If there are any missing tests or additional scenarios worth covering, feel free to list or add them. |
@benzaria Please check #1266 (comment). |
@som-sm I took a note of that and I fixed the problem. |
source/reverse.d.ts
Outdated
| @category Array | ||
| */ | ||
| export type Reverse<Array_ extends UnknownArray> = |
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.
Better to name the type ArrayReverse as mentioned in #1266.
I've kept the name as
ArrayReverseinstead of justReverseto match with our existingArray*types (likeArraySlice,ArraySplice).
test-d/reverse.ts
Outdated
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.
Cases like labelled tuples, non-tuple arrays seem missing. Can you once cross-check from the tests in #1266.
ArrayReverse- Reverses the order of elements in a tuple or array type..