search

selfrefactor / rambdax

Extended version of Rambda
https://selfrefactor.github.io/rambdax
MIT License
221 stars 26 forks source link

`is()` changed its behavior between 0.12.x and 0.13 (and is confusing) #23

Closed radex closed 5 years ago

radex commented 5 years ago

Hey! We use rambdax quite extensively in https://github.com/Nozbe/WatermelonDB and between 0.12.x and 0.13, it's been breaking one of the tests.

TL;DR. This:

is(Array)([{}])

evaluates to true in rambdax 0.12.x (consistently with ramda), but false in 0.13.

I think this is a regression, but perhaps this was never meant to work this way?

Which brings me to another point: I think is() should be called something else. It's incompatible with the API in Rambda and Ramda, which is very, very confusing, and will cause code to break when trying to upgrade from ram(b)da to rambdax. (and if in untested code, that might be especially bad).

It's a useful function, but I don't think it's a good thing that it's called the same thing as a function that does different things in what seems like the subset of the same library. I realize changing this now will result in a little bit of chaos, but I think it would be better in the long term.

(I'm happy to offer a PR with changes and/or missing tests — let me know what you think is the right thing to do here)

selfrefactor commented 5 years ago

It is a mess, I will admit that. The story of why I modified is is long. But I will go in direction to restore it back and change the current is to something else(like pass maybe). Bear with me, it will be ready in next few days and sorry for causing the confusion.

radex commented 5 years ago

Cool, thanks for explaining this @selfrefactor 👍

selfrefactor commented 5 years ago

I am closing the issue as new version 0.17.0 restores R.is as before. Feel free to commend and reopen if there is a need to.