feat: require jsr prefix for deno install and deno add (#25698)

cli/tools/registry/pm.rs

@@ -437,18 +437,6 @@ pub async fn add(
   }
 
   let http_client = cli_factory.http_client_provider();
-
-  let mut selected_packages = Vec::with_capacity(add_flags.packages.len());
-  let mut package_reqs = Vec::with_capacity(add_flags.packages.len());
-
-  for entry_text in add_flags.packages.iter() {
-    let req = AddPackageReq::parse(entry_text).with_context(|| {
-      format!("Failed to parse package required: {}", entry_text)
-    })?;
-
-    package_reqs.push(req);
-  }
-
   let deps_http_cache = cli_factory.global_http_cache()?;
   let mut deps_file_fetcher = FileFetcher::new(
     deps_http_cache.clone(),
@@ -463,6 +451,37 @@ pub async fn add(
   let jsr_resolver = Arc::new(JsrFetchResolver::new(deps_file_fetcher.clone()));
   let npm_resolver = Arc::new(NpmFetchResolver::new(deps_file_fetcher));
 
+  let mut selected_packages = Vec::with_capacity(add_flags.packages.len());
+  let mut package_reqs = Vec::with_capacity(add_flags.packages.len());
+
+  for entry_text in add_flags.packages.iter() {
+    let req = AddPackageReq::parse(entry_text).with_context(|| {
+      format!("Failed to parse package required: {}", entry_text)
+    })?;
+
+    match req {
+      Ok(add_req) => package_reqs.push(add_req),
+      Err(package_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 {cmd_name} 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 {cmd_name} npm:{package_req}"))
+          )
+        } else {
+          bail!(
+            "{} was not found in either jsr or npm.",
+            crate::colors::red(entry_text)
+          );
+        }
+      }
+    }
+  }
+
   let package_futures = package_reqs
     .into_iter()
     .map({
@@ -636,7 +655,7 @@ struct AddPackageReq {
 }
 
 impl AddPackageReq {
-  pub fn parse(entry_text: &str) -> Result<Self, AnyError> {
+  pub fn parse(entry_text: &str) -> Result<Result<Self, PackageReq>, AnyError> {
     enum Prefix {
       Jsr,
       Npm,
@@ -675,13 +694,13 @@ impl AddPackageReq {
       None => match parse_alias(entry_text) {
         Some((alias, text)) => {
           let (maybe_prefix, entry_text) = parse_prefix(text);
-          (
+          if maybe_prefix.is_none() {
-            maybe_prefix.unwrap_or(Prefix::Jsr),
+            return Ok(Err(PackageReq::from_str(entry_text)?));
-            Some(alias.to_string()),
-            entry_text,
-          )
           }
-        None => (Prefix::Jsr, None, entry_text),
+
+          (maybe_prefix.unwrap(), Some(alias.to_string()), entry_text)
+        }
+        None => return Ok(Err(PackageReq::from_str(entry_text)?)),
       },
     };
 
@@ -690,19 +709,19 @@ impl AddPackageReq {
         let req_ref =
           JsrPackageReqReference::from_str(&format!("jsr:{}", entry_text))?;
         let package_req = req_ref.into_inner().req;
-        Ok(AddPackageReq {
+        Ok(Ok(AddPackageReq {
           alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()),
           value: AddPackageReqValue::Jsr(package_req),
-        })
+        }))
       }
       Prefix::Npm => {
         let req_ref =
           NpmPackageReqReference::from_str(&format!("npm:{}", entry_text))?;
         let package_req = req_ref.into_inner().req;
-        Ok(AddPackageReq {
+        Ok(Ok(AddPackageReq {
           alias: maybe_alias.unwrap_or_else(|| package_req.name.to_string()),
           value: AddPackageReqValue::Npm(package_req),
-        })
+        }))
       }
     }
   }
@@ -847,42 +866,42 @@ fn update_config_file_content<
 
 #[cfg(test)]
 mod test {
-  use deno_semver::VersionReq;
-
   use super::*;
 
   #[test]
   fn test_parse_add_package_req() {
     assert_eq!(
-      AddPackageReq::parse("jsr:foo").unwrap(),
+      AddPackageReq::parse("jsr:foo").unwrap().unwrap(),
       AddPackageReq {
         alias: "foo".to_string(),
         value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("alias@jsr:foo").unwrap(),
+      AddPackageReq::parse("alias@jsr:foo").unwrap().unwrap(),
       AddPackageReq {
         alias: "alias".to_string(),
         value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@alias/pkg@npm:foo").unwrap(),
+      AddPackageReq::parse("@alias/pkg@npm:foo").unwrap().unwrap(),
       AddPackageReq {
         alias: "@alias/pkg".to_string(),
         value: AddPackageReqValue::Npm(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@alias/pkg@jsr:foo").unwrap(),
+      AddPackageReq::parse("@alias/pkg@jsr:foo").unwrap().unwrap(),
       AddPackageReq {
         alias: "@alias/pkg".to_string(),
         value: AddPackageReqValue::Jsr(PackageReq::from_str("foo").unwrap())
       }
     );
     assert_eq!(
-      AddPackageReq::parse("alias@jsr:foo@^1.5.0").unwrap(),
+      AddPackageReq::parse("alias@jsr:foo@^1.5.0")
+        .unwrap()
+        .unwrap(),
       AddPackageReq {
         alias: "alias".to_string(),
         value: AddPackageReqValue::Jsr(
@@ -891,15 +910,11 @@ mod test {
       }
     );
     assert_eq!(
-      AddPackageReq::parse("@scope/pkg@tag").unwrap(),
+      AddPackageReq::parse("@scope/pkg@tag")
-      AddPackageReq {
+        .unwrap()
-        alias: "@scope/pkg".to_string(),
+        .unwrap_err()
-        value: AddPackageReqValue::Jsr(PackageReq {
+        .to_string(),
-          name: "@scope/pkg".to_string(),
+      "@scope/pkg@tag",
-          // this is a tag
-          version_req: VersionReq::parse_from_specifier("tag").unwrap(),
-        }),
-      }
     );
   }
 }

tests/integration/pm_tests.rs

@@ -16,7 +16,7 @@ fn add_basic() {
   let temp_dir = context.temp_dir().path();
   temp_dir.join("deno.json").write_json(&starting_deno_json);
 
-  let output = context.new_command().args("add @denotest/add").run();
+  let output = context.new_command().args("add jsr:@denotest/add").run();
   output.assert_exit_code(0);
   let output = output.combined_output();
   assert_contains!(output, "Add jsr:@denotest/add");
@@ -35,7 +35,7 @@ fn add_basic_no_deno_json() {
   let context = pm_context_builder().build();
   let temp_dir = context.temp_dir().path();
 
-  let output = context.new_command().args("add @denotest/add").run();
+  let output = context.new_command().args("add jsr:@denotest/add").run();
   output.assert_exit_code(0);
   let output = output.combined_output();
   assert_contains!(output, "Add jsr:@denotest/add");
@@ -55,7 +55,7 @@ fn add_basic_with_empty_deno_json() {
   let temp_dir = context.temp_dir();
   temp_dir.write("deno.json", "");
 
-  let output = context.new_command().args("add @denotest/add").run();
+  let output = context.new_command().args("add jsr:@denotest/add").run();
   output.assert_exit_code(0);
   let output = output.combined_output();
   assert_contains!(output, "Add jsr:@denotest/add");
@@ -74,7 +74,7 @@ fn add_version_contraint() {
   let context = pm_context_builder().build();
   let temp_dir = context.temp_dir().path();
 
-  let output = context.new_command().args("add @denotest/add@1").run();
+  let output = context.new_command().args("add jsr:@denotest/add@1").run();
   output.assert_exit_code(0);
   let output = output.combined_output();
   assert_contains!(output, "Add jsr:@denotest/add");
@@ -90,7 +90,7 @@ fn add_tilde() {
   let context = pm_context_builder().build();
   let temp_dir = context.temp_dir().path();
 
-  let output = context.new_command().args("add @denotest/add@~1").run();
+  let output = context.new_command().args("add jsr:@denotest/add@~1").run();
   output.assert_exit_code(0);
   let output = output.combined_output();
   assert_contains!(output, "Add jsr:@denotest/add");
@@ -114,7 +114,7 @@ fn add_multiple() {
 
   let output = context
     .new_command()
-    .args("add @denotest/add @denotest/subset-type-graph")
+    .args("add jsr:@denotest/add jsr:@denotest/subset-type-graph")
     .run();
   output.assert_exit_code(0);
   let output = output.combined_output();

tests/specs/add/add_with_subpath/__test__.jsonc

@@ -2,11 +2,11 @@
   "tempDir": true,
   "steps": [
     {
-      "args": "add @std/testing/bdd npm:preact/hooks",
+      "args": "add jsr:@std/testing/bdd npm:preact/hooks",
       "output": "add.out"
     },
     {
-      "args": "add @std/testing/bdd@1 npm:preact/hooks@10",
+      "args": "add jsr:@std/testing/bdd@1 npm:preact/hooks@10",
       "output": "wrong_constraint_jsr.out",
       "exitCode": 1
     },

tests/specs/add/add_with_subpath/wrong_constraint_jsr.out

@@ -1,4 +1,4 @@
-error: Failed to parse package required: @std/testing/bdd@1
+error: Failed to parse package required: jsr:@std/testing/bdd@1
 
 Caused by:
     Invalid package specifier 'jsr:@std/testing/bdd@1'. Did you mean to write 'jsr:@std/testing@1/bdd'?

tests/specs/add/missing_npm_specifier/__test__.jsonc

@@ -2,22 +2,22 @@
   "tempDir": true,
   "steps": [
     {
-      "args": "add ajv@latest",
+      "args": "add jsr: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",
+      "args": "add jsr: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",
+      "args": "add jsr: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
     },
     {
-      "args": "install ajv@latest",
+      "args": "install jsr: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
     }

tests/specs/add/missing_prefix/__test__.jsonc

@@ -0,0 +1,15 @@
+{
+  "tempDir": true,
+  "steps": [
+    {
+      "args": "add ajv",
+      "output": "error: ajv is missing a prefix. Did you mean `deno add npm:ajv`?\n",
+      "exitCode": 1
+    },
+    {
+      "args": "add @std/testing",
+      "output": "error: @std/testing is missing a prefix. Did you mean `deno add jsr:@std/testing`?\n",
+      "exitCode": 1
+    }
+  ]
+}

tests/specs/add/no_root_export/__test__.jsonc

@@ -2,7 +2,7 @@
   "tempDir": true,
   "steps": [
     {
-      "args": "add @std/testing",
+      "args": "add jsr:@std/testing",
       "output": "add.out"
     }
   ]

tests/specs/add/package_json_and_deno_json/__test__.jsonc

@@ -4,7 +4,7 @@
     "npm_prefers_package_json": {
       "steps": [
         {
-          "args": "add npm:@denotest/esm-basic @denotest/add npm:@denotest/say-hello",
+          "args": "add npm:@denotest/esm-basic jsr:@denotest/add npm:@denotest/say-hello",
           "output": "add.out"
         },
         {

tests/specs/remove/basic/__test__.jsonc

@@ -1,7 +1,7 @@
 {
   "tempDir": true,
   "steps": [{
-    "args": ["add", "@std/assert", "@std/http"],
+    "args": ["add", "jsr:@std/assert", "jsr:@std/http"],
     "output": "add.out"
   }, {
     "args": ["eval", "console.log(Deno.readTextFileSync('deno.lock').trim())"],