win: update supported vs versions

[StefanStojanovic]

Checklist

• npm install && npm run lint && npm test passes
• commit message follows commit guidelines

Description of change

Since Node.js v22 will use C++20 standard, VS2017 can no longer be used for compiling native add-ons. This is already changed in the docs. This PR drop VS2017 support for Node.js v22 and above for node-gyp.

Note: Node-gyp with this change should be released and consumed by Node for the v22 RC to avoid potential problems on Windows.

cc @targos, @nodejs/node-gyp

Refs: nodejs/build#3603
Refs: nodejs/node#45427

[gengjiawen]

This function should be removed. Also this should be a breaking change. commit. msg should be like

win!: update supported vs versions 

[StefanStojanovic]

I've changed the commit message. Should I also add the semver-major label?

Regarding the removal of findVisualStudio2017, why do you think it should be removed? I've added it in this PR so that VS2017 can be prohibited on Node v22+, while VS2019 and VS2022 will be enabled. On previous Node versions all 3 VS versions should work, like they are working in the current main branch.

[lukekarrys]

I don't follow the reasoning why this would be a breaking change. @gengjiawen Can you elaborate on this?

[targos]

LGTM, but I don't have authority on this project.

[cclauss]

I re-ran the failed jobs...
A new attempt at this workflow will be started, including all failed jobs and dependents:

• macos - 3.8 - 20.x
• windows - 3.12 - 20.x

---

All tests pass.

[lukekarrys]

LGTM. I would like others' thoughts on if this is a breaking change as referenced here #2959 (comment).

I don't think it is, in which case I can squash and merge with the PR title as the commit so the automated release does not tag this as a semver major.

[gengjiawen]

LGTM. I would like others' thoughts on if this is a breaking change as referenced here #2959 (comment).

this mainly follow Node.js main repo for compiler change.

[lukekarrys]

@StefanStojanovic Do you think this should be merged while #2957 is awaiting updates to your feedback?

[StefanStojanovic]

@StefanStojanovic Do you think this should be merged while #2957 is awaiting updates to your feedback?

This can be merged. The thing is, if that happens, the function other PR adds (findVisualStudio2017OrNewerUsingSetupModule) should also be split into 2 parts: 2017 and 2019+. I can do that work in that PR directly or as additional PR after that one lands.

Just a note, before merging I should squash the fixup commit and probably change win! to win at the beginning of the first commit message. I'll do that a bit later during the day

[StefanStojanovic]

@lukekarrys I've made the changes preparing this for merge. As I mentioned earlier, I have no problem making adjustments in #2957 (or in a separate PR) to get it in line with this.

[StefanStojanovic]

I've rebased this PR on top of the main branch, which now has the changes from #2957. @lukekarrys, since you've already approved my previous changes, I'd like to ask you to review the new ones and if it LGTY, land this.

[StefanStojanovic]

A reminder to @nodejs/node-gyp about this PR. I've adjusted it after #2957 landed, and as far as I can tell, it should be ready for merging.

[StefanStojanovic]

Since this is already approved and no there were no objections, I plan to land this tomorrow. If there are any complaints please state them until then.

[lukekarrys]

Sorry for the delay on review @StefanStojanovic and thank you for landing this.

I will work on getting this published this week.