chore: bump node and npm versions #3404
Conversation
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
| { | ||
| "name": "hydrogen", | ||
| "packageManager": "npm@10.9.2", | ||
| "packageManager": "npm@11.6.2", |
There was a problem hiding this comment.
we don’t have to bump this, if you have strong feelings to keep using npm v10 im happy to revert
the more important part is that all devs use corepack enable so we don’t have lockfile conflicts between npm 10 and 11 (look at the changes in this PR, those were lockfile v11 upgrades)
There was a problem hiding this comment.
It seems the lockfileVersion in package-lock didn't change so it's probably OK to bump this
fredericoo
force-pushed
the
01-23-chore_use_node_lts_for_monorepo
branch
from
910913d to
17f2417
Compare
|
We detected some changes in |
| }, | ||
| "engines": { | ||
| "node": ">=18.0.0" | ||
| "node": "^22 || ^24", |
There was a problem hiding this comment.
If we're going to use multiple versions we will probably want to update our CI workflows to run tests on both, too, instead of leveraging the .nvmrc https://github.com/Shopify/hydrogen/blob/main/.github/workflows/ci.yml#L19
We'd need to set up a matrix kind of like the CLI: https://github.com/Shopify/cli/blob/main/.github/workflows/tests-main.yml#L34
(or we could just roll with v22 for now)
There was a problem hiding this comment.
- 22 only makes more practical sense (or 24 only even)
- this is only the monorepo, e2e tests run against the dev app which runs on workerd so no need to run those again
- unit tests already have low parity with prod, as our app runs on node/workerd at a number of versions (you can fully pass a unit test and fail to build in oxygen by using
node:fsin a function)
There was a problem hiding this comment.
Devs will test locally with v24, let's keep v22 covered on CI at least 🤔
fredericoo
changed the title
frandiox
left a comment
frandiox
left a comment
There was a problem hiding this comment.
Just a comment on CLI versioning, otherwise I think it's good to bump this for the next major release, thanks!
| "engines": { | ||
| "node": ">=18.0.0" | ||
| "node": "^22 || ^24" | ||
| }, |
There was a problem hiding this comment.
Oof, it seems Shopify's CLI relies on Node v20. Perhaps we should add ^20 || ^22 || ^24 here as well to ensure our plugin works with the global CLI bundling?
I don't think they require v20 for "running" the CLI, but at least during the bundling process they will install our plugin and use v20 to do things 🤔
| { | ||
| "name": "hydrogen", | ||
| "packageManager": "npm@10.9.2", | ||
| "packageManager": "npm@11.6.2", |
There was a problem hiding this comment.
It seems the lockfileVersion in package-lock didn't change so it's probably OK to bump this
kdaviduik
left a comment
kdaviduik
left a comment
There was a problem hiding this comment.
The node version in dev.yml also needs a bump. We have a CI check for this but this PR was put up before this was added
4 participants
WHY are these changes introduced?
Node v20 will reach end of life on 30 Apr 2026
This PR bumps the node version to the current LTS and npm to the version that comes bundled with that given node version (https://nodejs.org/en/blog/release/v24.11.1)
WHAT is this pull request doing?
.nvmrcHOW to test your changes?
nvm install && nvm use)npm install -g npm@11.6.2)npm installChecklist