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

Test ap with fantasy-land and use consistent ordering of combination #2181

Closed
wants to merge 1 commit into from
Closed

Conversation

LiamGoodacre
Copy link

This PR is assuming that fantasy-land's ap is supposed to be like Haskell's (<**>) :: Applicative f => f a -> f (a -> b) -> f b, and not like flip (<*>) :: Applicative f => f a -> f (a -> b) -> f b (which is how Ramda currently treats it).

My test case attempts to show the difference by using Either. The important case is the first one:

eq(R.ap(leftFn, leftVal).simple(), leftFn.simple());

// i.e: forall e x. R.ap(Left(e), x) = Left(e)

@kedashoe
Copy link
Contributor

Thanks for the PR @LiamGoodacre . To comply with FL, we just pass along the first argument to the second if the latter has fantasy-land/ap. So for example, if sanctuary's Either were passed in to us, we would get here:

https://github.com/sanctuary-js/sanctuary/blob/master/index.js#L2519

. In the instance you highlighted, other is not a Right and we would get back other, ie Left(e) in your example. Some quick tests seem to confirm this.

Ramda doesn't really have much to say about it, it is up to the instance to implement ap correctly.

@LiamGoodacre
Copy link
Author

So you're saying that for FL, ma['fantasy-land/ap'](mf) is supposed to be implemented like mf <*> ma and not ma <**> mf?

I haven't been able to see on the FL spec that it is supposed to be one or the other. Given the type signature I was expecting <**>. Conversations in the Gitter channel also suggest it was supposed to be <**>.

@kedashoe
Copy link
Contributor

You probably know more about this than me :) There have been many discussions about ap in FL, see fantasyland/fantasy-land#145 as a starting point. FL also has a gitter that is quite active, so you can ask there as well.

@LiamGoodacre
Copy link
Author

Closing because: lack of interest; I no-longer have the need to use Ramda; I'm not particularly invested in the outcome.

@LiamGoodacre LiamGoodacre deleted the fix/ap branch October 3, 2017 21:41
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.

2 participants