fix: correctly find max satisfying version from unsorted arrays#217
Open
leny-mi wants to merge 3 commits intoantfu-collective:mainfrom
Open
fix: correctly find max satisfying version from unsorted arrays#217leny-mi wants to merge 3 commits intoantfu-collective:mainfrom
leny-mi wants to merge 3 commits intoantfu-collective:mainfrom
Conversation
The getMaxSatisfying function assumed versions were sorted, but Object.keys() returns versions in arbitrary order from npm registry. This caused taze to suggest older compatible versions instead of the latest ones. Added comparison logic to track the actual maximum version during iteration. Also fixed TypeScript implicit any type error for the version variable. Fixes the issue where taze suggests ^2.4.1 instead of ^2.8.1 when both satisfy the version range.
Similar to the default mode bug, newest mode also assumed versions were sorted and simply returned the last element. This caused it to return an incorrect version when arrays were unsorted from Object.keys(). Changed to iterate through all versions and find the actual maximum. Test case: ['1.0.0', '3.0.0', '2.0.0'] now correctly returns '3.0.0' instead of '2.0.0'.
Author
|
After the initial fix, I found the same bug in 'newest' mode - it was also assuming sorted arrays. Fixed it the same way and added a previously failing test. Also cleaned up the test to match the existing style. I think both modes now handle unsorted arrays correctly. |
Author
|
On a second scan of the project issues, I think this is the same issue as and fixes #189 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
getMaxSatisfyingfunction assumed versions were sorted, butObject.keys()returns versions in arbitrary order from npm registry. This caused taze to suggest older compatible versions instead of the latest ones. Added comparison logic to track the actual maximum version during iteration. I've included a previously failing test for this.Linked Issues
Fixes #189
Additional context
I've been confused for a hot minute why taze wouldn't want to update to 2.8.1 of the yaml library if not using
taze latest.