Skip to content

Commit f763568

Browse files
committed
feat(env): make vp env off disable Node.js management for all vp commands
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 ced26cd commit f763568

8 files changed

Lines changed: 146 additions & 50 deletions

File tree

.github/workflows/ci.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,11 @@ jobs:
235235
if: ${{ matrix.target == 'x86_64-unknown-linux-gnu' }}
236236
run: pnpm tsgo
237237

238+
- name: Save system Node.js info
239+
run: |
240+
echo "SYSTEM_NODE_PATH=$(which node)" >> "$GITHUB_ENV"
241+
echo "SYSTEM_NODE_VERSION=$(node --version)" >> "$GITHUB_ENV"
242+
238243
- name: Install Global CLI vp
239244
run: |
240245
pnpm bootstrap-cli:ci
@@ -258,6 +263,28 @@ jobs:
258263
env:
259264
RUST_MIN_STACK: 8388608
260265

266+
- name: Test vp env off (system-first mode)
267+
run: |
268+
echo "System Node.js: $SYSTEM_NODE_PATH ($SYSTEM_NODE_VERSION)"
269+
270+
vp env off
271+
vp env doctor
272+
273+
# Verify vp --version still works using system Node.js
274+
vp --version
275+
276+
# In system-first mode, the node shim should resolve to system node
277+
SHIM_NODE_VERSION=$(node --version)
278+
echo "Shim Node.js version: $SHIM_NODE_VERSION"
279+
if [ "$SHIM_NODE_VERSION" != "$SYSTEM_NODE_VERSION" ]; then
280+
echo "Error: expected shim to use system Node.js $SYSTEM_NODE_VERSION, got $SHIM_NODE_VERSION"
281+
exit 1
282+
fi
283+
284+
vp env on
285+
vp env doctor
286+
echo "vp env off/on test passed"
287+
261288
- name: Test global package install (powershell)
262289
if: ${{ matrix.os == 'windows-latest' }}
263290
shell: pwsh
@@ -298,6 +325,27 @@ jobs:
298325
vp env use --unset
299326
node --version
300327
328+
- name: Test vp env off (powershell)
329+
if: ${{ matrix.os == 'windows-latest' }}
330+
shell: pwsh
331+
run: |
332+
Write-Host "System Node.js: $env:SYSTEM_NODE_PATH ($env:SYSTEM_NODE_VERSION)"
333+
334+
vp env off
335+
vp env doctor
336+
vp --version
337+
338+
$ShimNodeVersion = node --version
339+
Write-Host "Shim Node.js version: $ShimNodeVersion"
340+
if ($ShimNodeVersion -ne $env:SYSTEM_NODE_VERSION) {
341+
Write-Error "Expected shim to use system Node.js $env:SYSTEM_NODE_VERSION, got $ShimNodeVersion"
342+
exit 1
343+
}
344+
345+
vp env on
346+
vp env doctor
347+
Write-Host "vp env off/on test passed"
348+
301349
- name: Test global package install (cmd)
302350
if: ${{ matrix.os == 'windows-latest' }}
303351
shell: cmd

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

Lines changed: 12 additions & 39 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
@@ -263,23 +263,23 @@ async fn check_shim_mode() {
263263

264264
match config.shim_mode {
265265
ShimMode::Managed => {
266-
print_check(&output::CHECK.green().to_string(), "Shim mode", "managed");
266+
print_check(&output::CHECK.green().to_string(), "Node.js mode", "managed");
267267
}
268268
ShimMode::SystemFirst => {
269269
print_check(
270270
&output::CHECK.green().to_string(),
271-
"Shim mode",
271+
"Node.js mode",
272272
&"system-first".bright_blue().to_string(),
273273
);
274274

275275
// 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());
276+
if let Some(system_node) = shim::find_system_tool("node") {
277+
print_check(" ", "System Node.js", &system_node.as_path().display().to_string());
278278
} else {
279279
print_check(
280280
&output::WARN_SIGN.yellow().to_string(),
281281
"System Node.js",
282-
&"not found (will use managed)".yellow().to_string(),
282+
&"not found (will fall back to managed)".yellow().to_string(),
283283
);
284284
}
285285
}
@@ -338,36 +338,6 @@ fn check_env_sourcing() -> EnvSourcingStatus {
338338
EnvSourcingStatus::NotFound
339339
}
340340

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-
371341
/// Check for active session override via VP_NODE_VERSION or session file.
372342
fn check_session_override() {
373343
if let Ok(version) = std::env::var(config::VERSION_ENV_VAR) {
@@ -806,9 +776,12 @@ mod tests {
806776
std::env::set_var(env_vars::VP_BYPASS, dir_a.as_os_str());
807777
}
808778

809-
let result = find_system_node();
779+
let result = shim::find_system_tool("node");
810780
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");
781+
assert!(
782+
result.unwrap().as_path().starts_with(&dir_b),
783+
"Should find node in dir_b, not dir_a"
784+
);
812785
}
813786

814787
#[test]
@@ -826,7 +799,7 @@ mod tests {
826799
std::env::set_var(env_vars::VP_BYPASS, dir_a.as_os_str());
827800
}
828801

829-
let result = find_system_node();
802+
let result = shim::find_system_tool("node");
830803
assert!(result.is_none(), "Should return None when all paths are bypassed");
831804
}
832805

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: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ 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+
Self {
83+
runtime_type,
84+
version: "system".into(),
85+
install_dir,
86+
binary_relative_path: binary_filename,
87+
bin_dir_relative_path: Str::default(),
88+
}
89+
}
6990
}
7091

7192
/// Download and cache a JavaScript runtime
@@ -557,6 +578,26 @@ mod tests {
557578
assert_eq!(JsRuntimeType::Node.to_string(), "node");
558579
}
559580

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

0 commit comments

Comments
 (0)