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

fix(upgrade) Fix basicSemverOperatorRegex #7080

Merged
merged 2 commits into from
Mar 5, 2019

Conversation

shilcare
Copy link
Contributor

@shilcare shilcare commented Mar 4, 2019

Summary

Fix bug #7079 of basicSemverOperatorRegex. It doesn't match the range operator correctly if the dependency range is specified with >=. For example, when running yarn upgrade --latest, if a dependency is specified with >=, the matched result will be >, which is not correct.

This is because the regular expression engine looks for alternations one-by-one. We can fix it by putting >= before >.

fixes #7079

Test plan

When running `yarn upgrade --latest`, if the range operator is '>=', the matched result is '>', which is not correct
@rally25rs
Copy link
Contributor

Nice catch! Thanks for the contribution. 🎉
Could you please add a line to CHANGELOG.md for this change?

@rally25rs rally25rs changed the title Fix basicSemverOperatorRegex fix(upgrade) Fix basicSemverOperatorRegex Mar 4, 2019
@rally25rs rally25rs self-requested a review March 4, 2019 16:27
@rally25rs
Copy link
Contributor

You know what, maybe we should just take > out of the regex. I think it actually causes another bug. If yarn upgrades you to a "latest" of 1.0.0 and preserves the > then it will change your pacakge.json to >1.0.0 which makes the latest version not acceptable. 🤦‍♂️ oof.

@shilcare
Copy link
Contributor Author

shilcare commented Mar 5, 2019

You know what, maybe we should just take > out of the regex. I think it actually causes another bug. If yarn upgrades you to a "latest" of 1.0.0 and preserves the > then it will change your pacakge.json to >1.0.0 which makes the latest version not acceptable. 🤦‍♂️ oof.

I think you're right :). We can't handle > with current buildPatternToUpgradeTo implementation.

@rally25rs
Copy link
Contributor

The continuous-integration/appveyor/pr status isn't reporting correctly, but the build finished and passed: https://ci.appveyor.com/project/kittens/yarn/builds/22821129

Going to merge this PR.

@rally25rs rally25rs merged commit 82d5891 into yarnpkg:master Mar 5, 2019
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.

yarn upgrade --latest error for dependency specified with >=
2 participants