-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
chore: ensure npm@11 is installed for OIDC publishing #8091
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
Conversation
Signed-off-by: Luke Karrys <luke@lukekarrys.com>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Updates the publish-packages workflow to install npm@11 globally to ensure OIDC (OpenID Connect) support for package publishing.
- Adds a setup step to install npm@11 globally before publishing packages
- Ensures compatibility with OIDC authentication which requires npm >=11.5.1
- Maintains existing workflow structure while adding necessary npm version dependency
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8091 +/- ##
==========================================
+ Coverage 73.11% 73.17% +0.05%
==========================================
Files 97 97
Lines 8440 8440
Branches 229 228 -1
==========================================
+ Hits 6171 6176 +5
+ Misses 2268 2263 -5
Partials 1 1 ☔ View full report in Codecov by Sentry. |
🤔 Have we tested the OIDC publish as it is without this change? It seems odd to me that we need to install a specific version of npm in CI, when we're using pnpm, not npm (this also seems very fragile to future npm updates as this is another place we'd need to remember to update). |
I'm not super familiar with this repo but it looks like the last publish was 5 days ago and the OIDC PR (#8085) was merged yesterday. It might be better to hold off on this PR until the next publish.
I agree with this which is why I mentioned maybe it's better to use install |
@avivkeller are you able to test the publish workflow with the OIDC change? |
Ah okay, makes sense. I wonder if we can put this in our package.json instead, and just leave it as an open requirement for |
https://github.com/nodejs/nodejs.org/actions/runs/17107462287/job/48520018615#step:7:21 only installed npm 10, so, no, it does not publish. I tested it locally before making that PR, but I have npm 11+, so I didn't catch this.
It wouldn't need to be there that long, only until Node.js v24 is LTS |
Oh hi @lukekarrys, been a minute, how's going? <3 |
I feel like as part of this PR, or in another, we should fix that the CI "passes" when it fails to publish 😂 |
I'll make that in a follow-up. @lukekarrys After my comments are addressed, i'm fine to fast track this, seeing as in a month or two v24 will be LTS. |
👍 Hadn't realised this'll be default in v24 -- yeh this seems fine to hard-code for now then, we should open an issue immediately to remind us to remove this. |
Signed-off-by: Aviv Keller <me@aviv.sh>
Lighthouse Results
|
@avivkeller Looks good! Now that the suggestions have been applied I just wanted to check that there was nothing else for me to do? |
That's it on my mind, once @nodejs/web-infra approves, this can be merged |
Description
Update to
npm@11
in thepublish-packages
workflow to ensure that OIDC is supported.This could also pin to a specific npm version instead of using the entire
v11
range.Validation
I can't confirm that is working without running the workflow but I did see that a similar change to #8085 failed in the node-gyp repo (nodejs/node-gyp#3201). npm's docs say that npm >=11.5.1 is required to use OIDC. I've also run a similar workflow in a work repo that used
pnpm publish
and it worked to update npm in this way.Related Issues
#8085
Check List
pnpm format
to ensure the code follows the style guide.pnpm test
to check if all tests are passing.pnpm build
to check if the website builds without errors.