Skip to content

Commit 2fd446f

Browse files
leno23fengmk2
andauthored
fix(global): skip current packages on update (#1596)
## Summary - Skip `vp update -g` installs when the recorded global package version already matches the version resolved from npm. - Resolve the target registry version with the package's recorded Node runtime so checks match the package's managed environment. - Support npm `view ... version --json` outputs as strings, plain strings, and version arrays. Fixes #1477 Related #1476 ## Test Plan - `cargo fmt` - `cargo test -p vite_global_cli global_update --no-default-features` - `cargo test -p vite_global_cli updates_global_package_when_registry_version_differs_from_installed_version --no-default-features` - `cargo test -p vite_global_cli parse_npm_view_version --no-default-features` - `cargo test -p vite_global_cli --no-default-features --no-run` - `cargo clippy -p vite_global_cli --no-default-features -- -D warnings` Note: `cargo test -p vite_global_cli --no-default-features` aborts locally in the existing `tests::unknown_argument_detected_with_pass_as_value_hint` test with a stack overflow after the targeted tests pass. --------- Co-authored-by: MK (fengmk2) <fengmk2@gmail.com>
1 parent 9f718e7 commit 2fd446f

2 files changed

Lines changed: 247 additions & 10 deletions

File tree

crates/vite_global_cli/src/cli.rs

Lines changed: 128 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::{ffi::OsStr, process::ExitStatus};
88
use clap::{CommandFactory, FromArgMatches, Parser, Subcommand};
99
use clap_complete::ArgValueCompleter;
1010
use tokio::runtime::Runtime;
11-
use vite_path::AbsolutePathBuf;
11+
use vite_path::{AbsolutePath, AbsolutePathBuf};
1212
use vite_pm_cli::PackageManagerCommand;
1313

1414
use crate::{commands, error::Error, help};
@@ -528,7 +528,7 @@ async fn run_package_manager_command(
528528
}
529529

530530
PackageManagerCommand::Update { global: true, ref packages, .. } => {
531-
managed_update(packages).await
531+
managed_update(&cwd, packages).await
532532
}
533533

534534
// `pm list -g` lists vite-plus-managed globals, not the underlying PM's.
@@ -573,21 +573,123 @@ async fn managed_uninstall(packages: &[String], dry_run: bool) -> Result<ExitSta
573573
Ok(ExitStatus::default())
574574
}
575575

