Skip to content

Commit bd8cc52

Browse files
liangmiQwQfengmk2
andauthored
fix: use real package name for global local packages installation (#1685)
Related to #664. The current Vite+ will just treat the path inputed as the package name, and because Vite+ uses package name to create directories and manage installation, it will make Vite+ broken as it can be a bad path. This PR fixes it by resolving the true package name by reading the `package.json` under the entered path. --------- Co-authored-by: MK (fengmk2) <fengmk2@gmail.com>
1 parent de79b17 commit bd8cc52

19 files changed

Lines changed: 234 additions & 59 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vite_global_cli/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ clap = { workspace = true, features = ["derive"] }
1717
clap_complete = { workspace = true, features = ["unstable-dynamic"] }
1818
directories = { workspace = true }
1919
futures = { workspace = true }
20+
flate2 = { workspace = true }
2021
serde = { workspace = true }
2122
serde_json = { workspace = true }
2223
node-semver = { workspace = true }
2324
thiserror = { workspace = true }
25+
tar = { workspace = true }
2426
tokio = { workspace = true, features = ["full"] }
2527
tracing = { workspace = true }
2628
owo-colors = { workspace = true }

crates/vite_global_cli/src/cli.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,8 @@ async fn managed_update(
671671
continue;
672672
}
673673

674-
let (package_name, _) = global::parse_package_spec(package);
674+
// It is not a local package, so `parse_package_spec` there won't return `Err()`
675+
let (package_name, _) = global::parse_package_spec(package).unwrap();
675676
if PackageMetadata::load(&package_name).await?.is_some() {
676677
managed_specs.push(package.clone());
677678
} else {

crates/vite_global_cli/src/commands/global/install.rs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::{
2626
},
2727
package_metadata::PackageMetadata,
2828
},
29-
global::{CORE_SHIMS, parse_package_spec},
29+
global::{CORE_SHIMS, is_local_package_spec, parse_package_spec},
3030
},
3131
error::Error,
3232
};
@@ -105,7 +105,11 @@ pub async fn install(
105105
let mut packages = IndexMap::<String, Package>::new();
106106
for package_spec in package_specs {
107107
// Parse package spec (e.g., "typescript", "typescript@5.0.0", "@scope/pkg")
108-
let (package_name, _version_spec) = parse_package_spec(package_spec);
108+
109+
let (package_name, _version_spec) = match parse_package_spec(package_spec) {
110+
Ok(result) => result,
111+
Err(error) => return Err((Some(package_spec.clone()), error)),
112+
};
109113
packages.insert(package_name, Package { spec: package_spec, staging_dir: None });
110114
}
111115
let packages_count = packages.len();
@@ -395,7 +399,18 @@ async fn install_one(
395399
/// 1. Try to use PackageMetadata for binary list
396400
/// 2. Fallback to scanning BinConfig files for orphaned binaries
397401
pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
398-
let (package_name, _) = parse_package_spec(package_name);
402+
if is_local_package_spec(package_name) {
403+
// We can't resolve local packages for uninstall, follow npm's behavior
404+
return Err(Error::ConfigError(
405+
format!(
406+
"Local path {} can't be resolved, please enter a package name instead",
407+
package_name
408+
)
409+
.into(),
410+
));
411+
}
412+
413+
let (package_name, _) = parse_package_spec(package_name).unwrap();
399414

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

882897
#[test]
883898
fn test_parse_package_spec_simple() {
884-
let (name, version) = parse_package_spec("typescript");
899+
let (name, version) = parse_package_spec("typescript").unwrap();
885900
assert_eq!(name, "typescript");
886901
assert_eq!(version, None);
887902
}
888903

889904
#[test]
890905
fn test_parse_package_spec_with_version() {
891-
let (name, version) = parse_package_spec("typescript@5.0.0");
906+
let (name, version) = parse_package_spec("typescript@5.0.0").unwrap();
892907
assert_eq!(name, "typescript");
893908
assert_eq!(version, Some("5.0.0".to_string()));
894909
}
895910

896911
#[test]
897912
fn test_parse_package_spec_scoped() {
898-
let (name, version) = parse_package_spec("@types/node");
913+
let (name, version) = parse_package_spec("@types/node").unwrap();
899914
assert_eq!(name, "@types/node");
900915
assert_eq!(version, None);
901916
}
902917

903918
#[test]
904919
fn test_parse_package_spec_scoped_with_version() {
905-
let (name, version) = parse_package_spec("@types/node@20.0.0");
920+
let (name, version) = parse_package_spec("@types/node@20.0.0").unwrap();
906921
assert_eq!(name, "@types/node");
907922
assert_eq!(version, Some("20.0.0".to_string()));
908923
}

crates/vite_global_cli/src/commands/global/mod.rs

Lines changed: 142 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
//! Managed global package utilities.
22
3-
use std::{collections::HashMap, io::IsTerminal, process::Stdio, time::Duration};
4-
3+
use std::{
4+
collections::HashMap,
5+
fs::File,
6+
io::{IsTerminal, Read},
7+
process::Stdio,
8+
time::Duration,
9+
};
10+
11+
use flate2::read::GzDecoder;
512
use futures::{StreamExt, stream::FuturesUnordered};
613
use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle};
14+
use tar::Archive;
715
use tokio::process::Command;
816
use vite_path::{AbsolutePathBuf, current_dir};
917
use vite_shared::format_path_prepended;
@@ -48,23 +56,34 @@ impl NpmRegistry {
4856
}
4957

5058
async fn latest_package_version(&self, package_spec: &str) -> Result<String, Error> {
51-
let output = Command::new(self.npm_path.as_path())
52-
.args(["view", package_spec, "version", "--json"])
53-
.env("PATH", format_path_prepended(self.node_bin_dir.as_path()))
54-
.stdout(Stdio::piped())
55-
.stderr(Stdio::piped())
56-
.output()
57-
.await?;
58-
59-
if !output.status.success() {
60-
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
61-
return Err(Error::ConfigError(
62-
format!("npm view failed for {package_spec}: {stderr}").into(),
63-
));
64-
}
59+
let output = npm_view(&self.npm_path, &self.node_bin_dir, package_spec, "version").await?;
60+
61+
parse_npm_view_version(&output)
62+
}
63+
}
64+
65+
async fn npm_view(
66+
npm_path: &AbsolutePathBuf,
67+
node_bin_dir: &AbsolutePathBuf,
68+
package_spec: &str,
69+
field: &str,
70+
) -> Result<Vec<u8>, Error> {
71+
let output = Command::new(npm_path.as_path())
72+
.args(["view", package_spec, field, "--json"])
73+
.env("PATH", format_path_prepended(node_bin_dir.as_path()))
74+
.stdout(Stdio::piped())
75+
.stderr(Stdio::piped())
76+
.output()
77+
.await?;
6578

66-
parse_npm_view_version(&output.stdout)
79+
if !output.status.success() {
80+
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
81+
return Err(Error::ConfigError(
82+
format!("npm view failed for {package_spec}: {stderr}").into(),
83+
));
6784
}
85+
86+
Ok(output.stdout)
6887
}
6988

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

