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

Update: take #70

Closed
wants to merge 1 commit into from
Closed

Update: take #70

wants to merge 1 commit into from

Conversation

Harris-Miller
Copy link
Collaborator

Add curry support and fix issue with collapsing generic for dual function return when only passing a single argument
Added tests

@TClark1011
Copy link

Not sure if this matters for a typing package, but this is technically a breaking change for anyone who was calling the function like this: take(3)<number> since now that would have to be changed to take(3)<number[]>

@Harris-Miller
Copy link
Collaborator Author

Ohh good catch. That's fixable

@TClark1011
Copy link

TClark1011 commented Oct 16, 2023

Just thought of another problem; if the new curried signature is passed a tuple, the return type ends up being that exact tuple, which is not correct. You can see this on the g variable here https://tsplay.dev/w6Q5DW.

This can be fixed by changing the new overload signature to this:

export function take(
  n: number,
): <Collection extends readonly any[] | string>(
  xs: Collection,
) => Collection extends string ? string : Collection[number][];

Proof: https://tsplay.dev/w8Qo9N

This does have an interesting piece of behaviour though; the return type ends up being an array of all the literals that were contained in the tuple, eg; taking an items from a tuple of [1,2,3,4] returns the following (1 | 2 | 3 | 4)[] instead of number[]. Its subjective whether or not this is a bad thing, but this can also be avoided by creating a ToNonLiteral generic type, here is an example: https://tsplay.dev/NlXvrN.

@Harris-Miller
Copy link
Collaborator Author

Your ToNonLiteral we already have as WidenLiteral. You can find it in types/util/tools.d.ts

Both the issues you pointed out with this change make this MR a non-starter. Going to close

@TClark1011
Copy link

Dang, that's unfortunate, I'll keep looking into this, I feel like there's a way to get a perfect solution here somehow

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.

None yet

2 participants