change propEq/pathEq parameters order

[adispring]

Make propEq/pathEq parameters order be consistent to propSatisfies/pathSatisfies parameters order.

related to #2937

[CrossEye]

Thank you very much. I think this is a very positive change, but I'd like to leave it open for a bit to hear back from others. Any objections, @ramda/core?

[Bradcomp]

This is a big breaking change. I think it's good but I think it's not fair to make this without bumping to 1.0.

[Bradcomp]

@CrossEye What do you think about using this one as an impetus to get to 1.0?

It's a positive change, and we can look at the other path stuff, clean it up, then clear this out and 💥

[CrossEye]

@Bradcomp: That sounds like a very good idea.

[lambert-velir]

Any news on this? I just spent 15 minutes debugging why this didn't work:

R.propSatisfies("name", R.test(queryRegex))

[CrossEye]

This is one of the few open issues that I think might be worth a breaking change in v1.0 -- or possibly a short-lived v0.29.

[customcommander]

This is one of the few open issues that I think might be worth a breaking change in v1.0 -- or possibly a short-lived v0.29.

My personal preference would be to stop releasing breaking changes in minor v0 releases especially if we're committed to release v1.0 as soon as possible. As much as I prefer having a non-breaking v1.0, I think these changes bring some consistency and v1.0 is probably the best time to introduce such breaking change.

I'm in favour of this change but would prefer to release it as part of v1.0

[customcommander]

@adispring I took the liberty to update your branch:

1. Rebased against origin/master to fix a conflict
2. Fixed a failing test (somehow one test for propEq wasn't updated to reflect the change of parameter position)

@CrossEye I think this change was welcomed by pretty much everybody in the team. Could we merge this now and ship it in v1.0? I already added that milestone :)

[adispring]

Is it more natural to keep current propEq/pathEq's parameter order? And instead, we can change propSatisfies and pathSatisfies's parameter order.

[customcommander]

Is it more natural to keep current propEq/pathEq's parameter order? And instead, we can change propSatisfies and pathSatisfies's parameter order.

I think your change makes more sense as it focus on the value first as opposed to where the value is:

var isJohn = propEq('john');

isJohn('name', {name: 'john'});
isJohn('first_name', {first_name: 'john'});

With the current parameters order you would have to flip the function first.

[adispring]

I think your change makes more sense as it focus on the value first as opposed to where the value is:

OK.

[semmel]

@customcommander wrote

I think your change makes more sense as it focus on the value first as opposed to where the value is...

How to order the arguments is not so clear cut as your comment suggests. Is not

put the least changing arguments first

the principle of Ramda?

With that principle, the "focus" should sharpen with the position of the argument – so the most general would be first.

In the test example propEq('brown', 'hair'); one could also argue that the subject "hair" is more general than the particular color "brown".

What the least-changing or most general is, may depend on the particular context. E.g. I use more often the flipped version of contains to determine that a value is contained in a known set (enumeration). In that case the enumeration is clearly the least-changing argument and should come first. However, it would be perhaps unpractical for Ramda to introduce it's functions in all permutations. For that there is flip and __.

If this change comes through, we may eliminate the inconsistency with propSatisfies, but how many other will be introduced?
E.g. assoc, adjust and update are authored in the way that least-changing is where the value is.

[customcommander]

I don't think this is necessarily going against the philosophy of putting data last. We simply have different opinions about what the least-changing parameters are.

For me propEq(42) is the same as propSatisfies(equals(42)) as I consider 42 as a form of predicate. So in my view the parameters order should be consistent.

You make reasonable observations (as always) but we don't need to put everything into the same bag either. I don't see propEq and the like as related to functions such as assoc (just my opinion here).

This PR has been opened for more than two years now so we ought to make a decision at some point. Do you feel strongly against this change?

[semmel]

This PR has been opened for more than two years now so we ought to make a decision at some point. Do you feel strongly against this change?

No, I have anticipated (feared) this breaking change might come eventually and moved away from propEq. The trivial task it solves is not worth introducing potential bugs. Instead I write ({key}) => key === value.

I was just asking if someone else had a consistent conception about the order of parameters in Object-accepting functions of Ramda. This PR might be the right place to ask I thought.

[CrossEye]

I was just asking if someone else had a consistent conception about the order of parameters in Object-accepting functions of Ramda. This PR might be the right place to ask I thought.

I don't think there is any universal answer to these questions. For the majority of Ramda functions the order feels immediate, and obviously correct. But some, like includes and propEq seem to have equal weight, and no wonderful answer. Here's where I really miss Haskell's left- and right-sections.

[semmel]

@whitelizard Thanks for working out the concept of naming functions. It is easy to remember and brings consistency to the vast API surface.

Your Steps 1 and 2 should

• Go prominently in the Readme. It they were, it could have reduced the many times I had to visit the documentation to look up a function signature,
• Become the criterion for adding new functions

I also agree that naming propEq badly in the first place, brought us into this mess here.

I briefly checked the list of Ramda functions, if they comply with your Steps 1 and 2. I came up with just a few mismatches!

existing fn	signature	consistent name
allPass	[(*… → Boolean)] → *… → Boolean	passesAll
anyPass	[(*… → Boolean)] → *… → Boolean	passesAny
append	a -> [a] -> [a]	appendValue
appendFlipped	[a] -> a -> [a]	appendTo
nthArg	Number → *… → *	argNth
	[Idx] → a → {a} → Boolean	eqPath
	Idx → a → {a} → Boolean	eqProp
included	[a] → a → Boolean	includedIn

(last updated 21 July 2023)

[KurtMar]

This was quite the unexpected breaking change for a minor version.

While I understand the benefit, unlike with removed or renamed functions the change from e.g. R.propEq('id', '123') to R.propEq('123', 'id') will cause many hard to find bugs in non-typescript codebases.

It would be fun if people would comment here the amount of time they spent tracking the bug before finding the culprit and the amount of time they spent refactoring their codebases. :)

[yuercl]

This was quite the unexpected breaking change for a minor version.

While I understand the benefit, unlike with removed or renamed functions the change from e.g. R.propEq('id', '123') to R.propEq('123', 'id') will cause many hard to find bugs in non-typescript codebases.

It would be fun if people would comment here the amount of time they spent tracking the bug before finding the culprit and the amount of time they spent refactoring their codebases. :)

i spend 3 hours to find this change in my test case today morning, and will spend 4 hours afternoon to refactor my code. 😭

[semmel]

@yuercl Don't forget that R.of has also changed.

A full text search 'n replace over all my code seems inevitable. That's why, I am staying on v0.28 where I can.

[crshmk]

I often want to

const idEq = propEq('id')

I never want to

const somePropEqualsOne = propEq(1)

I thus want to
revert

[Lambdaphile]

@crshmk Is there a workaround for this?

[crshmk]

@crshmk Is there a workaround for this?

@Lambdaphile to recreate the idEq above you can use a placeholder to reorder the props.

import { __, propEq } from 'ramda'

const idEq = propEq(__, 'id')

If you have a lot of broken propEqs and you don't want to update them all, the easiest solution is to use version 0.28

"dependencies": {
  "ramda": "=0.28.0"
}

However, if you use ramda in many different apps, managing two different incompatible versions across them is unwieldy at best and impossible if they share code. This is actually the larger issue. To use =0.28.0 and >=0.29.0 across shared code, I suggest creating your own propEq locally.

// propEq.js, kept in local utils
import { curry, equals, prop } from 'ramda'

export const propEq = curry((name, val, obj) => 
  equals(val, prop(name, obj)))

// app.js
import { filter } from 'ramda'
import { propEq } from './utils'

I have my own everyday ramda utils I've built over the years, so I just added it to those. In this way, all my apps can use version 0.28 or 0.29 of ramda as long as they grab the current version of my utils and import propEq from there (as above).

Finally, respectfully, the best solution would be to admit that we already litigated this and to revert, preferably quickly.

[Gastonite]

