fix(update): stop downgrading prereleases#7466
Conversation
zkochan
left a comment
There was a problem hiding this comment.
You could write the tests before the changes. You can add a test for the CLI in the pnpm project tests.
| unsafePerm: boolean | ||
| registries: Registries | ||
| tag: string | ||
| cmdFullName?: string |
There was a problem hiding this comment.
Don't pass the command name. The installation engine doesn't need to have direct knowledge about the CLI. Add a boolean called updateIfNewer.
There was a problem hiding this comment.
I have renamed it to preventDowngrade.
| case 'tag': | ||
| version = meta['dist-tags'][spec.fetchSpec] | ||
| if (cmdFullName === 'update' && preferredVersionSelectors && spec.fetchSpec === 'latest') { | ||
| version = pickLatestVersionToUpdate(meta, preferredVersionSelectors) | ||
| } else { | ||
| version = meta['dist-tags'][spec.fetchSpec] | ||
| } | ||
| break |
There was a problem hiding this comment.
Don't change this only for tag. Change it for all types of selectors. Maybe it can be moved outside of this function then.
There was a problem hiding this comment.
I have changed it so that the version would be reverted as long as preventDowngrade is true.
| } | ||
|
|
||
| function pickVersionWithoutDowngrade (meta: PackageMeta, fetchSpec: string, preferredVersionSelectors: VersionSelectors, otherVersion: string): string { | ||
| const preferredVersions = prioritizePreferredVersions(meta, fetchSpec, preferredVersionSelectors)[0] |
There was a problem hiding this comment.
I am not sure using preferredVersionSelectors is a reliable way to get the current version specifier from package.json
There was a problem hiding this comment.
Does the version specified in the project package.json always have the highest priority?
There was a problem hiding this comment.
Yes, it will have weight set to 1000. However, it can be a direct dependency of another workspace package.
There was a problem hiding this comment.
So if workspace package foo has outdated stable prettier and workspace package bar has alpha release prettier, prettier in foo won't be updated?
There was a problem hiding this comment.
But if it was the case, then how could the function pickPackageFromMeta picks multiple versions when it only return a single version?
There was a problem hiding this comment.
So if workspace package foo has outdated stable prettier and workspace package bar has alpha release prettier, prettier in foo won't be updated?
Yes, I think so. But it would depend on the order in which their version selectors are added to the array.
|
After thinking about it more I have realized that the reason it is so hard to implement it is that the Right now, Hence, we need to change how the |
|
@zkochan How should |
|
I think |
If that's the case then everything becomes simple. We no longer need to refactor the code, only add more code on top of this existing code to throw an error. |
|
I am not sure you understood what I meant. The See related code: pnpm/pkg-manager/plugin-commands-installation/src/installDeps.ts Lines 268 to 274 in 5f05d90 It should be reimplemented with an option that is passed down to npm-resolver. |
f4e02c0 to
c757fff
Compare
|
The tests don't pass after your last commit. |
It's the recursive ones after I tried to remove the functions that appends |
|
The failed test for bfa18c0 is named |
|
The changes in toResolveImporter.ts are ugly but I didn't find a way to do it better without a huge refactoring. We can rewrite it one day but it is not in priority. |
close #7436 --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
* docs(cli): correct `--latest` to follow merged PR pnpm/pnpm#7466 * docs(cli): tweak wording
|
Thanks for this change @KSXGitHub. I can confirm it works in 8.15 and this behavior makes a lot more sense. |
Fixes #7436