Skip to content

Conversation

@som-sm
Copy link
Collaborator

@som-sm som-sm commented Oct 11, 2025

This PR fixes this issue in #1167. I was playing around with the fix and had done most of the work in the process, so I thought I’d open a separate PR.

Credits to @benzaria for suggesting this type.

Reverse works normally when the input array doesn't contain optional elements, but if the array includes an optional element, the result splits into a union of separate tuples, like:

Note: If the tuple contains optional elements, the result will be a union of tuples, refer to the examples below:
@example
```ts
import type {ArrayReverse} from 'type-fest';
type A = ArrayReverse<[string, number, boolean?]>;
//=> [boolean, number, string] | [number, string]
type B = ArrayReverse<[string, number?, boolean?]>;
//=> [boolean, number, string] | [number, string] | [string]
type C = ArrayReverse<[string?, number?, boolean?]>;
//=> [boolean, number, string] | [number, string] | [string] | []
type D = ArrayReverse<[string, number?, ...boolean[]]>;
//=> [...boolean[], number, string] | [string]
type E = ArrayReverse<[string?, number?, ...boolean[]]>;
//=> [...boolean[], number, string] | [number, string] | [string] | []
```


I've kept the name as ArrayReverse instead of just Reverse to match with our existing Array* types (like ArraySlice, ArraySplice).

@claude

This comment was marked as resolved.

@som-sm som-sm force-pushed the feat/add-reverse-type branch from 0e9cfea to af7ed2a Compare October 11, 2025 15:49
@claude

This comment was marked as outdated.

@claude

This comment was marked as outdated.

@som-sm som-sm requested a review from sindresorhus October 11, 2025 15:56
@som-sm
Copy link
Collaborator Author

som-sm commented Oct 11, 2025

For inputs containing only optional elements, I originally wasn't splitting the result, for example, ArrayReverse<[1?, 2?, 3?]> was returning [3?, 2?, 1?]. While this type of output is accurate, it's not precise, because [3?, 2?, 1?] could be [3] | [3, 2] but reversing [1?, 2?, 3?] would never yield [3] or [3, 2].

@som-sm
Copy link
Collaborator Author

som-sm commented Oct 11, 2025

@benzaria Would appreciate if you could share your thoughts on this!

@som-sm som-sm force-pushed the feat/add-reverse-type branch from 7e59b1b to 5aaf11d Compare October 11, 2025 18:59
@claude

This comment was marked as outdated.

@som-sm som-sm mentioned this pull request Oct 12, 2025
@som-sm
Copy link
Collaborator Author

som-sm commented Oct 12, 2025

@sindresorhus While adding test for long tuples, I realised my implementation was not tail recursive because I had simplified a conditional using the If type. While this mistake is obvious, it might get missed. Refer #6cc3e53.

So, does it make sense to add a note like the following to the If type or is it too trivial?

Note: Sometimes using the If type can make an implementation non tail recursive, which would affect the performance of the implementation. In such cases, it's better to use a conditional directly. Refer the following example:

import type {If, IsEqual, StringRepeat} from 'type-fest';

// The following implementation is not tail recursive
type Includes<S extends string, Char extends string> =
	S extends `${infer First}${infer Rest}`
		? If<IsEqual<First, Char>,
			'found',
			Includes<Rest, Char>>
		: 'not found';

// Hence, instantiations with long strings will fail
type HundredZeroes = StringRepeat<'0', 100>;

// @ts-expect-error
type Fails = Includes<HundredZeroes, '1'>;
//           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Error: Type instantiation is excessively deep and possibly infinite.

// However, if we use a simple conditional instead of `If`, the implementation becomes tail-recursive
type IncludesWithoutIf<S extends string, Char extends string> =
	S extends `${infer First}${infer Rest}`
		? IsEqual<First, Char> extends true
			? 'found'
			: IncludesWithoutIf<Rest, Char>
		: 'not found';

// Now, instantiations with long strings will work
type Works = IncludesWithoutIf<HundredZeroes, '1'>;
//=> 'not found'

@sindresorhus
Copy link
Owner

So, does it make sense to add a note like the following to the If type or is it too trivial?

Definitely makes sense 👍

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

Successfully merging this pull request may close these issues.

4 participants