576-
async fn managed_update(packages: &[String]) -> Result<ExitStatus, Error> {
577-
use crate::commands::env::package_metadata::PackageMetadata;
576+
fn is_global_package_up_to_date(
577+
installed_version: &str,
578+
registry_version: &str,
579+
installed_node_version: &str,
580+
target_node_version: &str,
581+
) -> bool {
582+
installed_version.trim() == registry_version.trim()
583+
&& installed_node_version.trim() == target_node_version.trim()
584+
}
585+
586+
async fn managed_update(cwd: &AbsolutePath, packages: &[String]) -> Result<ExitStatus, Error> {
587+
use crate::commands::env::{
588+
config::resolve_version, global_install, package_metadata::PackageMetadata,
589+
};
578590

579-
let to_update: Vec<String> = if packages.is_empty() {
591+
let all_packages = if packages.is_empty() {
580592
let all = PackageMetadata::list_all().await?;
581593
if all.is_empty() {
582594
vite_shared::output::raw("No global packages installed.");
583595
return Ok(ExitStatus::default());
584596
}
585-
all.iter().map(|p| p.name.clone()).collect()
597+
Some(all)
586598
} else {
587-
packages.to_vec()
599+
None
588600
};
601+
602+
let target_node_version = resolve_version(cwd).await?.version;
603+
let mut to_update: Vec<String> = Vec::new();
604+
let mut skipped = 0usize;
605+
606+
if let Some(all) = all_packages {
607+
for metadata in all {
608+
if metadata.platform.node.trim() != target_node_version.trim() {
609+
to_update.push(metadata.name.clone());
610+
continue;
611+
}
612+
613+
match global_install::latest_package_version(&metadata.name, Some(&target_node_version))
614+
.await
615+
{
616+
Ok(latest_version)
617+
if is_global_package_up_to_date(
618+
&metadata.version,
619+
&latest_version,
620+
&metadata.platform.node,
621+
&target_node_version,
622+
) =>
623+
{
624+
vite_shared::output::raw(&format!(
625+
"{} is already up to date (v{}).",
626+
metadata.name, metadata.version
627+
));
628+
skipped += 1;
629+
}
630+
Ok(_) => to_update.push(metadata.name.clone()),
631+
Err(e) => {
632+
vite_shared::output::raw_stderr(&format!(
633+
"Could not check latest version for {}: {e}; updating anyway.",
634+
metadata.name
635+
));
636+
to_update.push(metadata.name.clone());
637+
}
638+
}
639+
}
640+
} else {
641+
for package in packages {
642+
if global_install::is_local_package_spec(package) {
643+
to_update.push(package.clone());
644+
continue;
645+
}
646+
647+
let (package_name, _) = global_install::parse_package_spec(package);
648+
if let Some(metadata) = PackageMetadata::load(&package_name).await? {
649+
if metadata.platform.node.trim() != target_node_version.trim() {
650+
to_update.push(package.clone());
651+
continue;
652+
}
653+
654+
match global_install::latest_package_version(package, Some(&target_node_version))
655+
.await
656+
{
657+
Ok(latest_version)
658+
if is_global_package_up_to_date(
659+
&metadata.version,
660+
&latest_version,
661+
&metadata.platform.node,
662+
&target_node_version,
663+
) =>
664+
{
665+
vite_shared::output::raw(&format!(
666+
"{} is already up to date (v{}).",
667+
package_name, metadata.version
668+
));
669+
skipped += 1;
670+
continue;
671+
}
672+
Ok(_) => {}
673+
Err(e) => {
674+
vite_shared::output::raw_stderr(&format!(
675+
"Could not check latest version for {package}: {e}; updating anyway."
676+
));
677+
}
678+
}
679+
}
680+
to_update.push(package.clone());
681+
}
682+
}
683+
684+
if to_update.is_empty() {
685+
if skipped > 0 {
686+
vite_shared::output::raw("All global packages are up to date.");
687+
}
688+
return Ok(ExitStatus::default());
689+
}
690+
589691
for package in &to_update {
590-
if let Err(e) = crate::commands::env::global_install::install(package, None, false).await {
692+
if let Err(e) = global_install::install(package, Some(&target_node_version), false).await {
591693
vite_shared::output::raw_stderr(&format!("Failed to update {package}: {e}"));
592694
return Ok(exit_status(1));
593695
}
@@ -829,7 +931,24 @@ pub fn try_parse_args_from_with_options(
829931

830932
#[cfg(test)]
831933
mod tests {
832-
use super::{has_flag_before_terminator, should_force_global_delegate};
934+
use super::{
935+
has_flag_before_terminator, is_global_package_up_to_date, should_force_global_delegate,
936+
};
937+
938+
#[test]
939+
fn skips_global_update_when_registry_and_node_versions_match() {
940+
assert!(is_global_package_up_to_date("5.9.3", "5.9.3", "20.18.0", "20.18.0"));
941+
}
942+
943+
#[test]
944+
fn updates_global_package_when_registry_version_differs_from_installed_version() {
945+
assert!(!is_global_package_up_to_date("5.9.2", "5.9.3", "20.18.0", "20.18.0"));
946+
}
947+
948+
#[test]
949+
fn updates_global_package_when_node_version_differs_from_target_version() {
950+
assert!(!is_global_package_up_to_date("5.9.3", "5.9.3", "20.18.0", "24.15.0"));
951+
}
833952

834953
#[test]
835954
fn detects_flag_before_option_terminator() {

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

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,85 @@ pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
267267
Ok(())
268268
}
269269

270+
/// Resolve the version currently published for a package spec.
271+
///
272+
/// `package_spec` may be a bare package name (`typescript`) or include a
273+
/// version/tag (`typescript@beta`, `@scope/pkg@1.0.0`). The command returns the
274+
/// version that npm resolves for that spec.
275+
pub(crate) async fn latest_package_version(
276+
package_spec: &str,
277+
node_version: Option<&str>,
278+
) -> Result<String, Error> {
279+
let version = if let Some(v) = node_version {
280+
let provider = NodeProvider::new();
281+
resolve_version_alias(v, &provider).await?
282+
} else {
283+
let cwd = current_dir().map_err(|e| {
284+
Error::ConfigError(format!("Cannot get current directory: {}", e).into())
285+
})?;
286+
let resolution = resolve_version(&cwd).await?;
287+
resolution.version
288+
};
289+
290+
let runtime =
291+
vite_js_runtime::download_runtime(vite_js_runtime::JsRuntimeType::Node, &version).await?;
292+
let node_bin_dir = runtime.get_bin_prefix();
293+
let npm_path =
294+
if cfg!(windows) { node_bin_dir.join("npm.cmd") } else { node_bin_dir.join("npm") };
295+
296+
let output = Command::new(npm_path.as_path())
297+
.args(["view", package_spec, "version", "--json"])
298+
.env("PATH", format_path_prepended(node_bin_dir.as_path()))
299+
.stdout(Stdio::piped())
300+
.stderr(Stdio::piped())
301+
.output()
302+
.await?;
303+
304+
if !output.status.success() {
305+
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
306+
return Err(Error::ConfigError(
307+
format!("npm view failed for {package_spec}: {stderr}").into(),
308+
));
309+
}
310+
311+
parse_npm_view_version(&output.stdout)
312+
}
313+
314+
fn parse_npm_view_version(stdout: &[u8]) -> Result<String, Error> {
315+
let raw = String::from_utf8_lossy(stdout);
316+
let trimmed = raw.trim();
317+
if trimmed.is_empty() {
318+
return Err(Error::ConfigError("npm view returned an empty version".into()));
319+
}
320+
321+
match serde_json::from_str::<serde_json::Value>(trimmed) {
322+
Ok(serde_json::Value::String(version)) => Ok(version),
323+
Ok(serde_json::Value::Array(versions)) => versions
324+
.iter()
325+
.rev()
326+
.find_map(|version| version.as_str())
327+
.map(str::to_string)
328+
.ok_or_else(|| Error::ConfigError("npm view returned an empty version list".into())),
329+
_ => Ok(trimmed.to_string()),
330+
}
331+
}
332+
333+
/// Return true for package specs that refer to local filesystem content.
334+
pub(crate) fn is_local_package_spec(spec: &str) -> bool {
335+
spec == "."
336+
|| spec == ".."
337+
|| spec.starts_with("./")
338+
|| spec.starts_with("../")
339+
|| spec.starts_with('/')
340+
|| spec.starts_with("file:")
341+
|| (cfg!(windows)
342+
&& spec.len() >= 3
343+
&& spec.as_bytes()[1] == b':'
344+
&& (spec.as_bytes()[2] == b'\\' || spec.as_bytes()[2] == b'/'))
345+
}
346+
270347
/// Parse package spec into name and optional version.
271-
fn parse_package_spec(spec: &str) -> (String, Option<String>) {
348+
pub(crate) fn parse_package_spec(spec: &str) -> (String, Option<String>) {
272349
// Handle scoped packages: @scope/name@version
273350
if spec.starts_with('@') {
274351
// Find the second @ for version
@@ -699,6 +776,47 @@ mod tests {
699776
}
700777
}
701778

779+
#[test]
780+
fn test_parse_npm_view_version_json_string() {
781+
let version = parse_npm_view_version(b"\"5.9.3\"\n").unwrap();
782+
assert_eq!(version, "5.9.3");
783+
}
784+
785+
#[test]
786+
fn test_parse_npm_view_version_plain_string() {
787+
let version = parse_npm_view_version(b"5.9.3\n").unwrap();
788+
assert_eq!(version, "5.9.3");
789+
}
790+
791+
#[test]
792+
fn test_parse_npm_view_version_json_array_uses_latest_entry() {
793+
let version = parse_npm_view_version(b"[\"5.9.2\", \"5.9.3\"]\n").unwrap();
794+
assert_eq!(version, "5.9.3");
795+
}
796+
797+
#[test]
798+
fn test_parse_npm_view_version_rejects_empty_output() {
799+
let err = parse_npm_view_version(b"\n").unwrap_err();
800+
assert!(err.to_string().contains("empty version"));
801+
}
802+
803+
#[test]
804+
fn test_is_local_package_spec_relative_paths() {
805+
assert!(is_local_package_spec("."));
806+
assert!(is_local_package_spec(".."));
807+
assert!(is_local_package_spec("./pkg"));
808+
assert!(is_local_package_spec("../pkg"));
809+
assert!(is_local_package_spec("file:../pkg"));
810+
}
811+
812+
#[test]
813+
fn test_is_local_package_spec_registry_packages() {
814+
assert!(!is_local_package_spec("typescript"));
815+
assert!(!is_local_package_spec("typescript@5.9.3"));
816+
assert!(!is_local_package_spec("@scope/pkg"));
817+
assert!(!is_local_package_spec("@scope/pkg@1.0.0"));
818+
}
819+
702820
#[test]
703821
fn test_parse_package_spec_simple() {
704822
let (name, version) = parse_package_spec("typescript");

0 commit comments

Comments
 (0)