feat(lock): add version --sync-workspace-lock flag to update lock file

[ghiscoding]

Description

This flag would run npm install --package-lock-only or associated command depending on the package manager client defined in npmClient (npm, pnpm or yarn). It will try to use the package manager client to update the lock file instead of relying on Lerna-Lite to update the file which requires a lot more code.

Motivation and Context

in summary this simplifies a lot the process of updating the lock file and also make it a lot more reliable since we are relying solely on the client to its job with a lot less logic to implement in Lerna-Lite. Relying on the client is also future proof in case the lock file structure ever change.

How Has This Been Tested?

unit tests were added

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov]

Codecov Report

Merging #199 (53bec57) into main (bffcb33) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #199      +/-   ##
==========================================
+ Coverage   93.11%   93.22%   +0.11%     
==========================================
  Files         134      134              
  Lines        3831     3889      +58     
  Branches      846      770      -76     
==========================================
+ Hits         3567     3625      +58     
  Misses        264      264              

Impacted Files	Coverage Δ
...kages/cli/src/cli-commands/cli-version-commands.ts	94.34% <ø> (ø)
...ackages/version/src/lib/update-lockfile-version.ts	100.00% <100.00%> (ø)
packages/version/src/version-command.ts	99.72% <100.00%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bffcb33...53bec57. Read the comment docs.

[ghiscoding]

@ghiscoding I have a repro where NPM works fine like pnpm. Again as long as the version constraint match with the local packages, no error is thrown test-npm.zip. Could you share a reproducible example with your error?

And according to npm/cli#3637 only npm@>=8.5.0 implemented it properly.

@StarpTech
Thanks a lot for that great comment you wrote in the other issue, this was a real awesome finding and now my code supports both older/newer npm version which is awesome. Here's the commit I managed to work on over long lunch :D

All your feedback are extremely valuable, learning a lot of things 😉

I might still work on the other PR to update the lock file directly just in case that this option wouldn't work for everyone. I wonder if I should make this the new default, or just leave it as an opt-in flag since it might not work for everyone!?

[StarpTech]

I wonder if I should make this the new default, or just leave it as an opt-in flag since it might not work for everyone!?

I'd enable it by default but with an option to disable it, just in case. We are trying to fix an already broken state. I see no reason to enable it after some manual testing and unit tests.

[ghiscoding]

I wonder if I should make this the new default, or just leave it as an opt-in flag since it might not work for everyone!?

I'd enable it by default but with an option to disable it, just in case. We are trying to fix an already broken state. I see no reason to enable it after some manual testing and unit tests.

Not entirely broken, it is broken for pnpm yes but it's not currently broken for npm users and that is what that other flag is for updateRootLockFile, on a project that uses npm workspaces (like Lerna-Lite is), then current code has updateRootLockFile enabled by default and Lerna-Lite will update the lock file directly and that is what the other opened PR #168 was trying to reuse for pnpm. I know that there are users that did tell me that they use npm workspaces, and indirectly rely on the other approach, however they would probably not notice if I change which approach (which of the 2 flags) is used to update the lock file but I'd have to make sure it's always working (I basically don't want to introduce a new regression for npm workspace users).

Since Lerna-Lite requires the user to have a workspace pre-setup, it might be ok to use this new flag and perhaps abandon the code to update the file directly which is always riskier and harder to deal with (you never know when any of the pm client change their lock file structure)

[StarpTech]

Since Lerna-Lite requires the user to have a workspace pre-setup, it might be ok to use this new flag and perhaps abandon the code to update the file directly which is always riskier and harder to deal with (you never know when any of the pm client change their lock file structure)

Make sense.

[ghiscoding]

@StarpTech
I think I'm done with this PR, you can do a final review before I merge it (probably on Monday or Tuesday)

Note that I decided to not make this new flag the default because I'm afraid of possible breaking change (yarn 1.x classic will actually not work with it). I'm also afraid that some developers might be surprised that running a version command calls a client install lock only file and I also don't have a Yarn Berry setup (I assume it would but I don't have time to spend on installing/testing it)... so for all these reasons it's an opt-in flag and will ask developers to enable the new flag themselves. I did however add the no- flag for both options, so it can be enabled/disabled. Also Note that I found that the --no- flag can only work in the shell (meaning that --no-package-lockfile-only in the shell would negate the flag but adding noPackageLockfileOnly in lerna.json does not, however packageLockfileOnly: false should be ok).

