From 218ee1b1ffebbb53aa82c8ce55e7ee7061342249 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:29:11 +0200 Subject: [PATCH] fix(add): Better error message when missing npm specifier (#24970) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: Screenshot 2024-08-09 at 3 15 01 PM After: Screenshot 2024-08-09 at 3 52 15 PM --- cli/main.rs | 2 +- cli/tools/installer.rs | 7 ++- cli/tools/registry/mod.rs | 1 + cli/tools/registry/pm.rs | 55 +++++++++++++++++-- .../add/missing_npm_specifier/__test__.jsonc | 28 ++++++++++ .../specs/add/missing_npm_specifier/deno.json | 0 6 files changed, 86 insertions(+), 7 deletions(-) create mode 100644 tests/specs/add/missing_npm_specifier/__test__.jsonc create mode 100644 tests/specs/add/missing_npm_specifier/deno.json diff --git a/cli/main.rs b/cli/main.rs index 534fa92d42..8ebf65e16c 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -98,7 +98,7 @@ fn spawn_subcommand + 'static, T: SubcommandOutput>( async fn run_subcommand(flags: Arc) -> Result { let handle = match flags.subcommand.clone() { DenoSubcommand::Add(add_flags) => spawn_subcommand(async { - tools::registry::add(flags, add_flags).await + tools::registry::add(flags, add_flags, tools::registry::AddCommandName::Add).await }), DenoSubcommand::Bench(bench_flags) => spawn_subcommand(async { if bench_flags.watch.is_some() { diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index d1beb79bba..648b766b85 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -266,7 +266,12 @@ async fn install_local( maybe_add_flags: Option, ) -> Result<(), AnyError> { if let Some(add_flags) = maybe_add_flags { - return super::registry::add(flags, add_flags).await; + return super::registry::add( + flags, + add_flags, + super::registry::AddCommandName::Install, + ) + .await; } let factory = CliFactory::from_flags(flags); diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 51aeb12213..b3bed77217 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -64,6 +64,7 @@ mod unfurl; use auth::get_auth_method; use auth::AuthMethod; pub use pm::add; +pub use pm::AddCommandName; use publish_order::PublishOrderGraph; use unfurl::SpecifierUnfurler; diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index fb8847625d..86596df9dc 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -175,9 +175,26 @@ fn package_json_dependency_entry( } } +#[derive(Clone, Copy)] +/// The name of the subcommand invoking the `add` operation. +pub enum AddCommandName { + Add, + Install, +} + +impl std::fmt::Display for AddCommandName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AddCommandName::Add => write!(f, "add"), + AddCommandName::Install => write!(f, "install"), + } + } +} + pub async fn add( flags: Arc, add_flags: AddFlags, + cmd_name: AddCommandName, ) -> Result<(), AnyError> { let (config_file, cli_factory) = DenoOrPackageJson::from_flags(flags.clone())?; @@ -234,8 +251,16 @@ pub async fn add( let package_and_version = package_and_version_result?; match package_and_version { - PackageAndVersion::NotFound(package_name) => { - bail!("{} was not found.", crate::colors::red(package_name)); + PackageAndVersion::NotFound { + package: package_name, + found_npm_package, + 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)); + } } PackageAndVersion::Selected(selected) => { selected_packages.push(selected); @@ -327,7 +352,11 @@ struct SelectedPackage { } enum PackageAndVersion { - NotFound(String), + NotFound { + package: String, + found_npm_package: bool, + package_req: PackageReq, + }, Selected(SelectedPackage), } @@ -340,7 +369,19 @@ async fn find_package_and_select_version_for_req( AddPackageReqValue::Jsr(req) => { let jsr_prefixed_name = format!("jsr:{}", &req.name); let Some(nv) = jsr_resolver.req_to_nv(&req).await else { - return Ok(PackageAndVersion::NotFound(jsr_prefixed_name)); + if npm_resolver.req_to_nv(&req).await.is_some() { + return Ok(PackageAndVersion::NotFound { + package: jsr_prefixed_name, + found_npm_package: true, + package_req: req, + }); + } + + 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('~') { '~' @@ -357,7 +398,11 @@ async fn find_package_and_select_version_for_req( AddPackageReqValue::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(npm_prefixed_name)); + return Ok(PackageAndVersion::NotFound { + package: npm_prefixed_name, + found_npm_package: false, + package_req: req, + }); }; let range_symbol = if req.version_req.version_text().starts_with('~') { '~' diff --git a/tests/specs/add/missing_npm_specifier/__test__.jsonc b/tests/specs/add/missing_npm_specifier/__test__.jsonc new file mode 100644 index 0000000000..34cc61a373 --- /dev/null +++ b/tests/specs/add/missing_npm_specifier/__test__.jsonc @@ -0,0 +1,28 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "add ajv@latest", + "output": "error: jsr:ajv was not found, but a matching npm package exists. Did you mean `deno add npm:ajv@latest`?\n", + "exitCode": 1 + }, + { + "args": "add ajv", + "output": "error: jsr:ajv was not found, but a matching npm package exists. Did you mean `deno add npm:ajv`?\n", + "exitCode": 1 + }, + { + "args": "add ajv@8.11.0", + "output": "error: jsr:ajv was not found, but a matching npm package exists. Did you mean `deno add npm:ajv@8.11.0`?\n", + "exitCode": 1 + }, + { + "envs": { + "DENO_FUTURE": "1" + }, + "args": "install ajv@latest", + "output": "error: jsr:ajv was not found, but a matching npm package exists. Did you mean `deno install npm:ajv@latest`?\n", + "exitCode": 1 + } + ] +} diff --git a/tests/specs/add/missing_npm_specifier/deno.json b/tests/specs/add/missing_npm_specifier/deno.json new file mode 100644 index 0000000000..e69de29bb2