build: Upgrade to Node 20 #744
Conversation
arbrandes
added
the
maintenance
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #744 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 2 2
Lines 7 7
======================================
Misses 7 7 ☔ View full report in Codecov by Sentry. |
arbrandes
force-pushed
the
node20-upgrade-2
branch
2 times, most recently
from
134b1b8 to
8dd721a
Compare
.github/workflows/ci.yml
Outdated
| strategy: | ||
| matrix: | ||
| node: [18, 20] |
There was a problem hiding this comment.
I'm curious as to what benefits using a matrix with 18 and 20 has over just upgrading to 20.
My initial reaction is that I'm not sure using a matrix makes sense here.
package-lock.jsonis generated using node 20, so expectingnpm cito work when using node 18 seems odd.- No MFEs use a matrix for this step, a specific version is expected
- Any MFE created using this template would start off with a CI check that needs to pass on both 18 and 20
There was a problem hiding this comment.
The motivation behind the matrix is to try - if at all possible - to get MFEs and libraries to work with both Node 18 and 20 for a transitional period, much like we did for the Node 16 -> 18 upgrade. This is so we can have Tutor - while still on Node 18 - work with MFEs that have already been tested to work on 20. And then when all Tutor-supported MFEs are Node 20-ready, we flip over Tutor, and remove 18 from the Matrix.
But yes, it may be that in some cases it won't be possible. It seems to be working fine here, though.
.github/workflows/ci.yml
Outdated
| with: | ||
| node-version: ${{ env.NODE_VER }} | ||
| node-version: ${{ matrix.node }} |
There was a problem hiding this comment.
If we move away from the matrix, I'd love to switch everything over to using node-version-file instead https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#node-version-file (example from paragon's release workflow: https://github.com/openedx/paragon/blob/98e6313e923c2a70b199f358c2a7063f03647a36/.github/workflows/release.yml#L19)
There was a problem hiding this comment.
This is sorta what we have now, except it doesn't use that particular setup-nodeism. No objections to moving to that after we remove the matrix, though.
arbrandes
force-pushed
the
node20-upgrade-2
branch
from
8dd721a to
3452c0c
Compare
Add official Node 20 support by: * Upgrading to Node 20 in `.nvmrc` * Regenerating `package-lock.json` from scratch using it * Blocking on Node 20 CI failures * Updating lockfile check workflow to v3
arbrandes
force-pushed
the
node20-upgrade-2
branch
from
3452c0c to
d536a95
Compare
Description
As a second step in the Node 20 upgrade process, upgrade
.nvmrcand regeneratepackage-lock.jsonfrom scratch using Node 20. Also make the Node 20 test a blocking one.See the tracking issue for futher information.