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: update npm packages #358

Merged
merged 5 commits into from
Nov 17, 2023
Merged

fix: update npm packages #358

merged 5 commits into from
Nov 17, 2023

Conversation

joneugster
Copy link
Contributor

Update npm packages to address vulnerabilities.

@joneugster
Copy link
Contributor Author

joneugster commented Nov 14, 2023

Apparently there are now more packages after semver that need updating, in particular axios. See CI output.

Unfortunately, I didn't learn in #343 how to create the package-lock correctly

@mhuisi
Copy link
Collaborator

mhuisi commented Nov 15, 2023

Apparently there are now more packages after semver that need updating, in particular axios. See CI output.
Unfortunately, I didn't learn in #343 how to create the package-lock correctly

The simplest way: Revert the changes you made to the package-lock, delete the node_module folders in the root directory, lean4-infoview-api, lean4-infoview, and vscode-lean4, and then re-run the setup steps from the docs.

@joneugster
Copy link
Contributor Author

joneugster commented Nov 16, 2023

I'm sorry I haven't spotted the building instructions myself, but now I followed them.

(I don't fully understand lerna, so I deleted all four package-lock.json and node_module/ before following the building instructions: the ones in the main directory and in all three subdirectories. I believe that's right?)

@mhuisi
Copy link
Collaborator

mhuisi commented Nov 16, 2023

I'm sorry I haven't spotted the building instructions myself, but now I followed them.

No worries, it took me a while to figure out myself.

so I deleted all four package-lock.json and node_module/ before following the building instructions

The package-lock.json files shouldn't be deleted - note that they are checked into the repo as well.

@joneugster
Copy link
Contributor Author

I thought deleting/regenerating the package-lock.json was part of properly updating npm dependencies, is it not?

@mhuisi
Copy link
Collaborator

mhuisi commented Nov 16, 2023

No. If you want to update dependencies, you should go through npm instead (e.g. via npm outdated and npm update), which manages the lock-file for you.

Generally speaking, we don't always want to update every dependency to its most recent version. In this PR, we only want to update those with vulnerabilities.

@joneugster
Copy link
Contributor Author

I believe now this looks as it should, and hopefully that means I finally understood all the details.

@mhuisi
Copy link
Collaborator

mhuisi commented Nov 17, 2023

Looks correct now!

@mhuisi mhuisi merged commit a7ecccf into leanprover:master Nov 17, 2023
2 checks passed
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