It was not a good idea to reverse parameters of that function IMO =(

because :

1. It's way more easier to read propEq('hair', 'brown') than propEq('brown', 'hair')
2. a function propEq('hair') is more useful than propEq('brown')
3. It creates hard to find bugs
4. It's just a cosmetic change that has no other real motivations
5. It introduces a big breaking change in a minor release and does not comply with semver: "MAJOR version when you make incompatible API changes"

Sometime it's better to simply rename a function instead of changing its signature

(I spent 2h to find the origin of bugs in my app)

[Pyrolistical]

I got bit by this.

While I agree with this change, these kinds of subtle breaking changes is what will prevent large adoption of this library. I wish this project to immediately release 1.0 and follow strict semver. Breaking changes are fine. Just make a new major release. That is what that is for!

[whitelizard]

While I agree with this change, these kinds of subtle breaking changes is what will prevent large adoption of this library. I wish this project to immediately release 1.0 and follow strict semver. Breaking changes are fine. Just make a new major release. That is what that is for!

I agree. Every lib should actually always start with v1.0.0, from the very beginning, for the npm semver system to work well. I recommend to read e.g. this article on this subject: https://medium.com/javascript-scene/software-versions-are-broken-3d2dc0da0783

[Harris-Miller]

First, as a reminder for everyone, per the rules of semver, for major version zero, minor bumps should be considered breaking. Ref: https://semver.org/#spec-item-4

That being said, IMHO ramda should have reached 1.0.0 long ago. Regardless of function names and signatures, what ramda offers as an API and how each function works are very stable. I personally don't see a reason to keep it as 0.x. That would have alleviated the main issue of the "breaking" expectation

To address the argument order of propEq, I agree with the signature change made in this MR. With that change, the following functions all share the same argument order:

• pathEq
• pathOr
• pathSatisfies
• propEq
• propIs
• propOr
• propSatisfies

I'm going to argue that this consistency is the most important part here. What the argument order should be is completely subjective.

As for function naming, we have a pretty decent convention going already, for the aforementioned functions, the convention is subject -> prop/path -> object -> boolean. In contrast, we have these functions

• assoc
• assocPath
• dissoc
• dissocPath
• adjust
• update

If we fully qualify the names, we get this:

• assocProp
• assocPath
• dissocProp
• dissocPath
• adjustIndex
• updateIndex

And the convention for these are prop/path -> subject -> object -> object.

To summarize, when prop/path comes first in the function name, prop/path is the second argument, and when prop/path comes second in the function name, prop/path is the first argument

Now, that convention could also apply to the fact that prop/path second is "mutate" and prop/path first is "evaluate against". We could keep that naming convention for that purpose and have them all share the same general signature of prop/path -> subject -> object -> result (this is what I meant about the argument order being subjective).

In the end, I think consistency is the most important part here. Therefore I vote not to revert this MR.

If it is decided that the argument order should be changed, then I would propose that, as part of a long overdue 1.0.0 release, change ALL of their signatures to be prop/Path -> subject -> object -> boolean TOGETHER as part of a single release.

[whitelizard]

Every function should stand on its own and not be dependent on or ruled by other functions and their most logical & convenient names and signatures (arg order). I don't think that arg order is always subjective, but the name should reflect the order, that is most important.
You could probably find many different groupings among all ramda functions, but if a grouping is based on names, and we adjust signatures based on the group of names, then we are working backwards! Signature comes first, then a name that correctly reflects what the function does AND (almost more importantly) the argument order -- regardless of other functions in the library.
(See my previous post)

[Harris-Miller]

regardless of other functions in the library.

Good APIs have consistent and predictable behavior. And that includes function signatures and argument order. My point is that they should be consistent. I'm not particular about which of the 2 orders here is correct, subject -> prop/path -> object -> boolean or prop/Path -> subject -> object -> boolean.

I don't think that arg order is always subjective

What I mean to say is the reasoning behind why the arguments should be in the order they are is subjective. Using your own example here

Example 1: A function that checks a predicate for an object prop
Step 1:
(a → Boolean) → String → {String: a} → Boolean
Note, as mentioned above, a function is to be considered highly general and "far away" from being "data". Therefor the predicate is best put first.

and

Example 2: A function that checks an object prop aganst a value
Step 1:
String → a → Object → Boolean
a is a value closer to being "data" than the name of the prop, therefor it should come later in the argument list.

Let's rewrite these 2 signatures generically.

• Example 1: X -> Key -> Object -> Boolean
• Example 2: Key -> X -> Object -> Boolean

The idea that a "function" is further away from "data" than "value", so Key should come second in one case and second in the other is subjective. I could argue that the Key is closer to the Object in both cases.

Besides, propEq = (value, key, obj) => propSatisfies(x => x === value, key, obj)

Whatever the order is, just have it be consistent. propEq was inconsistent before, and now it is.

I don't disagree with you that propEq should be String -> a -> Object -> Boolean, all I'm saying is that if we choose to do that, we should for propSatisfies, propIs, etc, as well.

[Harris-Miller]

For any future argument changes, I think the community would benefit greatly from codemods that ramda could supply as part of it's release process. Similar to what react supplies: https://github.com/reactjs/react-codemod

[whitelizard]

@Harris-Miller I feel that you don't get my point.

The naming is wrong, not the signature.

As I wrote earlier: e.g. "valueAtProp" would be the better name if the prop name (key) comes first in the argument list (Key -> X -> Object -> Boolean).

It is not the start of the function name that refers to the first argument, it is the last part of the function name that should refer to the first function argument. This is because of currying.

const ageIs = R.valueAtProp('age');
const isPerson20yearsOld = ageIs(20);
const celebratePerson = isPerson20yearsOld(person);

Do you agree that the intention of ramda functions is to be able to use currying in the way like above? And that the names should read well with the argument that follows, like above?

If the rules that I wrote in my first long post is followed, then the API would be entirely consistent.

[Harris-Miller]

Do you agree that the intention of ramda functions is to be able to use currying in the way like above? And that the names should read well with the argument that follows, like above?

@whitelizard Yes I do agree. My arguments for why propEq should have been changed as described in this MR from the start is solely for that and that alone. That does not mean that I disagree with you on the fact that the names should be different to better represent their signatures, or that the signatures should be different to better match the current names.

My goal is for consistency in behavior and expectation in the API.

If the rules that I wrote in my first long post are followed, then the API would be entirely consistent.

Also agree, as it aligns with my stated goal

[whitelizard]

So, the solution could be simple.

propEq is not really a useful function, as @crshmk has pointed out multiple times above. It could stay in the library, with arg order changed as by this MR, but the library should have the more useful function valAtProp (or whatever would be the best name). Also change the other inconsistent functions (their names!) according to @semmel.

Release 1.0.0.

Again, the order of designing any API function should be:

Need -> Signature -> Name

Never change the signature of a function to match the faulty name it unfortunately got. The signature was designed out of greatest utility. The name should change if it got a bad one. Changing the name is not initially a breaking change, it could start with a minor release of adding the new name to the API and flagging the old as "will be removed in the next major". Ramda could have this as part of its change strategy.

[Gastonite]

100% agree with @whitelizard
And I would add that I think it's preferable to consider the users of the library and their (often large) code base when planning such a huge change. By this I mean the difficulty of the migration needed when inverting the parameters order of a curried function, which could be used in several ways, compared to the simplicity of the migration if we just changed its name.

I also agree with having consistent functions in a library is a good thing, but maybe it would be a good thing also to add another function (temporary or not) to replace with when migrating.

[Harris-Miller]

@whitelizard just to be clear, to extend on this table

existing fn	signature	consistent name
allPass	[(*… → Boolean)] → *… → Boolean	passesAll
anyPass	[(*… → Boolean)] → *… → Boolean	passesAny
append	a -> [a] -> [a]	appendValue
appendFlipped	[a] -> a -> [a]	appendTo
nthArg	Number → *… → *	argNth
	[Idx] → a → {a} → Boolean	eqPath
	Idx → a → {a} → Boolean	eqProp
included	[a] → a → Boolean	includedIn
pathEq	a → [Idx] → {a} → Boolean	valAtPath
propEq	a → Idx → {a} → Boolean	valAtProp

While obviously incomplete, the table just represents the idea of deprecating existing fn columns, adding their replacements in the consistent name column, as well as adding new functions that don't currently exist

For the sake of typescript I personally like having complementary functions eqProp/valAtProp, append/appendValue, etc. There is currently no good way to type flip for functions of arity 3+. Having both varieties of all these functions would help a lot in that regard.

CC: @kedashoe

[semmel]

For the sake of typescript I personally like having complementary functions eqProp/valAtProp, append/appendValue, etc. There is currently no good way to type flip for functions of arity 3+. Having both varieties of all these functions would help a lot in that regard.

Yes, and I would extend that to a few binary functions too. They have crept up in Ramda-adjunct eg. R.includes ≡ flip(RA.included), R.append ≡ flip(RA.appendFlipped), but in Ramda too e.g. R.difference ≅ flip(R.without).

This is always when "…the reasoning behind why the arguments should be in the order they are is subjective".

Another example which comes to my mind is concat. Written in a function pipeline pipe(() =>a, concat(b))() I always need to imagine the binary form concat(x, y) = [...x, ...y] to find out, that the pipeline kind of appends all of a to b and not the other way round. That reasoning is ok. Is is not too hard to figure out and consistent in Ramda (difference, subtract).

On the other hand, while you @Harris-Miller mention it, flip for functions of arity 3+ gives my headaches — and not because of TypeScript. I think its an anti-pattern. (Just as a side-note)

[jktravis]

I know this has been beat to death, but I'd just like to add my two cents. I really hate this change. I just propEq and pathEq way more than propSatisfies. Plus, the naming, as other has suggested, practically tells you how to use it: prop, then equals. I've always had to look up the docs when needing propSatisfies simply because of the naming inconsistency. But even still, I think it would have made more sense to change propSatisfies to match the other two. /shrug

[papaver]

would it be possible to mention this change in the propEq documentation at least? it's extremely confusing for anyone using 0.28.0 that hasn't read this thread. i just wasted several hours trying to figure out why my code was working in the doc page but not in my code.

thanks.

[michaek]

Ouch. It seems pretty onerous have to change any and all calls to propEq/pathEq for such a flimsy and subjective reason. I get that the library will probably not change back to its historical API at this point, but this seems hostile to longtime users, and it certainly makes me consider the wisdom of moving away from a dependency on Ramda.