-
Notifications
You must be signed in to change notification settings - Fork 516
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
chore: update Algolia dependencies #2689
Conversation
Does renovate not work here? |
Deploy preview for algolia-instantsearch ready! Built with commit 7b4a053 https://deploy-preview-2689--algolia-instantsearch.netlify.com |
package.json must be updated along with the lock file too for this to work well. The yarn.lock file is not read when you install/check the dependencies as a user. i.e. it means that if you release InstantSearch.js with this package.json and I update InstantSearch.js in my project, if my local node_modules says helper 2.23.0 then it won't be updated (but the feature you rely on won't work too) |
Update: it's expected because it will be updated only this weekend. |
@vvo are you referring to If you want it bumped to the exact latest version every time, there's really no benefit to any end user in having the caret By this I mean to say: Why use ranges at all? Usually it's in case a library is "consumed" downstream and you want to give users as wide a choice of dependencies as possible, so that's why we assume you don't want it updated to the latest. Otherwise having a range but keeping it updated to the very latest release every time is effectively the same as having no range at all. Another alternative for you is to enable lock file maintenance, this bumps lock files even if the |
@rarkins I believe if we wait this weekend, the helper will be bumped to ^2.24.0 by renovate right? |
No, that's not the behaviour actually. The logic is: "if the latest npmjs release falls within the existing range, do not update it". This is because in most cases, people use ranges because they want to keep a wide range for downstream users. If you need it bumped to >= v2.24.0 for functional reasons (e.g. using a feature of v2.24.0 that's not supported by v2.23.0 then that's something a developer needs to do manually, because only the developer involved could know that. If instead you just think it's best practice to keep your downstream users as updated as possible (e.g. to the latest, once a week), is there any reason why not to remove the caret range and pin it? Another option is to use tilde And finally.. this "logic" I mentioned is not written in stone and I can consider a feature request if you wish to keep bumping the range ever week. I just prefer we both understand "why" things are like they are.. my philosophy is that whenever you set out solving the wrong problem, you're always going to end up with a wrong solution :) In short: you use ranges if you don't want to overly restrict your downstream users about which (compatible) versions they should use. If you do wish to be restrictive, then a narrower range and/or pinning sounds better. Let me know if the above is clear or any parts are unclear or disagreeable? |
When we rely on such tools (like Renovate) we should go for exact dependencies. When we don't want to upgrade just close the PR. Correct me if I'm wrong but when you close a PR Renovate will ignore the dependency until a new version is released. |
@samouss yes that's right. e.g. if existing is |
Thanks @haroen, @samouss and @vvo for picking up this and asking questions 🤘 The reasoning here is that I was unsure about the behavior of renovate (thanks @rarkins for the explanation here 😅) and I wanted to be sure that this dependency is updated correctly for Tuesday release. It contains the new insight feature. Clearly @vvo there is an issue with the package.json 😅 I will fix that according to renovate behavior. #teamwork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 We could also add a .yarnrc
with:
# .yarnrc
save-prefix false
Will pin the dependencies by default when we add a new one.
package.json
Outdated
"prop-types": "^15.5.10", | ||
"to-factory": "^1.0.0" | ||
"classnames": "2.2.5", | ||
"events": "1.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that as long as we don't do a release, people will be stuck on the older version of these, while with the ^, they'd still get upgraded, even if we don't update yet. I wonder if that's a problem 🤔
Thanks for all the details, now I am in favour of having dependencies pinned in all libraries. |
Adds support for insights in the helper.