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

R.prepend/R.append fns accept broader types #699

Merged

Conversation

Peeja
Copy link
Contributor

@Peeja Peeja commented Aug 7, 2023

Before, R.prepend(4) or R.append(4) would produce a function which could only be applied to a number[]. However, you may well want to append a 4 to an array whose elements can be numbers or other things, such as a (string | number)[]. This commit improves the types to accept such arrays.

You may also have an array of a more specific type (such as number[]) and wish to use R.prepend/R.append to expand the type. Because it's usually a mistake, append("d")(listOfNumbers) will cause a type error; however, explicitly calling append("d")<string | number>(listOfNumbers) will give you a (string | number)[].

Before, `R.prepend(4)` or `R.append(4)` would produce a function which
could only be applied to a `number[]`. However, you may well want to
append a `4` to an array whose elements can be `number`s or other
things, such as a `(string | number)[]`. This commit improves the types
to accept such arrays.

You may also have an array of a more specific type (such as `number[]`)
and wish to use `R.prepend`/`R.append` to expand the type. Because it's
usually a mistake, `append("d")(listOfNumbers)` will cause a type error;
however, explicitly calling `append("d")<string |
number>(listOfNumbers)` will give you a `(string | number)[]`.
@Peeja Peeja force-pushed the append-prepend-work-with-broader-types branch from b847bac to f6b06b1 Compare August 7, 2023 17:12
Not clear why this changed. Ideally, the test runner wouldn't care about
the order of keys in an object type.
@selfrefactor selfrefactor merged commit 3f6d35f into selfrefactor:master Aug 7, 2023
2 checks passed
@selfrefactor
Copy link
Owner

Thanks for this PR.

I'd change few things before releasing:

  • Introducing TElement/TNewElement while correct in the context of R.append deviates from the rest of types, so I will make it more generic.

  • I'll check again if change in R.zipObj test is required.

@Peeja
Copy link
Contributor Author

Peeja commented Aug 8, 2023

Sounds good! I went with that sort of naming because I kept losing track of what was what otherwise, but please do adjust it as you see fit. 🙂

@Peeja Peeja deleted the append-prepend-work-with-broader-types branch August 16, 2023 15:22
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