Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/vite_global_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ clap = { workspace = true, features = ["derive"] }
clap_complete = { workspace = true, features = ["unstable-dynamic"] }
directories = { workspace = true }
futures = { workspace = true }
flate2 = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
node-semver = { workspace = true }
thiserror = { workspace = true }
tar = { workspace = true }
tokio = { workspace = true, features = ["full"] }
tracing = { workspace = true }
owo-colors = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion crates/vite_global_cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,8 @@ async fn managed_update(
continue;
}

let (package_name, _) = global::parse_package_spec(package);
// It is not a local package, so `parse_package_spec` there won't return `Err()`
let (package_name, _) = global::parse_package_spec(package).unwrap();
if PackageMetadata::load(&package_name).await?.is_some() {
managed_specs.push(package.clone());
} else {
Expand Down
29 changes: 22 additions & 7 deletions crates/vite_global_cli/src/commands/global/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
},
package_metadata::PackageMetadata,
},
global::{CORE_SHIMS, parse_package_spec},
global::{CORE_SHIMS, is_local_package_spec, parse_package_spec},
},
error::Error,
};
Expand Down Expand Up @@ -105,7 +105,11 @@ pub async fn install(
let mut packages = IndexMap::<String, Package>::new();
for package_spec in package_specs {
// Parse package spec (e.g., "typescript", "typescript@5.0.0", "@scope/pkg")
let (package_name, _version_spec) = parse_package_spec(package_spec);

let (package_name, _version_spec) = match parse_package_spec(package_spec) {
Ok(result) => result,
Err(error) => return Err((Some(package_spec.clone()), error)),
};
packages.insert(package_name, Package { spec: package_spec, staging_dir: None });
}
let packages_count = packages.len();
Expand Down Expand Up @@ -395,7 +399,18 @@ async fn install_one(
/// 1. Try to use PackageMetadata for binary list
/// 2. Fallback to scanning BinConfig files for orphaned binaries
pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
let (package_name, _) = parse_package_spec(package_name);
if is_local_package_spec(package_name) {
// We can't resolve local packages for uninstall, follow npm's behavior
return Err(Error::ConfigError(
format!(
Comment on lines +402 to +405
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve path-based uninstall for legacy local installs

This early return blocks vp remove -g ./local-path before any metadata/bin lookup, which breaks uninstall for packages installed by older versions that stored the local spec path as the managed package key (the behavior this commit is replacing). After upgrading, those existing installs can no longer be removed via vp by either path (now rejected here) or name (metadata/bin ownership still recorded under the old path), leaving orphaned shims and package state that can block subsequent installs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it will influence vp install -g --force's uninstall logic, but won't influence normal vp uninstall -g <pkg_name>, as we do not use BinConfig there. I've tested locally and it can still remove a global package

$ vp install -g ./conflict-pkg
VITE+ - The Unified Toolchain for the Web

info: Installing 1 global package with Node.js 24.16.0
warn: Package './conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed ./conflict-pkg 1.0.0
  Bins: conflict-cli, node

~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!]
$ pnpm bootstrap-cli
...

~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!] took 1m37s
$ vp uninstall -g conflict-pkg
Uninstalled conflict-pkg

tip: Available short aliases: i = install, rm = remove, un = uninstall, up = update, ls = list, ln = link

~/code/voidzero-dev/vite-plus on fix/check-real-package-json-for-global-install [!]
$ vp list -g conflict-pkg
No global packages matching 'conflict-pkg'.

Run 'vp list -g' to see all installed global packages.

"Local path {} can't be resolved, please enter a package name instead",
Comment thread
liangmiQwQ marked this conversation as resolved.
package_name
)
.into(),
));
}

let (package_name, _) = parse_package_spec(package_name).unwrap();

