Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(add): better error message when adding package that only has pre-release versions #26724

Merged
merged 3 commits into from
Nov 5, 2024
Merged
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
205 changes: 146 additions & 59 deletions cli/tools/registry/pm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use deno_core::futures::StreamExt;
use deno_path_util::url_to_file_path;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageNv;
use deno_semver::package::PackageReq;
use deno_semver::Version;
use deno_semver::VersionReq;
use jsonc_parser::cst::CstObject;
use jsonc_parser::cst::CstObjectProp;
Expand Down Expand Up @@ -455,15 +457,32 @@ pub async fn add(
match package_and_version {
PackageAndVersion::NotFound {
package: package_name,
found_npm_package,
help,
package_req,
} => {
if found_npm_package {
bail!("{} was not found, but a matching npm package exists. Did you mean `{}`?", crate::colors::red(package_name), crate::colors::yellow(format!("deno {cmd_name} npm:{package_req}")));
} else {
bail!("{} was not found.", crate::colors::red(package_name));
} => match help {
Some(NotFoundHelp::NpmPackage) => {
bail!(
"{} was not found, but a matching npm package exists. Did you mean `{}`?",
crate::colors::red(package_name),
crate::colors::yellow(format!("deno {cmd_name} npm:{package_req}"))
);
}
}
Some(NotFoundHelp::JsrPackage) => {
bail!(
"{} was not found, but a matching jsr package exists. Did you mean `{}`?",
crate::colors::red(package_name),
crate::colors::yellow(format!("deno {cmd_name} jsr:{package_req}"))
)
}
Some(NotFoundHelp::PreReleaseVersion(version)) => {
bail!(
"{} has only pre-release versions available. Try specifying a version: `{}`",
crate::colors::red(&package_name),
crate::colors::yellow(format!("deno {cmd_name} {package_name}@^{version}"))
)
}
None => bail!("{} was not found.", crate::colors::red(package_name)),
},
PackageAndVersion::Selected(selected) => {
selected_packages.push(selected);
}
Expand Down Expand Up @@ -511,76 +530,144 @@ struct SelectedPackage {
selected_version: String,
}

enum NotFoundHelp {
NpmPackage,
JsrPackage,
PreReleaseVersion(Version),
}

enum PackageAndVersion {
NotFound {
package: String,
found_npm_package: bool,
package_req: PackageReq,
help: Option<NotFoundHelp>,
},
Selected(SelectedPackage),
}

fn best_version<'a>(
versions: impl Iterator<Item = &'a Version>,
) -> Option<&'a Version> {
let mut maybe_best_version: Option<&Version> = None;
for version in versions {
let is_best_version = maybe_best_version
.as_ref()
.map(|best_version| (*best_version).cmp(version).is_lt())
.unwrap_or(true);
if is_best_version {
maybe_best_version = Some(version);
}
}
maybe_best_version
}

trait PackageInfoProvider {
const SPECIFIER_PREFIX: &str;
/// The help to return if a package is found by this provider
const HELP: NotFoundHelp;
async fn req_to_nv(&self, req: &PackageReq) -> Option<PackageNv>;
async fn latest_version<'a>(&self, req: &PackageReq) -> Option<Version>;
}

impl PackageInfoProvider for Arc<JsrFetchResolver> {
const HELP: NotFoundHelp = NotFoundHelp::JsrPackage;
const SPECIFIER_PREFIX: &str = "jsr";
async fn req_to_nv(&self, req: &PackageReq) -> Option<PackageNv> {
(**self).req_to_nv(req).await
}

async fn latest_version<'a>(&self, req: &PackageReq) -> Option<Version> {
let info = self.package_info(&req.name).await?;
best_version(
info
.versions
.iter()
.filter(|(_, version_info)| !version_info.yanked)
.map(|(version, _)| version),
)
.cloned()
}
}

impl PackageInfoProvider for Arc<NpmFetchResolver> {
const HELP: NotFoundHelp = NotFoundHelp::NpmPackage;
const SPECIFIER_PREFIX: &str = "npm";
async fn req_to_nv(&self, req: &PackageReq) -> Option<PackageNv> {
(**self).req_to_nv(req).await
}

async fn latest_version<'a>(&self, req: &PackageReq) -> Option<Version> {
let info = self.package_info(&req.name).await?;
best_version(info.versions.keys()).cloned()
}
}

