Repository: denoland/deno
Pull request: fix(cli): add prefix hint when installing a package globally

closes #26545

Shows a hint when a package is installed globally, otherwise fallbacks to the existing implementation.

Changed files:
filename: cli/tools/installer.rs
patch: @@ -3,6 +3,7 @@
 use crate::args::resolve_no_prompt;
 use crate::args::AddFlags;
 use crate::args::CaData;
+use crate::args::CacheSetting;
 use crate::args::ConfigFlag;
 use crate::args::Flags;
 use crate::args::InstallFlags;
@@ -13,8 +14,11 @@ use crate::args::TypeCheckMode;
 use crate::args::UninstallFlags;
 use crate::args::UninstallKind;
 use crate::factory::CliFactory;
+use crate::file_fetcher::FileFetcher;
 use crate::graph_container::ModuleGraphContainer;
 use crate::http_util::HttpClientProvider;
+use crate::jsr::JsrFetchResolver;
+use crate::npm::NpmFetchResolver;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
 
 use deno_core::anyhow::bail;
@@ -354,12 +358,51 @@ async fn install_global(
 ) -> Result<(), AnyError> {
   // ensure the module is cached
   let factory = CliFactory::from_flags(flags.clone());
+
+  let http_client = factory.http_client_provider();
+  let deps_http_cache = factory.global_http_cache()?;
+  let mut deps_file_fetcher = FileFetcher::new(
+    deps_http_cache.clone(),
+    CacheSetting::ReloadAll,
+    true,
+    http_client.clone(),
+    Default::default(),
+    None,
+  );
+
+  let npmrc = factory.cli_options().unwrap().npmrc();
+
+  deps_file_fetcher.set_download_log_level(log::Level::Trace);
+  let deps_file_fetcher = Arc::new(deps_file_fetcher);
+  let jsr_resolver = Arc::new(JsrFetchResolver::new(deps_file_fetcher.clone()));
+  let npm_resolver = Arc::new(NpmFetchResolver::new(
+    deps_file_fetcher.clone(),
+    npmrc.clone(),
+  ));
+
+  let entry_text = install_flags_global.module_url.as_str();
+  let req = super::registry::AddRmPackageReq::parse(entry_text);
+
+  // found a package requirement but missing the prefix
+  if let Ok(Err(package_req)) = req {
+    if jsr_resolver.req_to_nv(&package_req).await.is_some() {
+      bail!(
+        "{entry_text} is missing a prefix. Did you mean `{}`?",
+        crate::colors::yellow(format!("deno install -g jsr:{package_req}"))
+      );
+    } else if npm_resolver.req_to_nv(&package_req).await.is_some() {
+      bail!(
+        "{entry_text} is missing a prefix. Did you mean `{}`?",
+        crate::colors::yellow(format!("deno install -g npm:{package_req}"))
+      );
+    }
+  }
+
   factory
     .main_module_graph_container()
     .await?
     .load_and_type_check_files(&[install_flags_global.module_url.clone()])
     .await?;
-  let http_client = factory.http_client_provider();
 
   // create the install shim
   create_install_shim(http_client, &flags, install_flags_global).await

filename: cli/tools/registry/mod.rs
patch: @@ -69,6 +69,7 @@ pub use pm::add;
 pub use pm::cache_top_level_deps;
 pub use pm::remove;
 pub use pm::AddCommandName;
+pub use pm::AddRmPackageReq;
 use publish_order::PublishOrderGraph;
 use unfurl::SpecifierUnfurler;
 

filename: cli/tools/registry/pm.rs
patch: @@ -679,7 +679,7 @@ enum AddRmPackageReqValue {
 }
 
 #[derive(Debug, PartialEq, Eq)]
-struct AddRmPackageReq {
+pub struct AddRmPackageReq {
   alias: String,
   value: AddRmPackageReqValue,
 }

filename: tests/integration/install_tests.rs
patch: @@ -329,3 +329,60 @@ fn check_local_by_default2() {
     .skip_output_check()
     .assert_exit_code(0);
 }
+
+#[test]
+fn show_prefix_hint_on_global_install() {
+  let context = TestContextBuilder::new()
+    .add_npm_env_vars()
+    .add_jsr_env_vars()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  let temp_dir_str = temp_dir.path().to_string();
+
+  let env_vars = [
+    ("HOME", temp_dir_str.as_str()),
+    ("USERPROFILE", temp_dir_str.as_str()),
+    ("DENO_INSTALL_ROOT", ""),
+  ];
+
+  for pkg_req in ["npm:@denotest/bin", "jsr:@denotest/add"] {
+    let name = pkg_req.split_once('/').unwrap().1;
+    let pkg = pkg_req.split_once(':').unwrap().1;
+
+    // try with prefix and ensure that the installation succeeds
+    context
+      .new_command()
+      .args_vec(["install", "-g", "--name", name, pkg_req])
+      .envs(env_vars)
+      .run()
+      .skip_output_check()
+      .assert_exit_code(0);
+
+    // try without the prefix and ensure that the installation fails with the appropriate error
+    // message
+    let output = context
+      .new_command()
+      .args_vec(["install", "-g", "--name", name, pkg])
+      .envs(env_vars)
+      .run();
+    output.assert_exit_code(1);
+
+    let output_text = output.combined_output();
+    let expected_text =
+      format!("error: {pkg} is missing a prefix. Did you mean `deno install -g {pkg_req}`?");
+    assert_contains!(output_text, &expected_text);
+  }
+
+  // try a pckage not in npm and jsr to make sure the appropriate error message still appears
+  let output = context
+    .new_command()
+    .args_vec(["install", "-g", "package-that-does-not-exist"])
+    .envs(env_vars)
+    .run();
+  output.assert_exit_code(1);
+
+  let output_text = output.combined_output();
+  assert_contains!(output_text, "error: Module not found");
+}