// Phase 1: Try to use PackageMetadata for binary list
let bins = if let Some(metadata) = PackageMetadata::load(&package_name).await? {
Expand Down Expand Up @@ -881,28 +896,28 @@ mod tests {

#[test]
fn test_parse_package_spec_simple() {
let (name, version) = parse_package_spec("typescript");
let (name, version) = parse_package_spec("typescript").unwrap();
assert_eq!(name, "typescript");
assert_eq!(version, None);
}

#[test]
fn test_parse_package_spec_with_version() {
let (name, version) = parse_package_spec("typescript@5.0.0");
let (name, version) = parse_package_spec("typescript@5.0.0").unwrap();
assert_eq!(name, "typescript");
assert_eq!(version, Some("5.0.0".to_string()));
}

#[test]
fn test_parse_package_spec_scoped() {
let (name, version) = parse_package_spec("@types/node");
let (name, version) = parse_package_spec("@types/node").unwrap();
assert_eq!(name, "@types/node");
assert_eq!(version, None);
}

#[test]
fn test_parse_package_spec_scoped_with_version() {
let (name, version) = parse_package_spec("@types/node@20.0.0");
let (name, version) = parse_package_spec("@types/node@20.0.0").unwrap();
assert_eq!(name, "@types/node");
assert_eq!(version, Some("20.0.0".to_string()));
}
Expand Down
168 changes: 142 additions & 26 deletions crates/vite_global_cli/src/commands/global/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
//! Managed global package utilities.

use std::{collections::HashMap, io::IsTerminal, process::Stdio, time::Duration};

use std::{
collections::HashMap,
fs::File,
io::{IsTerminal, Read},
process::Stdio,
time::Duration,
};

use flate2::read::GzDecoder;
use futures::{StreamExt, stream::FuturesUnordered};
use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle};
use tar::Archive;
use tokio::process::Command;
use vite_path::{AbsolutePathBuf, current_dir};
use vite_shared::format_path_prepended;
Expand Down Expand Up @@ -48,23 +56,34 @@ impl NpmRegistry {
}

async fn latest_package_version(&self, package_spec: &str) -> Result<String, Error> {
let output = Command::new(self.npm_path.as_path())
.args(["view", package_spec, "version", "--json"])
.env("PATH", format_path_prepended(self.node_bin_dir.as_path()))
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.output()
.await?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
return Err(Error::ConfigError(
format!("npm view failed for {package_spec}: {stderr}").into(),
));
}
let output = npm_view(&self.npm_path, &self.node_bin_dir, package_spec, "version").await?;

parse_npm_view_version(&output)
}
}

async fn npm_view(
npm_path: &AbsolutePathBuf,
node_bin_dir: &AbsolutePathBuf,
package_spec: &str,
field: &str,
) -> Result<Vec<u8>, Error> {
let output = Command::new(npm_path.as_path())
.args(["view", package_spec, field, "--json"])
.env("PATH", format_path_prepended(node_bin_dir.as_path()))
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.output()
.await?;

parse_npm_view_version(&output.stdout)
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
return Err(Error::ConfigError(
format!("npm view failed for {package_spec}: {stderr}").into(),
));
}

Ok(output.stdout)
}

