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

add $ReadOnlyArray support to ramda includes + contains #3633

Merged

Conversation

LoganBarnett
Copy link
Contributor

Other notes:

Ramda's includes (same as contains, but contains is deprecated), would not respect $ReadOnlyArray. These changes allow for $ReadOnlyArray and also enforce a strict case when it comes to using strings (only strings may be compared with strings). Tests have been added to enforce this behavior.

Some tests have been problematic to enforce - namely the all-arity version of $ReadOnlyArray. It is not well understood why this is happening, but there are comments noting the expected outcome vs. actual. I don't believe it's a covariance issue, because 1) the curried form works fine, and 2) string and number shouldn't be covariant.

I also took the liberty of sorting some of the import lists, because it's very easy to find/place things when it's alphabetized :)

Ramda's includes (same as contains, but contains is deprecated), would not
respect $ReadOnlyArray. These changes allow for $ReadOnlyArray and also enforce
a strict case when it comes to using strings (only strings may be compared with
strings). Tests have been added to enforce this behavior.

Some tests have been problematic to enforce - namely the all-arity version of
$ReadOnlyArray. It is not well understood why this is happening, but there are
comments noting the expected outcome vs. actual. I don't believe it's a
covariance issue, because 1) the curried form works fine, and 2) string and
number shouldn't be covariant.

I also took the liberty of sorting some of the import lists, because it's very
easy to find/place things when it's alphabetized :)
Copy link
Contributor

@AndrewSouthpaw AndrewSouthpaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@AndrewSouthpaw AndrewSouthpaw merged commit 313c0f9 into flow-typed:master Nov 5, 2019
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