async fn find_package_and_select_version_for_req(
jsr_resolver: Arc<JsrFetchResolver>,
npm_resolver: Arc<NpmFetchResolver>,
add_package_req: AddRmPackageReq,
) -> Result<PackageAndVersion, AnyError> {
match add_package_req.value {
AddRmPackageReqValue::Jsr(req) => {
let jsr_prefixed_name = format!("jsr:{}", &req.name);
let Some(nv) = jsr_resolver.req_to_nv(&req).await else {
if npm_resolver.req_to_nv(&req).await.is_some() {
async fn select<T: PackageInfoProvider, S: PackageInfoProvider>(
main_resolver: T,
fallback_resolver: S,
add_package_req: AddRmPackageReq,
) -> Result<PackageAndVersion, AnyError> {
let req = match &add_package_req.value {
AddRmPackageReqValue::Jsr(req) => req,
AddRmPackageReqValue::Npm(req) => req,
};
let prefixed_name = format!("{}:{}", T::SPECIFIER_PREFIX, req.name);
let help_if_found_in_fallback = S::HELP;
let Some(nv) = main_resolver.req_to_nv(req).await else {
if fallback_resolver.req_to_nv(req).await.is_some() {
// it's in the other registry
return Ok(PackageAndVersion::NotFound {
package: prefixed_name,
help: Some(help_if_found_in_fallback),
package_req: req.clone(),
});
}
if req.version_req.version_text() == "*" {
if let Some(pre_release_version) =
main_resolver.latest_version(req).await
{
return Ok(PackageAndVersion::NotFound {
package: jsr_prefixed_name,
found_npm_package: true,
package_req: req,
package: prefixed_name,
package_req: req.clone(),
help: Some(NotFoundHelp::PreReleaseVersion(
pre_release_version.clone(),
)),
});
}
}

return Ok(PackageAndVersion::NotFound {
package: jsr_prefixed_name,
found_npm_package: false,
package_req: req,
});
};
let range_symbol = if req.version_req.version_text().starts_with('~') {
"~"
} else if req.version_req.version_text() == nv.version.to_string() {
""
} else {
"^"
};
Ok(PackageAndVersion::Selected(SelectedPackage {
import_name: add_package_req.alias,
package_name: jsr_prefixed_name,
version_req: format!("{}{}", range_symbol, &nv.version),
selected_version: nv.version.to_string(),
}))
}
AddRmPackageReqValue::Npm(req) => {
let npm_prefixed_name = format!("npm:{}", &req.name);
let Some(nv) = npm_resolver.req_to_nv(&req).await else {
return Ok(PackageAndVersion::NotFound {
package: npm_prefixed_name,
found_npm_package: false,
package_req: req,
});
};
return Ok(PackageAndVersion::NotFound {
package: prefixed_name,
help: None,
package_req: req.clone(),
});
};
let range_symbol = if req.version_req.version_text().starts_with('~') {
"~"
} else if req.version_req.version_text() == nv.version.to_string() {
""
} else {
"^"
};
Ok(PackageAndVersion::Selected(SelectedPackage {
import_name: add_package_req.alias,
package_name: prefixed_name,
version_req: format!("{}{}", range_symbol, &nv.version),
selected_version: nv.version.to_string(),
}))
}

let range_symbol = if req.version_req.version_text().starts_with('~') {
"~"
} else if req.version_req.version_text() == nv.version.to_string() {
""
} else {
"^"
};

Ok(PackageAndVersion::Selected(SelectedPackage {
import_name: add_package_req.alias,
package_name: npm_prefixed_name,
version_req: format!("{}{}", range_symbol, &nv.version),
selected_version: nv.version.to_string(),
}))
match &add_package_req.value {
AddRmPackageReqValue::Jsr(_) => {
select(jsr_resolver, npm_resolver, add_package_req).await
}
AddRmPackageReqValue::Npm(_) => {
select(npm_resolver, jsr_resolver, add_package_req).await
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions tests/registry/jsr/@denotest/unstable/1.0.0-beta.1/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function doThing() {
return "thing";
}
5 changes: 5 additions & 0 deletions tests/registry/jsr/@denotest/unstable/1.0.0-beta.1_meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"exports": {
".": "./mod.ts"
}
}
3 changes: 3 additions & 0 deletions tests/registry/jsr/@denotest/unstable/1.0.0-beta.2/mod.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function doThing() {
return "thing2";
}
5 changes: 5 additions & 0 deletions tests/registry/jsr/@denotest/unstable/1.0.0-beta.2_meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"exports": {
".": "./mod.ts"
}
}
6 changes: 6 additions & 0 deletions tests/registry/jsr/@denotest/unstable/meta.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"versions": {
"1.0.0-beta.1": {},
"1.0.0-beta.2": {}
}
}
23 changes: 18 additions & 5 deletions tests/specs/add/only_unstable_versions/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
{
"tempDir": true,
"steps": [
{
"args": "add npm:@denotest/unstable",
"output": "add.out"
"tests": {
"npm_package": {
"steps": [
{
"args": "add npm:@denotest/unstable",
"output": "add.out"
}
]
},
"jsr_package": {
"steps": [
{
"args": "add jsr:@denotest/unstable",
"output": "add_jsr.out",
"exitCode": 1
}
]
}
]
}
}
1 change: 1 addition & 0 deletions tests/specs/add/only_unstable_versions/add_jsr.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error: jsr:@denotest/unstable has only pre-release versions available. Try specifying a version: `deno add jsr:@denotest/unstable@^1.0.0-beta.2`