fix(info): Use version from latest dist-tag instead of the highest one
#4797
Conversation
|
This change will decrease the build size from 9.94 MB to 9.94 MB, a decrease of 213 bytes (0%)
|
1 similar comment
|
This change will decrease the build size from 9.94 MB to 9.94 MB, a decrease of 213 bytes (0%)
|
|
I think we need to combine this with a fix for #3560 as well, otherwise |
|
@neonowy Great work, thanks! Can you check to add this logic to the install process as well? As for the tests, the fixtures should get added to the folder after you run your tests. You then only have to add them to the commit, and it should be fine 👍 |
|
Great, thanks! :) |
|
Pushed #4804 to tackle the |
| @@ -72,7 +72,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg | |||
| const versions = result.versions; | |||
| // $FlowFixMe | |||
| result.versions = Object.keys(versions).sort(semver.compareLoose); | |||
| result.version = version || result.versions[result.versions.length - 1]; | |||
| result.version = version || result['dist-tags'].latest; | |||
There was a problem hiding this comment.
Are we always guaranteed to have dist-tags and ['dist-tags'].latest?
There was a problem hiding this comment.
Who knows, it's entirely up to the npm server implementation :/ For now I think we can make this assumption, and revisit if needed.
| @@ -72,7 +72,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg | |||
| const versions = result.versions; | |||
| // $FlowFixMe | |||
| result.versions = Object.keys(versions).sort(semver.compareLoose); | |||
| result.version = version || result.versions[result.versions.length - 1]; | |||
| result.version = version || result['dist-tags'].latest; | |||
There was a problem hiding this comment.
Who knows, it's entirely up to the npm server implementation :/ For now I think we can make this assumption, and revisit if needed.
|
@neonowy Can you rebase this PR? I tried to do it on my own, but Github denied me the permission to do so (have you unchecked the checkbox that gives us permission to update the PR?) |
Fixes yarnpkg#3947. By default, package `version` was set by sorting all the versions and getting the highest one. Now it's provided via package `latest` dist-tag.
neonowy
force-pushed
the
info-latest-version-tag
branch
from
4a3c0cf to
54cb658
Compare
|
@arcanis Rebased! Sorry for the trouble with edit permission, apparently I've unchecked it when creating this PR. |
|
np, thanks! |
…one (yarnpkg#4797) * fix(info): Use version from `latest` dist-tag instead of the highest one Fixes yarnpkg#3947. By default, package `version` was set by sorting all the versions and getting the highest one. Now it's provided via package `latest` dist-tag. * Fix linter issues by shortening the test description * Manually mock request * Add scenario comment from yarnpkg#4804
Summary
Fixes #3947. By default, package
versionwas set by sorting all the versions and getting the highest one. Now it's provided via packagelatestdist-tag.Test plan
Before:
After:
I've also added a test case for it in
__tests__/commands/info.js.Although, I'm not sure if using a specific package like
ui-select(from #3947) is a proper way to do it, because it'll break with the new release.Could we add it to the
__tests__/fixtures/request-cache/, so the package manifest won't change? Or do you have some other idea how can I write this test better?