-
Notifications
You must be signed in to change notification settings - Fork 34
feat(package-manager): support version ranges and multiple packages in pacquet add
#328
base: main
Are you sure you want to change the base?
Changes from all commits
01f3e26
9faa8a0
1932a8a
bd65f2b
3d6dbdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,13 @@ | ||
| use crate::{Install, InstallError, ResolvedPackages}; | ||
| use derive_more::{Display, Error}; | ||
| use miette::Diagnostic; | ||
| use node_semver::{Range, Version}; | ||
| use pacquet_lockfile::Lockfile; | ||
| use pacquet_network::ThrottledClient; | ||
| use pacquet_npmrc::Npmrc; | ||
| use pacquet_package_manifest::PackageManifestError; | ||
| use pacquet_package_manifest::{DependencyGroup, PackageManifest}; | ||
| use pacquet_registry::{PackageTag, PackageVersion}; | ||
| use pacquet_registry::{PackageTag, PackageVersion, RegistryError}; | ||
| use pacquet_tarball::MemCache; | ||
|
|
||
| /// This subroutine does everything `pacquet add` is supposed to do. | ||
|
|
@@ -23,13 +24,15 @@ where | |
| pub manifest: &'a mut PackageManifest, | ||
| pub lockfile: Option<&'a Lockfile>, | ||
| pub list_dependency_groups: ListDependencyGroups, // must be a function because it is called multiple times | ||
| pub package_name: &'a str, // TODO: 1. support version range, 2. multiple arguments, 3. name this `packages` | ||
| pub save_exact: bool, // TODO: add `save-exact` to `.npmrc`, merge configs, and remove this | ||
| pub packages: &'a [&'a str], | ||
| pub save_exact: bool, // TODO: add `save-exact` to `.npmrc`, merge configs, and remove this | ||
| } | ||
|
|
||
| /// Error type of [`Add`]. | ||
| #[derive(Debug, Display, Error, Diagnostic)] | ||
| pub enum AddError { | ||
| #[display("Failed to fetch version for package: {_0}")] | ||
| FetchVersion(#[error(source)] RegistryError), | ||
| #[display("Failed to add package to manifest: {_0}")] | ||
| AddDependencyToManifest(#[error(source)] PackageManifestError), | ||
| #[display("Failed save the manifest file: {_0}")] | ||
|
|
@@ -38,6 +41,20 @@ pub enum AddError { | |
| Install(#[error(source)] InstallError), | ||
| } | ||
|
|
||
| /// Split a package argument into name and version specifier. | ||
| /// | ||
| /// Handles scoped packages: `@scope/name@1.0.0` splits into `("@scope/name", "1.0.0")`. | ||
| fn parse_pkg_arg(arg: &str) -> (&str, &str) { | ||
| let start = usize::from(arg.starts_with('@')); | ||
| match arg[start..].find('@') { | ||
| Some(pos) => { | ||
| let split = start + pos; | ||
| (&arg[..split], &arg[split + 1..]) | ||
| } | ||
| None => (arg, ""), | ||
| } | ||
| } | ||
|
|
||
| impl<'a, ListDependencyGroups, DependencyGroupList> | ||
| Add<'a, ListDependencyGroups, DependencyGroupList> | ||
| where | ||
|
|
@@ -52,25 +69,60 @@ where | |
| manifest, | ||
| lockfile, | ||
| list_dependency_groups, | ||
| package_name, | ||
| packages, | ||
| save_exact, | ||
| resolved_packages, | ||
| } = self; | ||
|
|
||
| let latest_version = PackageVersion::fetch_from_registry( | ||
| package_name, | ||
| PackageTag::Latest, // TODO: add support for specifying tags | ||
| http_client, | ||
| &config.registry, | ||
| ) | ||
| .await | ||
| .expect("resolve latest tag"); // TODO: properly propagate this error | ||
| for &pkg in packages { | ||
| let (name, specifier) = parse_pkg_arg(pkg); | ||
|
|
||
| let version_range = latest_version.serialize(save_exact); | ||
| for dependency_group in list_dependency_groups() { | ||
| manifest | ||
| .add_dependency(package_name, &version_range, dependency_group) | ||
| .map_err(AddError::AddDependencyToManifest)?; | ||
| // Resolve the version specifier to a range string to save in package.json. | ||
| // For tags (no specifier or dist-tag), we fetch the resolved version first so | ||
| // we can save a pinned semver range rather than a mutable tag name. | ||
| let version_to_save = if specifier.is_empty() || specifier == "latest" { | ||
| let version = PackageVersion::fetch_from_registry( | ||
| name, | ||
| PackageTag::Latest, | ||
| http_client, | ||
| &config.registry, | ||
| ) | ||
| .await | ||
| .map_err(AddError::FetchVersion)?; | ||
| version.serialize(save_exact) | ||
| } else if let Ok(v) = specifier.parse::<Version>() { | ||
| // Exact semver version: fetch to validate, then save with ^ unless --save-exact. | ||
| PackageVersion::fetch_from_registry( | ||
| name, | ||
| PackageTag::Version(v), | ||
| http_client, | ||
| &config.registry, | ||
| ) | ||
| .await | ||
| .map_err(AddError::FetchVersion)?; | ||
| if save_exact { specifier.to_owned() } else { format!("^{specifier}") } | ||
| } else if specifier.parse::<Range>().is_ok() { | ||
| // Semver range (e.g. `^18`, `~1.0.0`, `>=1 <2`): save as-is and let | ||
| // the install step resolve the best matching version. | ||
| specifier.to_owned() | ||
| } else { | ||
| // Named dist-tag (e.g. `next`, `beta`): resolve to a concrete version. | ||
| let version = PackageVersion::fetch_from_registry( | ||
| name, | ||
| PackageTag::Tag(specifier.to_owned()), | ||
| http_client, | ||
| &config.registry, | ||
| ) | ||
|
Comment on lines
+110
to
+115
|
||
| .await | ||
| .map_err(AddError::FetchVersion)?; | ||
| version.serialize(save_exact) | ||
| }; | ||
|
|
||
| for dependency_group in list_dependency_groups() { | ||
| manifest | ||
| .add_dependency(name, &version_to_save, dependency_group) | ||
| .map_err(AddError::AddDependencyToManifest)?; | ||
| } | ||
| } | ||
|
|
||
| Install { | ||
|
|
@@ -92,3 +144,43 @@ where | |
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_no_specifier() { | ||
| assert_eq!(parse_pkg_arg("react"), ("react", "")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_with_version() { | ||
| assert_eq!(parse_pkg_arg("react@18.2.0"), ("react", "18.2.0")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_with_range() { | ||
| assert_eq!(parse_pkg_arg("react@^18"), ("react", "^18")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_with_tag() { | ||
| assert_eq!(parse_pkg_arg("react@next"), ("react", "next")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_scoped_no_specifier() { | ||
| assert_eq!(parse_pkg_arg("@scope/pkg"), ("@scope/pkg", "")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_scoped_with_version() { | ||
| assert_eq!(parse_pkg_arg("@scope/pkg@1.0.0"), ("@scope/pkg", "1.0.0")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_pkg_arg_scoped_with_range() { | ||
| assert_eq!(parse_pkg_arg("@scope/pkg@^1.0.0"), ("@scope/pkg", "^1.0.0")); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FetchVersion's display text says "for package" but it only formats the underlyingRegistryError, so the message doesn't include which package/specifier failed and can be misleading. Consider storing the package name (and maybe the requested specifier/tag) on this error variant and include it in the display string (while keepingRegistryErroras the source).