pub(crate) async fn latest_package_versions(
Expand Down Expand Up @@ -133,20 +152,117 @@ pub(crate) fn is_local_package_spec(spec: &str) -> bool {
}

/// Parse package spec into name and optional version.
pub(crate) fn parse_package_spec(spec: &str) -> (String, Option<String>) {
if spec.starts_with('@') {
if let Some(idx) = spec[1..].find('@') {
let idx = idx + 1;
return (spec[..idx].to_string(), Some(spec[idx + 1..].to_string()));
/// For local packages, read package.json from a directory or package tarball.
///
/// It will never return an `Err()` if it is not a local package
pub(crate) fn parse_package_spec(spec: &str) -> Result<(String, Option<String>), Error> {
if is_local_package_spec(spec) {
let package_json = read_local_package_json(spec)?;
let Some(package_name) = package_json.get("name").and_then(|name| name.as_str()) else {
return Err(Error::ConfigError(
format!("Local package {spec} must have a string name in package.json").into(),
));
};

Ok((package_name.to_string(), None))
} else {
if spec.starts_with('@') {
if let Some(idx) = spec[1..].find('@') {
let idx = idx + 1;
return Ok((spec[..idx].to_string(), Some(spec[idx + 1..].to_string())));
}
return Ok((spec.to_string(), None));
}
return (spec.to_string(), None);

if let Some(idx) = spec.find('@') {
return Ok((spec[..idx].to_string(), Some(spec[idx + 1..].to_string())));
}

Ok((spec.to_string(), None))
}
}

if let Some(idx) = spec.find('@') {
return (spec[..idx].to_string(), Some(spec[idx + 1..].to_string()));
fn resolve_local_package_path(spec: &str) -> Result<AbsolutePathBuf, Error> {
let path_spec = spec.strip_prefix("file:").unwrap_or(spec);
let path = std::path::Path::new(path_spec);
if path.is_absolute() {
AbsolutePathBuf::new(path.to_path_buf())
.ok_or_else(|| Error::ConfigError(format!("Invalid local package path {spec}").into()))
} else {
Ok(current_dir()
.map_err(|error| {
Error::ConfigError(format!("Cannot get current directory: {error}").into())
})?
.join(path))
}
}

fn read_local_package_json(spec: &str) -> Result<serde_json::Value, Error> {
let package_path = resolve_local_package_path(spec)?;
if package_path.as_path().is_file() && is_package_tarball(package_path.as_path()) {
return read_package_json_from_tarball(spec, &package_path);
}

let package_json_path = package_path.join("package.json");
let package_json_content =
std::fs::read_to_string(package_json_path.as_path()).map_err(|error| {
Error::ConfigError(
format!(
"Failed to read package.json for local package {spec} at {}: {error}",
package_json_path.as_path().display()
)
.into(),
)
})?;
serde_json::from_str(&package_json_content).map_err(Error::JsonError)
}

fn is_package_tarball(path: &std::path::Path) -> bool {
let path = path.to_string_lossy();
path.ends_with(".tgz") || path.ends_with(".tar.gz")
}

fn read_package_json_from_tarball(
spec: &str,
package_path: &AbsolutePathBuf,
) -> Result<serde_json::Value, Error> {
let file = File::open(package_path.as_path()).map_err(|error| {
Error::ConfigError(
format!(
"Failed to read package tarball {spec} at {}: {error}",
package_path.as_path().display()
)
.into(),
)
})?;
let decoder = GzDecoder::new(file);
let mut archive = Archive::new(decoder);

for entry in archive.entries().map_err(|error| {
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
})? {
let mut entry = entry.map_err(|error| {
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
})?;
let path = entry.path().map_err(|error| {
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
})?;
if path.as_ref() != std::path::Path::new("package/package.json") {
continue;
}

let mut package_json_content = String::new();
entry.read_to_string(&mut package_json_content).map_err(|error| {
Error::ConfigError(
format!("Failed to read package.json from package tarball {spec}: {error}").into(),
)
})?;
return serde_json::from_str(&package_json_content).map_err(Error::JsonError);
}

(spec.to_string(), None)
Err(Error::ConfigError(
format!("Package tarball {spec} must contain package/package.json").into(),
))
}

fn parse_npm_view_version(stdout: &[u8]) -> Result<String, Error> {
Expand Down
5 changes: 4 additions & 1 deletion crates/vite_global_cli/src/commands/global/outdated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ pub async fn get_outdated_packages(
let installed = if !packages.is_empty() {
let mut installed = Vec::new();
for package in packages {
let (package_name, _) = parse_package_spec(package);
let Ok((package_name, _)) = parse_package_spec(package) else {
// Silently skip, follow npm's behavior
continue;
};
if let Some(metadata) = PackageMetadata::load(&package_name).await? {
installed.push((metadata, Some(package.clone())));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
> vp install -g ./conflict-pkg # Install package with conflicting binary name (uses cwd version)
info: Installing 1 global package with Node.js <semver>
warn: Package './conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed ./conflict-pkg <semver>
warn: Package 'conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed conflict-pkg <semver>
Bins: conflict-cli, node

> vp remove -g conflict-pkg # Cleanup
Uninstalled conflict-pkg

> vp install -g --node 20 ./conflict-pkg # Install with specific Node.js version
info: Installing 1 global package with Node.js <semver>
warn: Package './conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed ./conflict-pkg <semver>
warn: Package 'conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
✓ Installed conflict-pkg <semver>
Bins: conflict-cli, node

> vp remove -g conflict-pkg # Cleanup
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('The package is installed successfully');
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "just-a-normal-package",
"version": "0.0.0",
"bin": {
"just-a-normal-package": "./bin.js"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
> vp install -g .
info: Installing 1 global package with Node.js <semver>
✓ Installed just-a-normal-package <semver>
Bins: just-a-normal-package

> vp install -g ./another-package.tgz
info: Installing 1 global package with Node.js <semver>
✓ Installed another-normal-package <semver>
Bins: another-normal-package

> vp list -g just-a-normal-package
Package Node version Binaries
--- --- ---
just-a-normal-package@<semver> <semver> just-a-normal-package

> vp list -g another-normal-package
Package Node version Binaries
--- --- ---
another-normal-package@<semver> <semver> another-normal-package
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"commands": [
"vp install -g .",
"vp install -g ./another-package.tgz",
"vp list -g just-a-normal-package",
"vp list -g another-normal-package"
],
"after": ["vp remove -g just-a-normal-package another-normal-package"]
}
Loading
Loading