feat: update package-lock.json workspace entiry versions #2088
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -34,6 +34,16 @@ export class PackageLockJson extends DefaultUpdater { | |||
| if (parsed.lockfileVersion === 2 || parsed.lockfileVersion === 3) { | |||
| parsed.packages[''].version = this.version.toString(); | |||
| } | |||
| if (this.versionsMap) { | |||
| for (const [p, obj] of Object.entries(parsed.packages)) { | |||
There was a problem hiding this comment.
Reading this not after midnight, I can probably clean this up a bit. But before I do I would like to get a general go ahead from a maintainer.
There was a problem hiding this comment.
Feel free to clean this up in a follow up
There was a problem hiding this comment.
Ok, yeah I don't think it was too bad, but it took me a second in the morning to read that and remember what exactly I had done. If you are good with it on this PR I honestly am good with just leaving it since it really isn't that bad and does appear to fully resolve the issue.
|
@chingor13 could I get some eyes on this? We are working toward using RP in a tool at Netflix and without this patch it is really painful to release a monorepo. It requires pulling down the release branch, running install, then ammending and force pushing. I tried to setup an automated action base workflow, but it has drawbacks and I was hoping this could land so I don't need to perfect that workaround. Especially since this was a pre-existing ask and I ensuring the locks are bumped will benefit all users. |
|
I'm not a maintainer, but I'd also be keen to see this land! |
|
@chingor13 sorry for the ping again, not sure if there are other folks who I can ping to look at this, but it would be nice if we could land this soon. Thanks! |
| @@ -34,6 +34,16 @@ export class PackageLockJson extends DefaultUpdater { | |||
| if (parsed.lockfileVersion === 2 || parsed.lockfileVersion === 3) { | |||
| parsed.packages[''].version = this.version.toString(); | |||
| } | |||
| if (this.versionsMap) { | |||
| for (const [p, obj] of Object.entries(parsed.packages)) { | |||
There was a problem hiding this comment.
Feel free to clean this up in a follow up
wesleytodd
force-pushed
the
npm-lockfile-workspace-update
branch
from
2501c20
to
a010d68
Compare
|
Fixed the lint issues on my branch. |
| for (const [name, ver] of this.versionsMap?.entries()) { | ||
| if (obj.name === name) { | ||
| obj.version = ver.toString(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reading this again, this could be slightly more performent:
| for (const [name, ver] of this.versionsMap?.entries()) { | |
| if (obj.name === name) { | |
| obj.version = ver.toString(); | |
| } | |
| } | |
| const ver = this.versionsMap.get(name); | |
| if (ver) { | |
| obj.version = ver.toString(); | |
| } |
There was a problem hiding this comment.
oh ha, thats funny I think I had written the original code assuming what I had there was a list and then instead of just reading it again when I modified it to take the map I just.....left it lol. Will fix that.
EDIT: oh yeah, it was the middle of the night...makes sense lol
wesleytodd
force-pushed
the
npm-lockfile-workspace-update
branch
from
a010d68
to
34d4608
Compare
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #1993 🦕