135154
/// Parse package spec into name and optional version.
136-
pub(crate) fn parse_package_spec(spec: &str) -> (String, Option<String>) {
137-
if spec.starts_with('@') {
138-
if let Some(idx) = spec[1..].find('@') {
139-
let idx = idx + 1;
140-
return (spec[..idx].to_string(), Some(spec[idx + 1..].to_string()));
155+
/// For local packages, read package.json from a directory or package tarball.
156+
///
157+
/// It will never return an `Err()` if it is not a local package
158+
pub(crate) fn parse_package_spec(spec: &str) -> Result<(String, Option<String>), Error> {
159+
if is_local_package_spec(spec) {
160+
let package_json = read_local_package_json(spec)?;
161+
let Some(package_name) = package_json.get("name").and_then(|name| name.as_str()) else {
162+
return Err(Error::ConfigError(
163+
format!("Local package {spec} must have a string name in package.json").into(),
164+
));
165+
};
166+
167+
Ok((package_name.to_string(), None))
168+
} else {
169+
if spec.starts_with('@') {
170+
if let Some(idx) = spec[1..].find('@') {
171+
let idx = idx + 1;
172+
return Ok((spec[..idx].to_string(), Some(spec[idx + 1..].to_string())));
173+
}
174+
return Ok((spec.to_string(), None));
141175
}
142-
return (spec.to_string(), None);
176+
177+
if let Some(idx) = spec.find('@') {
178+
return Ok((spec[..idx].to_string(), Some(spec[idx + 1..].to_string())));
179+
}
180+
181+
Ok((spec.to_string(), None))
143182
}
183+
}
144184

145-
if let Some(idx) = spec.find('@') {
146-
return (spec[..idx].to_string(), Some(spec[idx + 1..].to_string()));
185+
fn resolve_local_package_path(spec: &str) -> Result<AbsolutePathBuf, Error> {
186+
let path_spec = spec.strip_prefix("file:").unwrap_or(spec);
187+
let path = std::path::Path::new(path_spec);
188+
if path.is_absolute() {
189+
AbsolutePathBuf::new(path.to_path_buf())
190+
.ok_or_else(|| Error::ConfigError(format!("Invalid local package path {spec}").into()))
191+
} else {
192+
Ok(current_dir()
193+
.map_err(|error| {
194+
Error::ConfigError(format!("Cannot get current directory: {error}").into())
195+
})?
196+
.join(path))
197+
}
198+
}
199+
200+
fn read_local_package_json(spec: &str) -> Result<serde_json::Value, Error> {
201+
let package_path = resolve_local_package_path(spec)?;
202+
if package_path.as_path().is_file() && is_package_tarball(package_path.as_path()) {
203+
return read_package_json_from_tarball(spec, &package_path);
204+
}
205+
206+
let package_json_path = package_path.join("package.json");
207+
let package_json_content =
208+
std::fs::read_to_string(package_json_path.as_path()).map_err(|error| {
209+
Error::ConfigError(
210+
format!(
211+
"Failed to read package.json for local package {spec} at {}: {error}",
212+
package_json_path.as_path().display()
213+
)
214+
.into(),
215+
)
216+
})?;
217+
serde_json::from_str(&package_json_content).map_err(Error::JsonError)
218+
}
219+
220+
fn is_package_tarball(path: &std::path::Path) -> bool {
221+
let path = path.to_string_lossy();
222+
path.ends_with(".tgz") || path.ends_with(".tar.gz")
223+
}
224+
225+
fn read_package_json_from_tarball(
226+
spec: &str,
227+
package_path: &AbsolutePathBuf,
228+
) -> Result<serde_json::Value, Error> {
229+
let file = File::open(package_path.as_path()).map_err(|error| {
230+
Error::ConfigError(
231+
format!(
232+
"Failed to read package tarball {spec} at {}: {error}",
233+
package_path.as_path().display()
234+
)
235+
.into(),
236+
)
237+
})?;
238+
let decoder = GzDecoder::new(file);
239+
let mut archive = Archive::new(decoder);
240+
241+
for entry in archive.entries().map_err(|error| {
242+
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
243+
})? {
244+
let mut entry = entry.map_err(|error| {
245+
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
246+
})?;
247+
let path = entry.path().map_err(|error| {
248+
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
249+
})?;
250+
if path.as_ref() != std::path::Path::new("package/package.json") {
251+
continue;
252+
}
253+
254+
let mut package_json_content = String::new();
255+
entry.read_to_string(&mut package_json_content).map_err(|error| {
256+
Error::ConfigError(
257+
format!("Failed to read package.json from package tarball {spec}: {error}").into(),
258+
)
259+
})?;
260+
return serde_json::from_str(&package_json_content).map_err(Error::JsonError);
147261
}
148262

149-
(spec.to_string(), None)
263+
Err(Error::ConfigError(
264+
format!("Package tarball {spec} must contain package/package.json").into(),
265+
))
150266
}
151267

152268
fn parse_npm_view_version(stdout: &[u8]) -> Result<String, Error> {

crates/vite_global_cli/src/commands/global/outdated.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ pub async fn get_outdated_packages(
5050
let installed = if !packages.is_empty() {
5151
let mut installed = Vec::new();
5252
for package in packages {
53-
let (package_name, _) = parse_package_spec(package);
53+
let Ok((package_name, _)) = parse_package_spec(package) else {
54+
// Silently skip, follow npm's behavior
55+
continue;
56+
};
5457
if let Some(metadata) = PackageMetadata::load(&package_name).await? {
5558
installed.push((metadata, Some(package.clone())));
5659
}

packages/cli/snap-tests-global/command-env-install-conflict/snap.txt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
> vp install -g ./conflict-pkg # Install package with conflicting binary name (uses cwd version)
22
info: Installing 1 global package with Node.js <semver>
3-
warn: Package './conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
4-
✓ Installed ./conflict-pkg <semver>
3+
warn: Package 'conflict-pkg' provides 'node' binary, but it conflicts with a core shim. Skipping.
4+
✓ Installed conflict-pkg <semver>
55
Bins: conflict-cli, node
66

77
> vp remove -g conflict-pkg # Cleanup
88
Uninstalled conflict-pkg
99

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

1616
> vp remove -g conflict-pkg # Cleanup
Binary file not shown.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
console.log('The package is installed successfully');
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "just-a-normal-package",
3+
"version": "0.0.0",
4+
"bin": {
5+
"just-a-normal-package": "./bin.js"
6+
}
7+
}

0 commit comments

Comments
 (0)