I still have to test this PR with an actual pnpm project, if you get a change to try yourself then that'd be great too or you can just review the PR and that should be fine too. Thanks a lot

@JacobSoderblom if you have a chance to test this new approach, it would be nice too. You will need to enable the new flag in the shell or in lerna.json via --package-lockfile-only (this will run behind the scene pnpm install --lockfile-only and also add the updated lock file to Git changes)

[StarpTech]

Awesome, thank you. I'll do and report. One thing. I find the name packageLockfileOnly confusing. It's clear from the package manager perspective but inside lerna.json it's kinda weird. I'd prefer something like syncWorkspaceLock or similar.

[StarpTech]

Good state, but proper tests are missing.

[ghiscoding]

Alright, I have addressed all comments identified in the review and I believe to be feature complete

• renamed packageLockfileOnly to syncWorkspaceLock (good suggestion)
• also decided to move the install lockfile-only logic execution out of the VersionCommand and move into update-lockfile-version.ts where it should belong
• tested locally with Lerna-Lite with the npm option (lower and greater than npm version 8.5.0)

The only thing is that I wanted to also test it with that my pnpm test repo ts-pnpm-workspace project but I don't understand how to use the pnpm link command. I used npm link in the past and I would simply run the command from the cli folder and I'd be good to go but that doesn't seem to work with pnpm link. How would you test with pnpm?

hmm if I run npm link and then call lerna version it does pick up my local code but only within Lerna-Lite, if I go to the pnpm project it's still link to the installed version from the registry

[StarpTech]

I used npm link in the past and I would simply run the command from the cli folder and I'd be good to go but that doesn't seem to work with pnpm link. How would you test with pnpm?

I don't use pnpm link directly. I use https://pnpm.io/pnpm-workspace_yaml

[ghiscoding]

I used npm link in the past and I would simply run the command from the cli folder and I'd be good to go but that doesn't seem to work with pnpm link. How would you test with pnpm?

I don't use pnpm link directly. I use https://pnpm.io/pnpm-workspace_yaml

I'm not exactly sure what you mean by this line, I mean if you had Lerna-Lite repo cloned, how would you link your pnpm project to use the local lerna-lite code? Typically an npm link would do the trick

[StarpTech]

I'm not exactly sure what you mean by this line, I mean if you had Lerna-Lite repo cloned, how would you link your pnpm project to use the local lerna-lite code? Typically an npm link would do the trick

I misunderstood it. Yes, pnpm link should do the trick

[ghiscoding]

@StarpTech
This just got pushed under the new release v1.5.0 🚀, could you also please let me know once you give it a try on your next version release. That would be nice to have a confirmation that it worked fine (at least I got confirmation from the npm side by releasing this new version, everything went smoothly but it would be nice to have confirmation with workspace: protocol in place, though I might migrate Lerna-Lite to pnpm in next few weeks).

Thanks a lot for the great review and the great flag name suggestion 😉

[StarpTech]

Hi @ghiscoding, will do and thanks for the hard work.

[johnsoncodehk]

@ghiscoding I needed it for a lone time, thanks to implement this!

But I can't make it works with lerna version command, is --no-git-tag-version--sync-workspace-lock only for lerna release?

If you need reproduction, you can clone https://github.com/johnsoncodehk/volar and run $ pnpm i && npm run version:test to try.

[ghiscoding]

@johnsoncodehk
Hey thanks for using it, I did see you switched to Lerna-Lite not long ago and I'm also a user of Volar so it'd be great to help you. Just as a reminder, I did create Lerna-Lite fork but I don't know all of Lerna's original code (neither do I understand it all). It's probably better to track this under a new issue. I'd have to check if --no-git-tag-version has any impact (I didn't test without git tag so it's possible but I'm unsure). Would you mind opening a new issue for this though. Thanks

[johnsoncodehk]

@ghiscoding Of course it's here: #214

(Btw --no-git-tag-version is typo, I copy wrong span, I mean --sync-workspace-lock)

[ghiscoding]

quick investigation seems to point to a simple missing npmClient, we should be good. It still makes me want to migrate Lerna-Lite itself to pnpm even more. 😄