Skip to content

Commit 62bfff3

Browse files
authored
feat(shim): improve npm global install experience (#708)
## Summary - **Post-install hint for** **`npm install -g`**: When Vite+ manages Node via shims, `npm install -g <pkg>` installs binaries into the managed Node's bin dir which is not on PATH. After install, the shim now checks whether installed binaries are reachable and offers to create links in `~/.vite-plus/bin/`, plus a tip suggesting `vp install -g` instead. - **Suppress npm output from** **`vp install -g`**: Pipe npm stdout/stderr so install noise is hidden on success, only shown on failure for debugging. Add `--no-fund` flag. - **Clean up CLI output**: Replace raw `println!` with `output::raw`/`output::warn`, remove unnecessary two-space prefix from install/uninstall messages. ## Test plan - [x] `cargo test -p vite_global_cli` — unit tests pass - [x] `cargo clippy -p vite_global_cli` — no new warnings - [x] Snap tests updated for all affected test cases - [x] `pnpm bootstrap-cli && vp install -g cowsay` — verify clean output closes VP-217 ![image.png](https://app.graphite.com/user-attachments/assets/e8256e72-0c97-4e38-abd6-ee61f778f9a1.png)
1 parent 8c5823f commit 62bfff3

57 files changed

Lines changed: 2009 additions & 152 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

crates/vite_global_cli/src/commands/env/bin_config.rs

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@ use vite_path::AbsolutePathBuf;
1313
use super::config::get_vite_plus_home;
1414
use crate::error::Error;
1515

16+
/// Source that installed a binary.
17+
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
18+
#[serde(rename_all = "lowercase")]
19+
pub enum BinSource {
20+
/// Installed via `vp install -g` (managed shim)
21+
#[default]
22+
Vp,
23+
/// Installed via `npm install -g` shim interception (direct symlink)
24+
Npm,
25+
}
26+
1627
/// Config for a single binary, stored at ~/.vite-plus/bins/{name}.json
1728
#[derive(Debug, Clone, Serialize, Deserialize)]
1829
#[serde(rename_all = "camelCase")]
@@ -25,12 +36,20 @@ pub struct BinConfig {
2536
pub version: String,
2637
/// Node.js version used
2738
pub node_version: String,
39+
/// How this binary was installed
40+
#[serde(default)]
41+
pub source: BinSource,
2842
}
2943

3044
impl BinConfig {
31-
/// Create a new BinConfig.
45+
/// Create a new BinConfig with `Vp` source (used by `vp install -g`).
3246
pub fn new(name: String, package: String, version: String, node_version: String) -> Self {
33-
Self { name, package, version, node_version }
47+
Self { name, package, version, node_version, source: BinSource::Vp }
48+
}
49+
50+
/// Create a new BinConfig with `Npm` source (used by npm install -g interception).
51+
pub fn new_npm(name: String, package: String, node_version: String) -> Self {
52+
Self { name, package, version: String::new(), node_version, source: BinSource::Npm }
3453
}
3554

3655
/// Get the bins directory path (~/.vite-plus/bins/).
@@ -43,6 +62,44 @@ impl BinConfig {
4362
Ok(Self::bins_dir()?.join(format!("{bin_name}.json")))
4463
}
4564

65+
/// Load config for a binary (synchronous).
66+
pub fn load_sync(bin_name: &str) -> Result<Option<Self>, Error> {
67+
let path = Self::path(bin_name)?;
68+
match std::fs::read_to_string(path.as_path()) {
69+
Ok(content) => {
70+
let config: Self = serde_json::from_str(&content).map_err(|e| {
71+
Error::ConfigError(format!("Failed to parse bin config: {e}").into())
72+
})?;
73+
Ok(Some(config))
74+
}
75+
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
76+
Err(e) => Err(e.into()),
77+
}
78+
}
79+
80+
/// Save config for a binary (synchronous).
81+
pub fn save_sync(&self) -> Result<(), Error> {
82+
let path = Self::path(&self.name)?;
83+
if let Some(parent) = path.parent() {
84+
std::fs::create_dir_all(parent)?;
85+
}
86+
let content = serde_json::to_string_pretty(self).map_err(|e| {
87+
Error::ConfigError(format!("Failed to serialize bin config: {e}").into())
88+
})?;
89+
std::fs::write(path.as_path(), content)?;
90+
Ok(())
91+
}
92+
93+
/// Delete config for a binary (synchronous).
94+
pub fn delete_sync(bin_name: &str) -> Result<(), Error> {
95+
let path = Self::path(bin_name)?;
96+
match std::fs::remove_file(path.as_path()) {
97+
Ok(()) => Ok(()),
98+
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()),
99+
Err(e) => Err(e.into()),
100+
}
101+
}
102+
46103
/// Load config for a binary.
47104
pub async fn load(bin_name: &str) -> Result<Option<Self>, Error> {
48105
let path = Self::path(bin_name)?;
@@ -227,4 +284,66 @@ mod tests {
227284
let loaded = BinConfig::load("nonexistent").await.unwrap();
228285
assert!(loaded.is_none());
229286
}
287+
288+
#[test]
289+
fn test_source_defaults_to_vp() {
290+
let config = BinConfig::new(
291+
"tsc".to_string(),
292+
"typescript".to_string(),
293+
"5.0.0".to_string(),
294+
"20.18.0".to_string(),
295+
);
296+
assert_eq!(config.source, BinSource::Vp);
297+
}
298+
299+
#[test]
300+
fn test_new_npm_source() {
301+
let config = BinConfig::new_npm(
302+
"codex".to_string(),
303+
"@openai/codex".to_string(),
304+
"22.22.0".to_string(),
305+
);
306+
assert_eq!(config.source, BinSource::Npm);
307+
assert_eq!(config.name, "codex");
308+
assert_eq!(config.package, "@openai/codex");
309+
assert!(config.version.is_empty());
310+
assert_eq!(config.node_version, "22.22.0");
311+
}
312+
313+
#[test]
314+
fn test_source_backward_compat_deserialize() {
315+
// Old BinConfig files without "source" field should default to "vp"
316+
let json =
317+
r#"{"name":"tsc","package":"typescript","version":"5.0.0","nodeVersion":"20.18.0"}"#;
318+
let config: BinConfig = serde_json::from_str(json).unwrap();
319+
assert_eq!(config.source, BinSource::Vp);
320+
}
321+
322+
#[test]
323+
fn test_sync_save_load_delete() {
324+
let temp_dir = TempDir::new().unwrap();
325+
let _guard = vite_shared::EnvConfig::test_guard(
326+
vite_shared::EnvConfig::for_test_with_home(temp_dir.path()),
327+
);
328+
329+
let config = BinConfig::new_npm(
330+
"codex".to_string(),
331+
"@openai/codex".to_string(),
332+
"22.22.0".to_string(),
333+
);
334+
config.save_sync().unwrap();
335+
336+
let loaded = BinConfig::load_sync("codex").unwrap();
337+
assert!(loaded.is_some());
338+
let loaded = loaded.unwrap();
339+
assert_eq!(loaded.source, BinSource::Npm);
340+
assert_eq!(loaded.package, "@openai/codex");
341+
342+
BinConfig::delete_sync("codex").unwrap();
343+
let loaded = BinConfig::load_sync("codex").unwrap();
344+
assert!(loaded.is_none());
345+
346+
// Delete again should not error
347+
BinConfig::delete_sync("codex").unwrap();
348+
}
230349
}

crates/vite_global_cli/src/commands/env/global_install.rs

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
//! Global package installation handling.
22
3-
use std::{collections::HashSet, io::Read, process::Stdio};
3+
use std::{
4+
collections::HashSet,
5+
io::{Read, Write},
6+
process::Stdio,
7+
};
48

59
use tokio::process::Command;
610
use vite_js_runtime::NodeProvider;
711
use vite_path::{AbsolutePath, current_dir};
8-
use vite_shared::format_path_prepended;
12+
use vite_shared::{format_path_prepended, output};
913

1014
use super::{
1115
bin_config::BinConfig,
@@ -29,7 +33,7 @@ pub async fn install(
2933
// Parse package spec (e.g., "typescript", "typescript@5.0.0", "@scope/pkg")
3034
let (package_name, _version_spec) = parse_package_spec(package_spec);
3135

32-
println!(" Installing {} globally...", package_spec);
36+
output::raw(&format!("Installing {} globally...", package_spec));
3337

3438
// 1. Resolve Node.js version
3539
let version = if let Some(v) = node_version {
@@ -63,22 +67,24 @@ pub async fn install(
6367
tokio::fs::create_dir_all(&staging_dir).await?;
6468

6569
// 4. Run npm install with prefix set to staging directory
66-
println!(" Running npm install...");
67-
68-
let status = Command::new(npm_path.as_path())
69-
.args(["install", "-g", package_spec])
70+
// Pipe stdout/stderr so npm output is hidden on success, shown on failure
71+
let output = Command::new(npm_path.as_path())
72+
.args(["install", "-g", "--no-fund", package_spec])
7073
.env("npm_config_prefix", staging_dir.as_path())
7174
.env("PATH", format_path_prepended(node_bin_dir.as_path()))
72-
.stdout(Stdio::inherit())
73-
.stderr(Stdio::inherit())
74-
.status()
75+
.stdout(Stdio::piped())
76+
.stderr(Stdio::piped())
77+
.output()
7578
.await?;
7679

77-
if !status.success() {
80+
if !output.status.success() {
7881
// Clean up staging directory
7982
let _ = tokio::fs::remove_dir_all(&staging_dir).await;
83+
// Show captured output to help debug the failure
84+
let _ = std::io::stdout().write_all(&output.stdout);
85+
let _ = std::io::stderr().write_all(&output.stderr);
8086
return Err(Error::ConfigError(
81-
format!("npm install failed with exit code: {:?}", status.code()).into(),
87+
format!("npm install failed with exit code: {:?}", output.status.code()).into(),
8288
));
8389
}
8490

@@ -136,7 +142,7 @@ pub async fn install(
136142
let packages_to_remove: HashSet<_> =
137143
conflicts.iter().map(|(_, pkg)| pkg.clone()).collect();
138144
for pkg in packages_to_remove {
139-
println!(" Uninstalling {} (conflicts with {})...", pkg, package_name);
145+
output::raw(&format!("Uninstalling {} (conflicts with {})...", pkg, package_name));
140146
// Use Box::pin to avoid recursive async type issues
141147
Box::pin(uninstall(&pkg, false)).await?;
142148
}
@@ -194,9 +200,9 @@ pub async fn install(
194200
bin_config.save().await?;
195201
}
196202

197-
println!(" Installed {} v{}", package_name, installed_version);
203+
output::raw(&format!("Installed {} v{}", package_name, installed_version));
198204
if !bin_names.is_empty() {
199-
println!(" Binaries: {}", bin_names.join(", "));
205+
output::raw(&format!("Binaries: {}", bin_names.join(", ")));
200206
}
201207

202208
Ok(())
@@ -230,17 +236,15 @@ pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
230236
let package_dir = packages_dir.join(&package_name);
231237
let metadata_path = PackageMetadata::metadata_path(&package_name)?;
232238

233-
println!(" Would uninstall {}:", package_name);
239+
output::raw(&format!("Would uninstall {}:", package_name));
234240
for bin_name in &bins {
235-
println!(" - shim: {}", bin_dir.join(bin_name).as_path().display());
241+
output::raw(&format!(" - shim: {}", bin_dir.join(bin_name).as_path().display()));
236242
}
237-
println!(" - package dir: {}", package_dir.as_path().display());
238-
println!(" - metadata: {}", metadata_path.as_path().display());
243+
output::raw(&format!(" - package dir: {}", package_dir.as_path().display()));
244+
output::raw(&format!(" - metadata: {}", metadata_path.as_path().display()));
239245
return Ok(());
240246
}
241247

242-
println!(" Uninstalling {}...", package_name);
243-
244248
// Remove shims and bin configs
245249
let bin_dir = get_bin_dir()?;
246250
for bin_name in &bins {
@@ -258,7 +262,7 @@ pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
258262
// Remove metadata file
259263
PackageMetadata::delete(&package_name).await?;
260264

261-
println!(" Uninstalled {}", package_name);
265+
output::raw(&format!("Uninstalled {}", package_name));
262266

263267
Ok(())
264268
}
@@ -359,7 +363,7 @@ fn is_javascript_binary(path: &AbsolutePath) -> bool {
359363
}
360364

361365
/// Core shims that should not be overwritten by package binaries.
362-
const CORE_SHIMS: &[&str] = &["node", "npm", "npx", "vp"];
366+
pub(crate) const CORE_SHIMS: &[&str] = &["node", "npm", "npx", "vp"];
363367

364368
/// Create a shim for a package binary.
365369
///
@@ -372,10 +376,10 @@ async fn create_package_shim(
372376
) -> Result<(), Error> {
373377
// Check for conflicts with core shims
374378
if CORE_SHIMS.contains(&bin_name) {
375-
println!(
376-
" Warning: Package '{}' provides '{}' binary, but it conflicts with a core shim. Skipping.",
379+
output::warn(&format!(
380+
"Package '{}' provides '{}' binary, but it conflicts with a core shim. Skipping.",
377381
package_name, bin_name
378-
);
382+
));
379383
return Ok(());
380384
}
381385

@@ -386,9 +390,13 @@ async fn create_package_shim(
386390
{
387391
let shim_path = bin_dir.join(bin_name);
388392

389-
// Skip if already exists (e.g., re-installing the same package)
390-
if tokio::fs::try_exists(&shim_path).await.unwrap_or(false) {
391-
return Ok(());
393+
// Check if already a managed shim (symlink to ../current/bin/vp)
394+
if let Ok(target) = tokio::fs::read_link(&shim_path).await {
395+
if target == std::path::Path::new("../current/bin/vp") {
396+
return Ok(());
397+
}
398+
// Exists but points elsewhere (e.g., npm-installed direct symlink) — replace it
399+
tokio::fs::remove_file(&shim_path).await?;
392400
}
393401

394402
// Create symlink to ../current/bin/vp

0 commit comments

Comments
 (0)