fix(install): use managed npm for global commands#1442
fix(install): use managed npm for global commands#1442lyzno1 wants to merge 8 commits intovoidzero-dev:mainfrom
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
lyzno1
force-pushed
the
codex/managed-npm-global-commands
branch
from
39c38fb to
d6af7bc
Compare
lyzno1
force-pushed
the
codex/managed-npm-global-commands
branch
from
d6af7bc to
6c66bcc
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c66bccd71
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e9603627f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| Ok(package_manager.run_add_command(&add_command_options, &self.cwd).await?) | ||
| Ok(package_manager | ||
| .run_add_command_with_global_npm_bin( |
There was a problem hiding this comment.
Why change this logic? What specific problem does it solve? Could you first submit an issue describing the problem?
There was a problem hiding this comment.
The intent is to use the npm that belongs to the Node runtime resolved by Vite+, instead of relying only on PATH. It still falls back to PATH npm when the adjacent npm is missing or not executable.
If this is not the intended direction, I’m happy to close this PR and open an issue first. :)
2 participants
Summary
npmwhen no explicit binary is provided.Why
Global npm commands previously depended on PATH being prepared by the caller. This makes the managed npm path explicit when it is safe to do so, while preserving PATH-based fallback for system runtimes and unchanged command arguments.
Tests
cargo test -p vite_command --no-fail-fastcargo test -p vite_install --no-fail-fastcargo test -p vite_global_cli --no-fail-fast