Skip to content

Commit cb849cd

Browse files
committed
feat(env): make vp env off disable Node.js management for all vp commands (#1255)
Previously, `vp env off` only affected shim dispatch (node/npm/npx invoked directly). All vp commands (create, build, install, etc.) still downloaded managed Node.js via JsExecutor, ignoring the system-first mode setting. Now when `vp env off` is active, JsExecutor checks ShimMode before downloading and uses the system-installed Node.js found in PATH. This fixes NixOS/Guix (where downloaded binaries fail), air-gapped environments, and users managing Node.js via other tools (mise, nvm). - Add JsRuntime::from_system() constructor for system binary paths - Add system-first checks to ensure_cli_runtime/ensure_project_runtime - Expose find_system_tool as pub(crate) and deduplicate doctor.rs copy - Update vp env off/on messaging to reflect broader scope - Add CI E2E tests for system-first mode on all platforms Closes #977
1 parent 09372d9 commit cb849cd

File tree

13 files changed

+256
-82
lines changed

13 files changed

+256
-82
lines changed

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

Lines changed: 60 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use vite_path::{AbsolutePathBuf, current_dir};
77
use vite_shared::{env_vars, output};
88

99
use super::config::{self, ShimMode, get_bin_dir, get_vp_home, load_config, resolve_version};
10-
use crate::error::Error;
10+
use crate::{error::Error, shim};
1111

1212
/// IDE-relevant profile files that GUI-launched applications can see.
1313
/// GUI apps don't run through an interactive shell, so only login/environment
@@ -114,7 +114,7 @@ pub async fn execute(cwd: AbsolutePathBuf) -> Result<ExitStatus, Error> {
114114

115115
// Section: Configuration
116116
print_section("Configuration");
117-
check_shim_mode().await;
117+
let (shim_mode, system_node_path) = check_shim_mode().await;
118118

119119
// Check env sourcing: IDE-relevant profiles first, then all shell profiles
120120
#[cfg(not(windows))]
@@ -128,7 +128,7 @@ pub async fn execute(cwd: AbsolutePathBuf) -> Result<ExitStatus, Error> {
128128

129129
// Section: Version Resolution
130130
print_section("Version Resolution");
131-
check_current_resolution(&cwd).await;
131+
check_current_resolution(&cwd, shim_mode, system_node_path).await;
132132

133133
// Section: Conflicts (conditional)
134134
check_conflicts();
@@ -247,43 +247,48 @@ fn shim_filename(tool: &str) -> String {
247247
}
248248
}
249249

250-
/// Check and display shim mode.
251-
async fn check_shim_mode() {
250+
/// Check and display shim mode. Returns the mode and any found system node path.
251+
async fn check_shim_mode() -> (ShimMode, Option<AbsolutePathBuf>) {
252252
let config = match load_config().await {
253253
Ok(c) => c,
254254
Err(e) => {
255255
print_check(
256256
&output::WARN_SIGN.yellow().to_string(),
257-
"Shim mode",
257+
"Node.js mode",
258258
&format!("config error: {e}").yellow().to_string(),
259259
);
260-
return;
260+
return (ShimMode::default(), None);
261261
}
262262
};
263263

264+
let mut system_node_path = None;
265+
264266
match config.shim_mode {
265267
ShimMode::Managed => {
266-
print_check(&output::CHECK.green().to_string(), "Shim mode", "managed");
268+
print_check(&output::CHECK.green().to_string(), "Node.js mode", "managed");
267269
}
268270
ShimMode::SystemFirst => {
269271
print_check(
270272
&output::CHECK.green().to_string(),
271-
"Shim mode",
273+
"Node.js mode",
272274
&"system-first".bright_blue().to_string(),
273275
);
274276

275277
// Check if system Node.js is available
276-
if let Some(system_node) = find_system_node() {
277-
print_check(" ", "System Node.js", &system_node.display().to_string());
278+
if let Some(system_node) = shim::find_system_tool("node") {
279+
print_check(" ", "System Node.js", &system_node.as_path().display().to_string());
280+
system_node_path = Some(system_node);
278281
} else {
279282
print_check(
280283
&output::WARN_SIGN.yellow().to_string(),
281284
"System Node.js",
282-
&"not found (will use managed)".yellow().to_string(),
285+
&"not found (will fall back to managed)".yellow().to_string(),
283286
);
284287
}
285288
}
286289
}
290+
291+
(config.shim_mode, system_node_path)
287292
}
288293

289294
/// Check profile files for env sourcing and classify where it was found.
@@ -338,36 +343,6 @@ fn check_env_sourcing() -> EnvSourcingStatus {
338343
EnvSourcingStatus::NotFound
339344
}
340345

341-
/// Find system Node.js, skipping vite-plus bin directory and any
342-
/// directories listed in `VP_BYPASS`.
343-
fn find_system_node() -> Option<std::path::PathBuf> {
344-
let bin_dir = get_bin_dir().ok();
345-
let path_var = std::env::var_os("PATH")?;
346-
347-
// Parse VP_BYPASS as a PATH-style list of additional directories to skip
348-
let bypass_paths: Vec<std::path::PathBuf> = std::env::var_os(env_vars::VP_BYPASS)
349-
.map(|v| std::env::split_paths(&v).collect())
350-
.unwrap_or_default();
351-
352-
// Filter PATH to exclude our bin directory and any bypass directories
353-
let filtered_paths: Vec<_> = std::env::split_paths(&path_var)
354-
.filter(|p| {
355-
if let Some(ref bin) = bin_dir {
356-
if p == bin.as_path() {
357-
return false;
358-
}
359-
}
360-
!bypass_paths.iter().any(|bp| p == bp)
361-
})
362-
.collect();
363-
364-
let filtered_path = std::env::join_paths(filtered_paths).ok()?;
365-
366-
// Use vite_command::resolve_bin with filtered PATH - stops at first match
367-
let cwd = current_dir().ok()?;
368-
vite_command::resolve_bin("node", Some(&filtered_path), &cwd).ok().map(|p| p.into_path_buf())
369-
}
370-
371346
/// Check for active session override via VP_NODE_VERSION or session file.
372347
fn check_session_override() {
373348
if let Ok(version) = std::env::var(config::VERSION_ENV_VAR) {
@@ -613,9 +588,35 @@ fn print_ide_setup_guidance(bin_dir: &vite_path::AbsolutePath) {
613588
}
614589

615590
/// Check current directory version resolution.
616-
async fn check_current_resolution(cwd: &AbsolutePathBuf) {
591+
async fn check_current_resolution(
592+
cwd: &AbsolutePathBuf,
593+
shim_mode: ShimMode,
594+
system_node_path: Option<AbsolutePathBuf>,
595+
) {
617596
print_check(" ", "Directory", &cwd.as_path().display().to_string());
618597

598+
// In system-first mode, show system Node.js info instead of managed resolution
599+
if shim_mode == ShimMode::SystemFirst {
600+
if let Some(system_node) = system_node_path {
601+
let version = get_node_version(&system_node).await;
602+
print_check(" ", "Source", "system PATH");
603+
print_check(" ", "Version", &version.bright_green().to_string());
604+
print_check(
605+
&output::CHECK.green().to_string(),
606+
"Node binary",
607+
&system_node.as_path().display().to_string(),
608+
);
609+
} else {
610+
print_check(
611+
&output::WARN_SIGN.yellow().to_string(),
612+
"System Node.js",
613+
&"not found in PATH".yellow().to_string(),
614+
);
615+
print_hint("Install Node.js or run 'vp env on' to use managed Node.js.");
616+
}
617+
return;
618+
}
619+
619620
match resolve_version(cwd).await {
620621
Ok(resolution) => {
621622
let source_display = resolution
@@ -658,6 +659,16 @@ async fn check_current_resolution(cwd: &AbsolutePathBuf) {
658659
}
659660
}
660661

662+
/// Get the version string from a Node.js binary.
663+
async fn get_node_version(node_path: &vite_path::AbsolutePath) -> String {
664+
match tokio::process::Command::new(node_path.as_path()).arg("--version").output().await {
665+
Ok(output) if output.status.success() => {
666+
String::from_utf8_lossy(&output.stdout).trim().to_string()
667+
}
668+
_ => "unknown".to_string(),
669+
}
670+
}
671+
661672
/// Check for conflicts with other version managers.
662673
fn check_conflicts() {
663674
let mut conflicts = Vec::new();
@@ -806,9 +817,12 @@ mod tests {
806817
std::env::set_var(env_vars::VP_BYPASS, dir_a.as_os_str());
807818
}
808819

809-
let result = find_system_node();
820+
let result = shim::find_system_tool("node");
810821
assert!(result.is_some(), "Should find node in non-bypassed directory");
811-
assert!(result.unwrap().starts_with(&dir_b), "Should find node in dir_b, not dir_a");
822+
assert!(
823+
result.unwrap().as_path().starts_with(&dir_b),
824+
"Should find node in dir_b, not dir_a"
825+
);
812826
}
813827

814828
#[test]
@@ -826,7 +840,7 @@ mod tests {
826840
std::env::set_var(env_vars::VP_BYPASS, dir_a.as_os_str());
827841
}
828842

829-
let result = find_system_node();
843+
let result = shim::find_system_tool("node");
830844
assert!(result.is_none(), "Should return None when all paths are bypassed");
831845
}
832846

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,23 +23,23 @@ pub async fn execute() -> Result<ExitStatus, Error> {
2323
let mut config = load_config().await?;
2424

2525
if config.shim_mode == ShimMode::SystemFirst {
26-
println!("Shim mode is already set to system-first.");
26+
println!("Node.js management is already set to system-first.");
2727
println!(
28-
"Shims will prefer system Node.js, falling back to Vite+ managed Node.js if not found."
28+
"All vp commands and shims will prefer system Node.js, falling back to managed if not found."
2929
);
3030
return Ok(ExitStatus::default());
3131
}
3232

3333
config.shim_mode = ShimMode::SystemFirst;
3434
save_config(&config).await?;
3535

36-
println!("\u{2713} Shim mode set to system-first.");
36+
println!("\u{2713} Node.js management set to system-first.");
3737
println!();
3838
println!(
39-
"Shims will now prefer system Node.js, falling back to Vite+ managed Node.js if not found."
39+
"All vp commands and shims will now prefer system Node.js, falling back to managed if not found."
4040
);
4141
println!();
42-
println!("Run {} to always use the Vite+ managed Node.js.", accent_command("vp env on"));
42+
println!("Run {} to always use Vite+ managed Node.js.", accent_command("vp env on"));
4343

4444
Ok(ExitStatus::default())
4545
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,17 @@ pub async fn execute() -> Result<ExitStatus, Error> {
2222
let mut config = load_config().await?;
2323

2424
if config.shim_mode == ShimMode::Managed {
25-
println!("Shim mode is already set to managed.");
26-
println!("Shims will always use the Vite+ managed Node.js.");
25+
println!("Node.js management is already set to managed.");
26+
println!("All vp commands and shims will always use Vite+ managed Node.js.");
2727
return Ok(ExitStatus::default());
2828
}
2929

3030
config.shim_mode = ShimMode::Managed;
3131
save_config(&config).await?;
3232

33-
println!("\u{2713} Shim mode set to managed.");
33+
println!("\u{2713} Node.js management set to managed.");
3434
println!();
35-
println!("Shims will now always use the Vite+ managed Node.js.");
35+
println!("All vp commands and shims will now always use Vite+ managed Node.js.");
3636
println!();
3737
println!("Run {} to prefer system Node.js instead.", accent_command("vp env off"));
3838

crates/vite_global_cli/src/js_executor.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ use vite_js_runtime::{
1313
use vite_path::{AbsolutePath, AbsolutePathBuf};
1414
use vite_shared::{PrependOptions, PrependResult, env_vars, format_path_with_prepend};
1515

16-
use crate::{commands::env::config, error::Error};
16+
use crate::{
17+
commands::env::config::{self, ShimMode},
18+
error::Error,
19+
shim,
20+
};
1721

1822
/// JavaScript executor using managed Node.js runtime.
1923
///
@@ -125,8 +129,15 @@ impl JsExecutor {
125129
///
126130
/// Uses the CLI's package.json `devEngines.runtime` configuration
127131
/// to determine which Node.js version to use.
132+
///
133+
/// When system-first mode is active (`vp env off`), prefers the
134+
/// system-installed Node.js found in PATH.
128135
pub async fn ensure_cli_runtime(&mut self) -> Result<&JsRuntime, Error> {
129136
if self.cli_runtime.is_none() {
137+
if let Some(system_runtime) = find_system_node_runtime().await {
138+
return Ok(self.cli_runtime.insert(system_runtime));
139+
}
140+
130141
let cli_dir = self.get_cli_package_dir()?;
131142
tracing::debug!("Resolving CLI runtime from {:?}", cli_dir);
132143
let runtime = download_runtime_for_project(&cli_dir).await?;
@@ -151,6 +162,10 @@ impl JsExecutor {
151162
if self.project_runtime.is_none() {
152163
tracing::debug!("Resolving project runtime from {:?}", project_path);
153164

165+
if let Some(system_runtime) = find_system_node_runtime().await {
166+
return Ok(self.project_runtime.insert(system_runtime));
167+
}
168+
154169
// 1–2. Session overrides: env var (from `vp env use`), then file
155170
let session_version = vite_shared::EnvConfig::get()
156171
.node_version
@@ -391,6 +406,24 @@ async fn has_valid_version_source(
391406
Ok(engines_valid || dev_engines_valid)
392407
}
393408

409+
/// Try to find system Node.js when in system-first mode (`vp env off`).
410+
///
411+
/// Returns `Some(JsRuntime)` when both conditions are met:
412+
/// 1. Config has `shim_mode == SystemFirst`
413+
/// 2. A system `node` binary is found in PATH (excluding the vite-plus bin directory)
414+
///
415+
/// Returns `None` if mode is `Managed` or no system Node.js is found,
416+
/// allowing the caller to fall through to managed runtime resolution.
417+
async fn find_system_node_runtime() -> Option<JsRuntime> {
418+
let config = config::load_config().await.ok()?;
419+
if config.shim_mode != ShimMode::SystemFirst {
420+
return None;
421+
}
422+
let system_node = shim::find_system_tool("node")?;
423+
tracing::info!("System-first mode: using system Node.js at {:?}", system_node);
424+
Some(JsRuntime::from_system(JsRuntimeType::Node, system_node))
425+
}
426+
394427
#[cfg(test)]
395428
mod tests {
396429
use serial_test::serial;

crates/vite_global_cli/src/shim/dispatch.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1159,7 +1159,7 @@ async fn load_shim_mode() -> ShimMode {
11591159
/// directories listed in `VP_BYPASS`.
11601160
///
11611161
/// Returns the absolute path to the tool if found, None otherwise.
1162-
fn find_system_tool(tool: &str) -> Option<AbsolutePathBuf> {
1162+
pub(crate) fn find_system_tool(tool: &str) -> Option<AbsolutePathBuf> {
11631163
let bin_dir = config::get_bin_dir().ok();
11641164
let path_var = std::env::var_os("PATH")?;
11651165
tracing::debug!("path_var: {:?}", path_var);

crates/vite_global_cli/src/shim/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub(crate) mod exec;
1414

1515
pub(crate) use cache::invalidate_cache;
1616
pub use dispatch::dispatch;
17+
pub(crate) use dispatch::find_system_tool;
1718
use vite_shared::env_vars;
1819

1920
/// Core shim tools (node, npm, npx)

crates/vite_js_runtime/src/runtime.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,28 @@ impl JsRuntime {
6666
pub fn version(&self) -> &str {
6767
&self.version
6868
}
69+
70+
/// Create a `JsRuntime` from a system-installed binary path.
71+
///
72+
/// `get_bin_prefix()` returns the parent directory of `binary_path`.
73+
#[must_use]
74+
pub fn from_system(runtime_type: JsRuntimeType, binary_path: AbsolutePathBuf) -> Self {
75+
let install_dir = binary_path
76+
.parent()
77+
.map(vite_path::AbsolutePath::to_absolute_path_buf)
78+
.unwrap_or_else(|| binary_path.clone());
79+
let binary_filename: Str = Str::from(
80+
binary_path.as_path().file_name().unwrap_or_default().to_string_lossy().as_ref(),
81+
);
82+
debug_assert!(!binary_filename.is_empty(), "binary_path has no filename: {binary_path:?}");
83+
Self {
84+
runtime_type,
85+
version: "system".into(),
86+
install_dir,
87+
binary_relative_path: binary_filename,
88+
bin_dir_relative_path: Str::default(),
89+
}
90+
}
6991
}
7092

7193
/// Download and cache a JavaScript runtime
@@ -557,6 +579,26 @@ mod tests {
557579
assert_eq!(JsRuntimeType::Node.to_string(), "node");
558580
}
559581

582+
#[test]
583+
fn test_js_runtime_from_system() {
584+
let binary_path = AbsolutePathBuf::new(std::path::PathBuf::from(if cfg!(windows) {
585+
"C:\\Program Files\\nodejs\\node.exe"
586+
} else {
587+
"/usr/local/bin/node"
588+
}))
589+
.unwrap();
590+
591+
let runtime = JsRuntime::from_system(JsRuntimeType::Node, binary_path.clone());
592+
593+
assert_eq!(runtime.runtime_type(), JsRuntimeType::Node);
594+
assert_eq!(runtime.version(), "system");
595+
assert_eq!(runtime.get_binary_path(), binary_path);
596+
597+
// bin prefix should be the directory containing the binary
598+
let expected_bin_prefix = binary_path.parent().unwrap().to_absolute_path_buf();
599+
assert_eq!(runtime.get_bin_prefix(), expected_bin_prefix);
600+
}
601+
560602
/// Test that install_dir path is constructed correctly without embedded forward slashes.
561603
/// This ensures Windows compatibility by using separate join() calls.
562604
#[test]

0 commit comments

Comments
 (0)