-
Notifications
You must be signed in to change notification settings - Fork 370
Updates node version to v24 #7079
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
base: main
Are you sure you want to change the base?
Conversation
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
This PR updates the CLI for Microsoft 365 project from Node.js v22 to v24. However, there are critical issues with the package versions specified that need to be addressed before merging.
Key changes:
- Updated required Node.js version from 22 to 24 across all configuration and CI/CD files
- Replaced deprecated
url.parse()with modernURLAPI in AuthServer.ts - Updated @types/node dependency from v22.19.3 to v24.10.4
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/AuthServer.ts | Replaced deprecated url.parse() with modern URL API for parsing query strings |
| scripts/check-version.js | Updated Node.js version check from 22 to 24 |
| package.json | Updated @types/node dependency to v24.10.4 |
| npm-shrinkwrap.json | Updated @types/node and undici-types dependencies for Node.js 24 |
| docs/docs/contribute/environment-setup.mdx | Clarified Node.js version requirement to "current active LTS version" |
| README.md | Updated minimum Node.js version requirement to >= 24.0.0 |
| .github/workflows/release_next.yml | Updated all Node.js version references and matrix configurations to 24, added Node.js 22 compatibility testing |
| .github/workflows/release.yml | Updated all Node.js version references to 24 in build, test, and deployment jobs |
| .github/workflows/check_pr.yml | Updated all Node.js version references and matrix configurations to 24, added Node.js 22 compatibility testing |
| .github/copilot-instructions.md | Updated developer workflow documentation to reflect Node.js 24 requirement |
Files not reviewed (1)
- npm-shrinkwrap.json: Language not supported
waldekmastykarz
left a comment
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.
Like mentioned in the issue, let's use 24 as default, but keep 22 for testing, and maybe add 25 if it doesn't give too much trouble.
|
We have 22 for testing, right? https://github.com/pnp/cli-microsoft365/actions/runs/20415236848/job/58657966722?pr=7079 I can add 25 if wanted, but usually we don't add uneven versions for testing, do we? Most of the time, they are quite short-lived. |
|
You're right, sorry, looked at the wrong thing. In the past we haven't added odd numbers because they were problematic and would lead to refactoring for short-lived things. We can see if that's still the case. If it just works, then it would give us extra assurance for everyone on 25. If it requires odd workarounds, then it's not worth it. |
|
Alright, I'll look into it. |
04ed6fb to
1b0c81d
Compare
|
Added v25 as well. |
waldekmastykarz
left a comment
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.
Very cool! Ship it!
Closes #7078