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

this repository should contain a package-lock.json #3556

Closed
4 tasks done
baywet opened this issue Feb 8, 2024 · 2 comments · Fixed by #3564
Closed
4 tasks done

this repository should contain a package-lock.json #3556

baywet opened this issue Feb 8, 2024 · 2 comments · Fixed by #3564
Assignees
Labels
script Pull requests that update Bash or JavaScript code

Comments

@baywet
Copy link
Contributor

baywet commented Feb 8, 2024

Follow up to the conversation on today's call @handrews
Having only a package.json makes it so the CI installs might install different versions from what was originally installed and tested.
TODO:

I'm happy to take this on if we agree on the content. :)

@handrews handrews added the script Pull requests that update Bash or JavaScript code label Feb 8, 2024
@handrews
Copy link
Member

handrews commented Feb 8, 2024

@baywet all of that sounds good to me. I'll note that our workflows don't all expect the same version of node, which means different packages give different warnings and errors in each workflow, but it somehow more-or-less works.

We don't need to fix that immediately unless the package-lock.json file might be different for different versions of node (I have no idea if npm downgrades some packages to make them work on older versions?)

@baywet
Copy link
Contributor Author

baywet commented Feb 9, 2024

That's a good callout, the workflows should ALWAYS specify which version of tooling they except through a setup action or equivalent. Otherwise we expose ourselves to the underlying image changing under our feet and things starting to break randomly (happened to me in the past, never fun). I've added setup node actions where I think it was needed. Right now it's not changing the default because the default for node has been 20.x lately (hence all the actions bumping their major version over the last few months)

No, it doesn't downgrade packages dependencies depending on the engine version AFAIK, but it produces warning like those:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'abnf@0.0.5',
npm WARN EBADENGINE   required: { node: '~0.10.10' },
npm WARN EBADENGINE   current: { node: 'v20.11.0', npm: '10.2.4' }
npm WARN EBADENGINE }

(that's from this repo, we might want to reach out to the package owner to understand why the specific version, if anything could be upgraded, and if no answer from them, consider an alternative as this is a very old version of node)

Authored #3564

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
script Pull requests that update Bash or JavaScript code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants