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

0.29.0 Upgrade Guide #3369

Closed
adispring opened this issue Apr 7, 2023 · 14 comments
Closed

0.29.0 Upgrade Guide #3369

adispring opened this issue Apr 7, 2023 · 14 comments

Comments

@adispring
Copy link
Member

adispring commented Apr 7, 2023

v0.28.0...v0.29.0

🆕 Added:

💥 Breaking Changes:

💡 Changes:

⭐ Miscellaneous

📋 Documentation fixes: #2004 #2116 #2368 #2797 #2955 #3010 #3220 #3221 #3228 #3235 #3242 #3244 #3263 #3291

✔️ Testing improvements: #3269 #3276

:shipit: Build and other internal improvements: #3018 #3082 #3100


Many thanks to all the contributors supporting this release.

We'd also like to call out to all those who submitted issues or PRs that we've chosen not to include. It's often easy to forget that the attempts that fail are often just as important to long-term success as those that succeed. Thank you for all your great work! 🎆

@jlissner
Copy link

jlissner commented Apr 8, 2023

It feels like the breaking changes are kind of glossed over, like, R.of, R.propEq, and R.pathEq could mean quite a few changes for large code bases (mine included). Feels like breaking changes should be highlighted, or at least separated, from non-breaking changes

@adispring
Copy link
Member Author

It feels like the breaking changes are kind of glossed over, like, R.of, R.propEq, and R.pathEq could mean quite a few changes for large code bases (mine included). Feels like breaking changes should be highlighted, or at least separated, from non-breaking changes

I have split breaking changes from normal improve changes.

@mendrik
Copy link

mendrik commented Apr 14, 2023

looks like export function of<T>(x: T): T[]; is still the old definition in 0.29.0 @types/ramda

@mendrik
Copy link

mendrik commented Apr 14, 2023

btw what's the point of changing order in propEq ? I think the arguments should be sorted by how static they are.
now the prop name/path is the most static element in the chain, so it should be placed first. IMO it would have been way better to swap propSatisfies order instead.

consider this:

import { cond, propEq, T as _, flip } from 'ramda'

const eq = propEq('hour') // this has to be now changed to flip(propEq)('hour')

type OwnProps = {
  hour: number
}

export const ClockBullet = cond<[OwnProps], JSX.Element>([
  [eq(1), () => <IconClockHour1 fill="white" color="#228be6" />],
  [eq(2), () => <IconClockHour2 fill="white" color="#228be6" />],
   ...
  [_, () => <IconClock fill="white" color="#228be6" />]
])

@JamesRamm
Copy link

What sort of versioning scheme is ramda following? Don't the breaking changes warrant a v1 release?

@bunglegrind
Copy link
Contributor

What sort of versioning scheme is ramda following? Don't the breaking changes warrant a v1 release?

No. Breaking changes are allowed in semantic versioning when you are in 0.*

@JamesRamm
Copy link

No. Breaking changes are allowed in semantic versioning when you are in 0.*

Major version zero is for initial development; ramda has been available for I-dont-know-how-many-years and has thousands of dependent packages. Isn't it time to declare v1 and start adhering to some stricter API stability requirements?

@SeanCannon
Copy link

Would have appreciated a param swap on R.gt, R.gte, R.lt, and R.lte as well. It always seemed like I should be able to do this:

const isGreaterThanThree = R.gt(3);

@bunglegrind
Copy link
Contributor

Would have appreciated a param swap on R.gt, R.gte, R.lt, and R.lte as well. It always seemed like I should be able to do this:

const isGreaterThanThree = R.gt(3);

see the discussion in
#1497

@davidsavoie1
Copy link

I really don't understand why you're switching the parameters order in propEq and pathEq. I use these functions all over my codebases and changing every single function call just to continue using Ramda's latest version is very inconvenient. I was tempted to go to 0.29.0 to fix the groupBy issue when used as a transducer, but now, I hesitate to do so...

Why not coming up with new names, but leaving the original functions intact? Maybe propEquals, propIsEq or even propEq2... I know it's not as elegant, but it wouldn't break all your users. That's actually what Rich Hickey is proposing in his talk Spec-ulation Keynote: sequence Breaking Changes are Broken.

Just to be clear: I absolutely LOVE Ramda and I include it de facto in each NodeJS or Javascript project I work on. So it's not a rant, but a recommendation for improvement.

@kpkonghk01
Copy link

kpkonghk01 commented Jul 14, 2023

#2938 change propEq/pathEq parameters order: comparing value first, prop/path second. This change will make propEq/pathEq parameters order be consistent to propSatisfies/pathSatisfies

I would prefer revert this change, based on the most common use case, discussed in the below issues/prs:

#1279

#2938 (comment)

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

This change was happened and reverted before. Aligning function's props order is the same reason as it was introducing at the last time. There is no stronger reason for introducing the same change again. It also breaks the expectation of the semantic versioning convention if only bumps the minor version. (I know anything can happen in 0.x, but many bot settings like dependabot ignore rule is set to ignore major version and expecting no breaking change in minor version update)

Maybe it's a naming problem as stated in this comment:
#2938 (comment)

the order is important. valueAtProp has the right order of arg references so it's a great candidate (but perhaps is missing the word "equals"/"eq")? eqAtProp or even eqProp then? propEq is not a good candidate however, because it implies that the first argument should be the equality check somehow.

Could we clone the current propEq to eqProp in the minor version upgrade and bring the order change back in version 1.0?
It would be easier for the users gracefully replacing their current usage of propEq with the new eqProp before major version update.

@kedashoe
Copy link
Contributor

Thanks all for the feedback. It seems the comments in favor of the old order far outnumber those in favor of the new order. I like the logic presented here, but from the reception of this change we need to go about any breaking changes we might make to these functions in a more deliberate manor, maybe after 1.0 as well.

I think the simplest thing to do is revert this change asap, release 0.30.0, and possibly revisit at a later date.

I will try to put up a PR at some point this week.

@SeanCannon
Copy link

Please don't change the order back. I refactored thousands of lines of code in multiple repos to change the order while also delivering tickets.. I'll have to lock that project to 0.29.0 .. this is really amateur guys, just release a 1.0.0 already so you start taking this library seriously.

@Harris-Miller
Copy link
Contributor

Just a reminder for everyone, the rules of semver dictact that any update for 0.x are considered breaking. See here: https://semver.org/#spec-item-4.

Though I would also argue that ramda should have been made 1.0.0 long ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants