-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: install npm PowerShell scripts on Windows #52009
Conversation
npm 9.8.0 added PowerShell scripts that should be installed alongside the cmd and shell scripts on Windows. Fixes: #48471 Refs: npm/cli#6548
@nodejs/platform-windows |
Hey @lukekarrys I worked on the Node.js MSI installer some time ago. I'll check this PR once I get some free cycles. I'll share my findings afterward. |
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.
@lukekarrys I've tried the installer locally. I've tried all the regular installation stuff - install, change, repair, etc. All cases worked as expected.
For me, this looks good, and thanks for adding it!
Landed in 4d86020 |
Added
baking-for-lts
|
npm 9.8.0 added PowerShell scripts that should be installed alongside the cmd and shell scripts on Windows. Fixes: #48471 Refs: npm/cli#6548 PR-URL: #52009 Reviewed-By: Luigi Pinca <[email protected]>
npm 9.8.0 added PowerShell scripts that should be installed alongside the cmd and shell scripts on Windows. Fixes: #48471 Refs: npm/cli#6548 PR-URL: #52009 Reviewed-By: Luigi Pinca <[email protected]>
npm 9.8.0 added PowerShell scripts that should be installed alongside the cmd and shell scripts on Windows.
All current versions of Node.js ship with npm@10 so this change should be safe to land.
I am not well versed in Windows development so I am not sure how this change should tested, and would like pointers on how to best to that.
I also want to add that there have been requests on the npm repo that PowerShell be made the default (npm/cli#6789). From my understanding, that is up to a user configuration but will now be possible by including these files. I plan to close that issue if this is merged unless there is follow-up work to somehow change the default which I think would need a different discussion.
Closes: #48471
Ref: npm/cli#6548