fix(dev-config): activate Node via nvm instead of conflicting MSI install (#55)#68
fix(dev-config): activate Node via nvm instead of conflicting MSI install (#55)#68primetimetank21 wants to merge 4 commits intomicrosoft:mainmicrosoft/WindowsDeveloperConfig:mainfrom primetimetank21:squad/55-nvm-activationprimetimetank21/WindowsDeveloperConfig:squad/55-nvm-activationCopy head branch name to clipboard
Conversation
|
@microsoft-github-policy-service agree |
…crosoft#55) dev-config.winget installed both OpenJS.NodeJS.LTS (MSI) and CoreyButler.NVMforWindows, but never ran nvm install / nvm use. nvm4w owns the front of PATH via the C:\nvm4w\nodejs junction, which pointed at nothing, so node failed with "command not found" in new terminals. - Remove the OpenJS.NodeJS.LTS MSI resource (ends the PATH race) - Keep CoreyButler.NVMforWindows as the canonical Node manager - Add an NvmActivateNode DSC script that runs nvm install 24 + nvm use 24, refreshing PATH before and after so Node 24 is active for later steps - Update both READMEs (13 -> 12 apps) Node is pinned to major 24 to match the cli-checks.ps1 ^v24. assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d038267 to
22e0bbc
Compare
|
Why this PR pins Node 24 rather than Three reasons drove the decision to pin
The original Happy to be redirected: There's no written policy that explicitly rules out lts-tracking — this is an inference from observed practice. If the maintainers prefer |
There was a problem hiding this comment.
Pull request overview
This PR resolves a broken Node.js setup caused by installing both the Node.js MSI and NVM for Windows in the same dev-config run, by standardizing on NVM and explicitly activating a Node 24 toolchain during provisioning.
Changes:
- Removed the
OpenJS.NodeJS.LTSpackage fromdev-config.wingetto avoid PATH/junction conflicts with NVM for Windows. - Added an idempotent DSC
PowerShellScriptresource (NvmActivateNode) that installs/uses Node major 24 vianvm. - Updated README docs to reflect the Node/NVM approach and package list changes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
windows-dev-config/dev-config.winget |
Drops Node MSI install and adds a post-NVM activation script for Node 24. |
src/windows-dev-config/dev-config.winget |
Mirror of the dev-config update (NVM-only + Node 24 activation). |
windows-dev-config/README.md |
Updates documentation to reflect NVM-managed Node 24 and adjusts the apps list. |
src/windows-dev-config/README.md |
Mirror of the README documentation update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- NvmActivateNode: add PowerShell to dependsOn (matches file convention so pwsh 7 is present before the script runs) - Drop misleading "LTS" wording from the Node 24 activation description (script pins by major, not the lts alias) - README: add Windows Terminal to the app list and reframe the count as 11 core apps plus 2 optional (Oh My Posh, PowerToys) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- NvmActivateNode: add PowerShell to dependsOn (matches file convention so pwsh 7 is present before the script runs) - Drop misleading "LTS" wording from the Node 24 activation description (script pins by major, not the lts alias) - README: add Windows Terminal to the app list and reframe the count as 11 core apps plus 2 optional (Oh My Posh, PowerToys) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3ab8222 to
a334cdb
Compare
…ivateNode Addresses Copilot review on microsoft#68: - Guard node --version with Get-Command in getScript/testScript so the DSC run doesn't throw before Node is activated. - Switch the activation log line from Write-Host to Write-Information. Applied to both mirror config files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Need to go back and do the following:
|
…signed-copy edits (src/ only)
Conflict in src/windows-dev-config/dev-config.winget resolved: kept nvm-activation changes
(NvmActivateNode resource, NodeJS MSI removal, Node-24 pin, PowerShell dependsOn, Get-Command
guards, Write-Information) while incorporating main's silent-install additions. Top-level
windows-dev-config/{README.md,dev-config.winget} reverted to match main per src/docs/development.md
(signed release copies regenerated by sign pipeline; only src/ is editable source).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes #55
Root cause:
dev-config.wingetinstalled bothOpenJS.NodeJS.LTS(MSI) andCoreyButler.NVMforWindowsin the same provisioning run. nvm4w prependsC:\nvm4w\nodejs(a junction) to the Machine PATH, but becausenvm installwas never called, the junction target was empty. In any new terminal,noderesolved to the dead symlink (PATH priority) instead of the MSI install, producing "command not found". The conflict was internal todev-config.wingetitself, before any workload was applied.Changes
src/windows-dev-config/dev-config.wingetNodeJS(OpenJS.NodeJS.LTS) MSI resource; keepnvmForNode; addNvmActivateNodePowerShellScript resourcesrc/windows-dev-config/README.mdNodeJSrow; updatenvmForNoderow; clean up pinned-id referencesOnly the editable source under
src/windows-dev-config/is changed; the top-level signed release copies are regenerated by the sign pipeline (seesrc/docs/development.md) and are intentionally left untouched.Fix detail
Remove the
NodeJS(OpenJS.NodeJS.LTS) MSI resource -- eliminates the PATH race.Keep
nvmForNode(CoreyButler.NVMforWindows) as the canonical Node manager.Add
NvmActivateNode-- aMicrosoft.DSC.Transitional/PowerShellScriptresource withdependsOn: [nvmForNode]. Its three-phasesetScript:$env:Pathfrom Machine + User registry PATH sonvmis visible in the DSC process.nvm install 24thennvm use 24(throws on non-zero exit code).$env:Pathagain sonode/npmare visible to downstream resources.The
testScriptalso refreshes PATH and assertsnvmis on PATH andnode --versionmatches^v24\..Node is pinned to major 24 (not the
ltsalias) to stay in sync with thecli-checks.ps1assertion^v24.and avoid LTS-drift failures when Node 26 eventually becomes LTS.Acceptance criteria (cli-checks.ps1)
The three existing assertions remain valid and unchanged:
nvm versionsucceedsnode --versionmatches^v24.npm --versionsucceedsManual verification needed
nvm use 24creates a filesystem junction atC:\nvm4w\nodejs, which requires elevation and Developer Mode.securityContext: elevatedis set onNvmActivateNode. Please confirm junction creation succeeds on a real provisioning run.