fix(vscode): properly retrieve Node.js when it isn't by default on the PATH
#14920
Conversation
…ATH` - should work cross-platform - tested with fnm on windows, should also work with nvm, nvm-windows and volta - the added checks should only run when typeAware is enabled **and** Node isn't already in the `PATH`
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
github-actions
bot
added
A-editor
There was a problem hiding this comment.
Pull Request Overview
This PR adds automatic Node.js detection for type-aware linting in the VS Code extension when Node.js isn't directly available on the PATH. It implements support for popular Node.js version managers (fnm, nvm, nvm-windows, and Volta) to ensure the language server can locate Node.js even when managed dynamically.
Key Changes:
- Implements environment resolvers for fnm, nvm, nvm-windows, and Volta to locate Node.js installations
- Adds PATH manipulation logic to prepend detected Node.js binary directories
- Only activates when type-aware linting is enabled and Node.js isn't already on PATH
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // The following path construction assumes the standard Unix-like nvm directory structure : | ||
| // $NVM_DIR/versions/node/<version>/bin | ||
| // nvm-windows is handled separately above via NVM_SYMLINK | ||
| const binDir = join(nvmDir, 'versions', 'node', sanitized, 'bin'); |
There was a problem hiding this comment.
The hardcoded path segments 'versions', 'node', and 'bin' represent magic values that encode nvm's directory structure. Consider extracting these as named constants (e.g., 'NVM_VERSIONS_DIR', 'NVM_NODE_DIR', 'NVM_BIN_DIR') to improve maintainability and make the directory structure assumption explicit.
|
EDM115
changed the title
PATHPATH
This comment was marked as outdated.
This comment was marked as outdated.
|
I am not sure if this is something a VSCode extension should solve. I have no problems with my nvm setup, it auto added the node path to the Other extension are providing a |
|
hmmm indeed that's actually clever once again it was just a try on my end to fix the issue, but you guys know better what to actually do :) |
see #14920 (comment) > When Node.js isn't on the PATH by default but rather added dynamically by a 3rd-party tool (mostly to manage on-the-fly multiple Node versions), the type aware checking in the VS Code extension will fail to run.
2 participants
Important
Full disclosure : Most of the code here has been written by OpenAI Codex.
Although I was able to check and fix the part related to FNM (which now works), I can't be sure about NVM, nvm-windows and Volta. My machine is sadly not powerful enough to spin up Windows & Linux VMs to test that out. Speaking of tests, I admit it would be better to add some related to the added functionality but I have no clue how to properly test this.
I am also unsure about any format guidelines (I did ran
just fmt) or if this should be put somewhere else than in theextension.ts.I do not really expect this PR to be merged, at least as-is. Consider this more as an example on how to implement #13497
What does it do ?
When Node.js isn't on the
PATHby default but rather added dynamically by a 3rd-party tool (mostly to manage on-the-fly multiple Node versions), the type aware checking in the VS Code extension will fail to run.PATHenvoutput once rather than using it as a middleman withexec nodeFixes an issue ?
fix #13497
Additional reference
Workaround while this isn't merged
For FNM users, add the path to the
defaultalias to yourPATH, by default$HOME\AppData\Roaming\fnm\aliases\defaulton Windows or if you customized the install dirC:\path\to\fnm\aliases\default