Skip to content

Conversation

@milanholemans
Copy link
Contributor

Closes #7078

Copilot AI review requested due to automatic review settings December 21, 2025 20:15
Copy link

Copilot AI left a 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 modern URL API 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

Copy link
Member

@waldekmastykarz waldekmastykarz left a 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.

@milanholemans
Copy link
Contributor Author

milanholemans commented Dec 22, 2025

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.

@waldekmastykarz
Copy link
Member

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.

@milanholemans
Copy link
Contributor Author

milanholemans commented Dec 23, 2025

Alright, I'll look into it.

@milanholemans milanholemans marked this pull request as draft December 23, 2025 13:14
@milanholemans milanholemans marked this pull request as ready for review December 23, 2025 19:59
@milanholemans
Copy link
Contributor Author

Added v25 as well.

Copy link
Member

@waldekmastykarz waldekmastykarz left a 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to Node@24

2 participants