fix(npm): better error is version is specified after subpath (#16131)

5 files changed, 40 insertions, 4 deletions

diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs
index d56cc87bc..497f40906 100644
--- a/cli/npm/resolution.rs
+++ b/cli/npm/resolution.rs
@@ -77,6 +77,18 @@ impl NpmPackageReference {
     } else {
       Some(parts[name_part_len..].join("/"))
     };
+
+    if let Some(sub_path) = &sub_path {
+      if let Some(at_index) = sub_path.rfind('@') {
+        let (new_sub_path, version) = sub_path.split_at(at_index);
+        let msg = format!(
+          "Invalid package specifier 'npm:{}/{}'. Did you mean to write 'npm:{}{}/{}'?",
+          name, sub_path, name, version, new_sub_path
+        );
+        return Err(generic_error(msg));
+      }
+    }
+
     Ok(NpmPackageReference {
       req: NpmPackageReq { name, version_req },
       sub_path,
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index 32ec12656..de3f81674 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -195,6 +195,14 @@ itest!(require_json {
   http_server: true,
 });
 
+itest!(error_version_after_subpath {
+  args: "run --unstable -A --quiet npm/error_version_after_subpath/main.js",
+  output: "npm/error_version_after_subpath/main.out",
+  envs: env_vars(),
+  http_server: true,
+  exit_code: 1,
+});
+
 #[test]
 fn parallel_downloading() {
   let (out, _err) = util::run_and_collect_output_with_args(
diff --git a/cli/tests/testdata/npm/error_version_after_subpath/main.js b/cli/tests/testdata/npm/error_version_after_subpath/main.js
new file mode 100644
index 000000000..77c7a017c
--- /dev/null
+++ b/cli/tests/testdata/npm/error_version_after_subpath/main.js
@@ -0,0 +1 @@
+import "npm:react-dom/server@18.2.0";
diff --git a/cli/tests/testdata/npm/error_version_after_subpath/main.out b/cli/tests/testdata/npm/error_version_after_subpath/main.out
new file mode 100644
index 000000000..0cdd1b6da
--- /dev/null
+++ b/cli/tests/testdata/npm/error_version_after_subpath/main.out
@@ -0,0 +1,2 @@
+error: Invalid package specifier 'npm:react-dom/server@18.2.0'. Did you mean to write 'npm:react-dom@18.2.0/server'?
+    at [WILDCARD]/npm/error_version_after_subpath/main.js:1:8
diff --git a/ext/node/errors.rs b/ext/node/errors.rs
index 4b55d1982..929f51e1b 100644
--- a/ext/node/errors.rs
+++ b/ext/node/errors.rs
@@ -81,23 +81,36 @@ pub fn err_invalid_package_target(
 }
 
 pub fn err_package_path_not_exported(
-  pkg_path: String,
+  mut pkg_path: String,
   subpath: String,
   maybe_referrer: Option<String>,
 ) -> AnyError {
   let mut msg = "[ERR_PACKAGE_PATH_NOT_EXPORTED]".to_string();
 
+  #[cfg(windows)]
+  {
+    if !pkg_path.ends_with('\\') {
+      pkg_path.push('\\');
+    }
+  }
+  #[cfg(not(windows))]
+  {
+    if !pkg_path.ends_with('/') {
+      pkg_path.push('/');
+    }
+  }
+
   if subpath == "." {
     msg = format!(
-      "{} No \"exports\" main defined in {}package.json",
+      "{} No \"exports\" main defined in '{}package.json'",
       msg, pkg_path
     );
   } else {
-    msg = format!("{} Package subpath \'{}\' is not defined by \"exports\" in {}package.json", msg, subpath, pkg_path);
+    msg = format!("{} Package subpath '{}' is not defined by \"exports\" in '{}package.json'", msg, subpath, pkg_path);
   };
 
   if let Some(referrer) = maybe_referrer {
-    msg = format!("{} imported from {}", msg, referrer);
+    msg = format!("{} imported from '{}'", msg, referrer);
   }
 
   generic_error(msg)