Skip to content

Commit d43215b

Browse files
authored
fix(env): stop auto-writing .node-version file (#666) (#667)
Remove the auto-write behavior where the shim would create a .node-version file when no version source existed. The file should only be written explicitly via `vp env pin`. closes [VP-208](https://linear.app/voidzero/issue/VP-208/stop-auto-write-node-version-file)
1 parent 3901dfb commit d43215b

21 files changed

Lines changed: 306 additions & 191 deletions

File tree

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

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -258,29 +258,34 @@ pub async fn resolve_version(cwd: &AbsolutePath) -> Result<VersionResolution, Er
258258
});
259259
}
260260

261-
// Invalid .node-version - check package.json sources in the same directory
262-
// This mirrors the fallback logic in download_runtime_for_project()
263-
if matches!(resolution.source, VersionSource::NodeVersionFile) {
261+
// Invalid version from a project source - try lower-priority sources in the same directory.
262+
// This mirrors the fallback logic in download_runtime_for_project().
263+
// - NodeVersionFile: try engines.node, then devEngines.runtime
264+
// - EnginesNode: try devEngines.runtime
265+
if matches!(resolution.source, VersionSource::NodeVersionFile | VersionSource::EnginesNode)
266+
{
264267
if let Some(project_root) = &resolution.project_root {
265268
let package_json_path = project_root.join("package.json");
266269
if let Ok(Some(pkg)) = read_package_json(&package_json_path).await {
267-
// Try engines.node
268-
if let Some(engines_node) = pkg
269-
.engines
270-
.as_ref()
271-
.and_then(|e| e.node.clone())
272-
.and_then(|v| normalize_version(&v, "engines.node"))
273-
{
274-
let resolved = resolve_version_string(&engines_node, &provider).await?;
275-
let is_range = NodeProvider::is_lts_alias(&engines_node)
276-
|| !NodeProvider::is_exact_version(&engines_node);
277-
return Ok(VersionResolution {
278-
version: resolved,
279-
source: "engines.node".into(),
280-
source_path: Some(package_json_path),
281-
project_root: Some(project_root.clone()),
282-
is_range,
283-
});
270+
// Try engines.node (only when falling back from .node-version)
271+
if matches!(resolution.source, VersionSource::NodeVersionFile) {
272+
if let Some(engines_node) = pkg
273+
.engines
274+
.as_ref()
275+
.and_then(|e| e.node.clone())
276+
.and_then(|v| normalize_version(&v, "engines.node"))
277+
{
278+
let resolved = resolve_version_string(&engines_node, &provider).await?;
279+
let is_range = NodeProvider::is_lts_alias(&engines_node)
280+
|| !NodeProvider::is_exact_version(&engines_node);
281+
return Ok(VersionResolution {
282+
version: resolved,
283+
source: "engines.node".into(),
284+
source_path: Some(package_json_path),
285+
project_root: Some(project_root.clone()),
286+
is_range,
287+
});
288+
}
284289
}
285290

286291
// Try devEngines.runtime
@@ -751,6 +756,30 @@ mod tests {
751756
);
752757
}
753758

759+
#[tokio::test]
760+
async fn test_resolve_version_invalid_engines_node_falls_through_to_dev_engines() {
761+
let temp_dir = TempDir::new().unwrap();
762+
let temp_path = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap();
763+
let _guard = vite_shared::EnvConfig::test_guard(
764+
vite_shared::EnvConfig::for_test_with_home(temp_dir.path()),
765+
);
766+
767+
// Create package.json with invalid engines.node but valid devEngines.runtime
768+
// No .node-version file — resolve_node_version returns EnginesNode source
769+
let package_json = r#"{"engines":{"node":"invalid"},"devEngines":{"runtime":{"name":"node","version":"^20.18.0"}}}"#;
770+
tokio::fs::write(temp_path.join("package.json"), package_json).await.unwrap();
771+
772+
// resolve_version should fall through from invalid engines.node to devEngines.runtime
773+
let resolution = resolve_version(&temp_path).await.unwrap();
774+
775+
assert_eq!(resolution.source, "devEngines.runtime");
776+
assert!(
777+
resolution.version.starts_with("20."),
778+
"Expected version to start with '20.', got: {}",
779+
resolution.version
780+
);
781+
}
782+
754783
#[tokio::test]
755784
async fn test_resolve_version_latest_alias_in_node_version() {
756785
let temp_dir = TempDir::new().unwrap();

crates/vite_global_cli/src/js_executor.rs

Lines changed: 88 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
use std::process::ExitStatus;
77

88
use tokio::process::Command;
9-
use vite_js_runtime::{JsRuntime, JsRuntimeType, download_runtime, download_runtime_for_project};
9+
use vite_js_runtime::{
10+
JsRuntime, JsRuntimeType, download_runtime, download_runtime_for_project, is_valid_version,
11+
read_package_json, resolve_node_version,
12+
};
1013
use vite_path::{AbsolutePath, AbsolutePathBuf};
1114
use vite_shared::{PrependOptions, PrependResult, env_vars, format_path_with_prepend};
1215

13-
use crate::error::Error;
16+
use crate::{commands::env::config, error::Error};
1417

1518
/// JavaScript executor using managed Node.js runtime.
1619
///
@@ -134,15 +137,52 @@ impl JsExecutor {
134137

135138
/// Ensure the project runtime is downloaded and cached.
136139
///
137-
/// Uses the project's package.json `devEngines.runtime` configuration
138-
/// to determine which Node.js version to use.
140+
/// Resolution order:
141+
/// 1. Session override (env var from `vp env use`)
142+
/// 2. Session override (file from `vp env use`)
143+
/// 3. Project sources (.node-version, engines.node, devEngines.runtime) —
144+
/// delegates to `download_runtime_for_project()` for cache-aware resolution
145+
/// 4. User default from config.json
146+
/// 5. Latest LTS
139147
pub async fn ensure_project_runtime(
140148
&mut self,
141149
project_path: &AbsolutePath,
142150
) -> Result<&JsRuntime, Error> {
143151
if self.project_runtime.is_none() {
144152
tracing::debug!("Resolving project runtime from {:?}", project_path);
145-
let runtime = download_runtime_for_project(project_path).await?;
153+
154+
// 1–2. Session overrides: env var (from `vp env use`), then file
155+
let session_version = vite_shared::EnvConfig::get()
156+
.node_version
157+
.map(|v| v.trim().to_string())
158+
.filter(|v| !v.is_empty());
159+
let session_version = if session_version.is_some() {
160+
session_version
161+
} else {
162+
config::read_session_version().await
163+
};
164+
if let Some(version) = session_version {
165+
let runtime = download_runtime(JsRuntimeType::Node, &version).await?;
166+
return Ok(self.project_runtime.insert(runtime));
167+
}
168+
169+
// 3. Check if project has any *valid* version source.
170+
// resolve_node_version returns Some for any non-empty value,
171+
// even invalid ones. We must validate before routing to
172+
// download_runtime_for_project, which falls to LTS on all-invalid
173+
// and would skip the user's configured default.
174+
let has_valid_project_source = has_valid_version_source(project_path).await?;
175+
176+
let runtime = if has_valid_project_source {
177+
// At least one valid project source exists — delegate to
178+
// download_runtime_for_project for cache-aware range resolution
179+
// and intra-project fallback chain
180+
download_runtime_for_project(project_path).await?
181+
} else {
182+
// No valid project source — check user default from config, then LTS
183+
let resolution = config::resolve_version(project_path).await?;
184+
download_runtime(JsRuntimeType::Node, &resolution.version).await?
185+
};
146186
self.project_runtime = Some(runtime);
147187
}
148188
Ok(self.project_runtime.as_ref().unwrap())
@@ -163,8 +203,7 @@ impl JsExecutor {
163203
/// If found, runs the local `dist/bin.js` directly. Otherwise, falls back
164204
/// to the global installation's `dist/bin.js`.
165205
///
166-
/// Uses the project's runtime (from its `devEngines.runtime` configuration).
167-
/// This may write a `.node-version` file if the project has no version source.
206+
/// Uses the project's runtime resolved via `config::resolve_version()`.
168207
/// For side-effect-free commands like `--version`, use [`delegate_with_cli_runtime`] instead.
169208
///
170209
/// # Arguments
@@ -252,6 +291,48 @@ impl JsExecutor {
252291
}
253292
}
254293

294+
/// Check whether a project directory has at least one valid version source.
295+
///
296+
/// Uses `is_valid_version` (no warning side effects) to avoid duplicate
297+
/// warnings when `download_runtime_for_project` or `config::resolve_version`
298+
/// later call `normalize_version` on the same values.
299+
///
300+
/// Returns `false` when all sources are missing or invalid, so the caller
301+
/// can fall through to the user's configured default instead of LTS.
302+
async fn has_valid_version_source(
303+
project_path: &AbsolutePath,
304+
) -> Result<bool, vite_js_runtime::Error> {
305+
let resolution = resolve_node_version(project_path, true).await?;
306+
let Some(ref r) = resolution else {
307+
return Ok(false);
308+
};
309+
310+
// Primary source is a valid version?
311+
if is_valid_version(&r.version) {
312+
return Ok(true);
313+
}
314+
315+
// Primary source invalid — check package.json for valid fallbacks
316+
let pkg_path = project_path.join("package.json");
317+
let Ok(Some(pkg)) = read_package_json(&pkg_path).await else {
318+
return Ok(false);
319+
};
320+
321+
let engines_valid =
322+
pkg.engines.as_ref().and_then(|e| e.node.as_ref()).is_some_and(|v| is_valid_version(v));
323+
324+
let dev_engines_valid = !engines_valid
325+
&& pkg
326+
.dev_engines
327+
.as_ref()
328+
.and_then(|de| de.runtime.as_ref())
329+
.and_then(|rt| rt.find_by_name("node"))
330+
.filter(|r| !r.version.is_empty())
331+
.is_some_and(|r| is_valid_version(&r.version));
332+
333+
Ok(engines_valid || dev_engines_valid)
334+
}
335+
255336
#[cfg(test)]
256337
mod tests {
257338
use serial_test::serial;

crates/vite_js_runtime/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,6 @@ pub use provider::{ArchiveFormat, DownloadInfo, HashVerification, JsRuntimeProvi
5151
pub use providers::{LtsInfo, NodeProvider, NodeVersionEntry};
5252
pub use runtime::{
5353
JsRuntime, JsRuntimeType, VersionResolution, VersionSource, download_runtime,
54-
download_runtime_for_project, download_runtime_with_provider, normalize_version,
55-
read_package_json, resolve_node_version,
54+
download_runtime_for_project, download_runtime_with_provider, is_valid_version,
55+
normalize_version, read_package_json, resolve_node_version,
5656
};

0 commit comments

Comments
 (0)