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

Update dependencies #583

Merged
merged 8 commits into from
Jun 27, 2022
Merged

Update dependencies #583

merged 8 commits into from
Jun 27, 2022

Conversation

TheJaredWilcurt
Copy link
Member

@TheJaredWilcurt TheJaredWilcurt commented Jun 27, 2022

  • Updated all dependencies
    • Documented the one dependency I couldn't easily update
  • Removed all pnpm stuff, to simplify the repo.
    • package-lock.json is from npm
    • docs now give npm instructions
    • npm scripts no longer use pnpm
    • ci/cd uses npm ci
  • Updated GHA to use the package-lock.json

@@ -13,7 +13,7 @@ jobs:
with:
node-version: "16"

- run: npm install
- run: npm ci
Copy link
Member Author

Choose a reason for hiding this comment

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

npm ci will install the exact modules listed in the package-lock.json file. So you know that your published version will work exactly the same as it did for you locally. Without a lock file, or using npm install (which updates the lock file), you can end up with a sub-dependency at a different version which may cause unforeseen issues.

npm ci basically solves the "works on my machine" issue. Now it works on all machines.

"build": "esbuild src/index.js --bundle --minify --platform=node --outfile=./dist/index.cjs"
},
"ManifestComments": [
"Pinned update-notifier to 5.1.0, v6+ requires ESM"
],
Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately JSON files do not support comments. So I've added this section here, so documentation around the package.json file can still occur.

If you think package.json files should be more permissive to allow for comments, you can +1 this post:

- run: npm run install
node-version: ${{ matrix.node-version }}
npm-version: '8.5.0'
- run: npm ci
Copy link
Member Author

@TheJaredWilcurt TheJaredWilcurt Jun 27, 2022

Choose a reason for hiding this comment

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

File Changes:

  • node => node-version, most repos name the node matrix this, don't know why, but just making it like the rest.
  • npm 8.5.0 - By default Node 14 comes with npm 6, which supports v1 lock files. But this PR includes a V2 lockfile. So we need to use npm 7 or above for that.
  • actions-setup-node => volta-cli/action, This lets us easily set the Node and npm versions in one go, so we can do Node 14, 16, or 18 all with npm 8.5.0
  • npm run install => npm ci, this makes sure we download the exact same dependency versions that were used during development

Copy link
Collaborator

@ayushmanchhabra ayushmanchhabra left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

3 participants