Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #89: update root version in package-lock.json #90

Merged
merged 1 commit into from
Mar 3, 2023
Merged

fix #89: update root version in package-lock.json #90

merged 1 commit into from
Mar 3, 2023

Conversation

strlns
Copy link
Collaborator

@strlns strlns commented Mar 2, 2023

Maybe "bug" is the wrong label here, more like a "chore"

@Viijay-Kr Viijay-Kr self-requested a review March 2, 2023 13:30
Copy link
Owner

@Viijay-Kr Viijay-Kr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @strlns Thanks a lot for opening this PR.

Package lock is one of my least favourite file hence I don't have much intel on its conventions.

However I see what you've done and what was the issue.
Does this fix the issue and guarantees that it won't recur ?

@strlns
Copy link
Collaborator Author

strlns commented Mar 2, 2023

Hi @Viijay-Kr , package-lock.json is updated automatically whenever you run npm install, so if you commit this file when releasing a new version, the issue shouldn't occur again.

You should be fine by just keeping the root "version" in package-lock.json in sync using npm install after updating your version number and then commiting the file.
Think of it as the same process as updating your build dependencies, but if you don't explicitly do that, only the root "version" should change.


If you use npm ci instead of npm install (or do not re-run npm install at all), the lockfile will not be updated.

It could also be a good idea to add an .nvmrc file for people who use nvm to manage the version of Node they use, as different Node versions can lead to different dependency graphs and hence differing package-lock.json.

For projects that are not VSCode extensions, the desired Node version is usually specified in the "engines" key in package.json as well (I guess I'm lecturing about stuff you already know here, sorry).

But for VSCode extensions it's different:
VSCode extensions specify "vscode" in the "engines"objects key instead of "node" and "npm", because VSCode provides the Node runtime for the extension.

It's important to differentiate between build setup and extension runtime here.
You have another Node runtime for your webpack build here.

Long story short: I had no unwanted changes to package-lock.json other than the "version" when running npm install as stated in CONTRIBUTING.md. I used the current LTS which is Node 18.

So changing "engines" would not be useful.

Module graph changes in package-lock because of differing Node versions are mostly a thing caused by older modules that use node-gyp and the like.

@strlns
Copy link
Collaborator Author

strlns commented Mar 2, 2023

Just coming back from being annoyed by a mismatch between "engines" and .nvmrc (see nvm-sh/nvm#964 and this table https://nodejs.org/en/download/releases/), I can totally understand you being annoyed by this topic :D

TL;DR of my longer message above: running npm install before releasing a new version and always assuming latest Node LTS version should be enough to provide an exactly reproducible package-lock.json for your repository (which doesn't change when someone checks out the repo and installs).

@Viijay-Kr
Copy link
Owner

@strlns That was awesome . Thanks a lot for your great insights.

I was thinking of integrating 'changesets' into the release process (which should automatically make the relevant changes when I prepare a release if am not wrong)

I will accept this PR and also would be delighted to have you as a collaborator if you prefer to contribute to this extension . wdyt ?

@Viijay-Kr Viijay-Kr merged commit cc7a452 into Viijay-Kr:main Mar 3, 2023
@strlns
Copy link
Collaborator Author

strlns commented Mar 3, 2023

Yes, thank you for your kind words - I would very much like to contribute!
Currently my time is sparse but that will change during the coming weeks.
Have a great day!

@Viijay-Kr
Copy link
Owner

Great , I've added you as a collaborator , you should have direct access to the repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants