-
-
Notifications
You must be signed in to change notification settings - Fork 68
fix: Loosen dependency requirements #182
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
base: master
Are you sure you want to change the base?
Conversation
Using exact dependency versions can be harmful, as it blocks downstream bug fixes, security patches, and new features. It can also increase the risk of duplicate packages in node_modules, leading to subtle, hard-to-debug issues. This PR changes all user-facing dependencies from x.y.z to ^x.y.z, allowing end users to automatically benefit from compatible updates as they are released.
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #182 +/- ##
=======================================
Coverage 95.84% 95.84%
=======================================
Files 23 23
Lines 842 842
=======================================
Hits 807 807
Misses 35 35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
We generally use exact version references to allow for deterministic testing. The solution is to continuously update the dependencies. |
The label |
Seems like something that a lockfile could help with. |
There is a package-lock file. If you look at merged PRs, you can see that they are opened automatically. If there are dependencies that are not updated, then it may be a bot issue we'd need to look into. |
Therefore you already have everything to make your tests deterministic and there's no point in carrying on your shoulders the burden of responsibility for updating consumer's dependencies! :) |
There are several reasons why it is the way it is. One being that package-lock can get corrupted and may need to be recreated, in which case the versions would be all over the place if we didn't fix them in the package.json as well. But this is beside your original point: dependencies should be automatically upgraded by dependabot, so if a dependency is not upgraded, please let me know which one it is. |
The lock file is stored in git. There is no way that it can become corrupted that we can't just revert to a working version. And even if we did need to start over for some reason, we could do so while keeping the versions the same - it's a bit complicated to do so, but straightforward once you understand how. If every project were to pin dependencies you would end up with a dozen copies of popular dependencies because it prevents them from being deduplicated when slightly different versions are used. |
First, thanks everyone for the thoughtful discussion. Over the years we’ve run into just about every issue imaginable around package-lock.json corruption and edge cases. That’s one reason why we keep the lockfile committed and rely on locked dependency versions for reproducible builds. It would be inconsistent to say “we have a lockfile to lock versions” while also expecting package.json to remain version-unlocked. Our intent is to lock versions. As a final note on the topic, we discourage changing dependency versions as our CI tests only run against the pinned versions. If you’d like to try different versions and run your own CI, please feel free to fork the repo, remove the lockfile, and rebuild, considering the risks. |
Just so we're in the clear: no one's advocating against the presence of the lockfile, which definitely should stay! We're advocating against pinning dependency versions for end consumers, which pushes your determinism problem onto all consumers and often causes duplicate trees and missed/delayed security/bugfix releases. |
It’s a trade-off. We prefer pinning direct dependencies because it guarantees our CI runs against the exact versions we ship. In the wild, upstreams occasionally don’t follow semver, and unpinned ranges can let those breakages reach developers before we can catch them. With pins, our CI identifies regressions without passing risk downstream. Sure there's the downside of duplicate installs, but that’s mostly bloat rather than a functional issue, and developers who want a single version can use npm overrides. In our view, pinning defaults to stability while still giving a conscious escape hatch. |
I will reformat the title to use the proper commit message syntax. |
Using exact dependency versions can be harmful, as it blocks downstream bug fixes, security patches, and new features. It can also increase the risk of duplicate packages in node_modules, leading to subtle, hard-to-debug issues.
This PR changes all user-facing dependencies from x.y.z to ^x.y.z, allowing end users to automatically benefit from compatible updates as they are released.
Summary by CodeRabbit