From d0a4e23ae824baf1994291d8480be29719f1b99a Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Fri, 25 Nov 2022 13:42:05 -0500
Subject: [PATCH] fix(npm): better error message when attempting to use
 typescript in npm packages (#16813)

---
 cli/node/mod.rs                               | 19 ++++++++++++-------
 cli/tests/integration/npm_tests.rs            |  8 ++++++++
 .../@denotest/typescript-file/1.0.0/index.ts  |  4 ++++
 .../typescript-file/1.0.0/package.json        |  5 +++++
 .../npm/typescript_file_in_package/main.out   |  6 ++++++
 .../npm/typescript_file_in_package/main.ts    |  5 +++++
 6 files changed, 40 insertions(+), 7 deletions(-)
 create mode 100644 cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/index.ts
 create mode 100644 cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/package.json
 create mode 100644 cli/tests/testdata/npm/typescript_file_in_package/main.out
 create mode 100644 cli/tests/testdata/npm/typescript_file_in_package/main.ts

diff --git a/cli/node/mod.rs b/cli/node/mod.rs
index 8f0f0ef6dc..fc4db33ee2 100644
--- a/cli/node/mod.rs
+++ b/cli/node/mod.rs
@@ -703,20 +703,25 @@ pub fn url_to_node_resolution(
   npm_resolver: &dyn RequireNpmResolver,
 ) -> Result<NodeResolution, AnyError> {
   let url_str = url.as_str().to_lowercase();
-  Ok(if url_str.starts_with("http") {
-    NodeResolution::Esm(url)
+  if url_str.starts_with("http") {
+    Ok(NodeResolution::Esm(url))
   } else if url_str.ends_with(".js") || url_str.ends_with(".d.ts") {
     let package_config = get_closest_package_json(&url, npm_resolver)?;
     if package_config.typ == "module" {
-      NodeResolution::Esm(url)
+      Ok(NodeResolution::Esm(url))
     } else {
-      NodeResolution::CommonJs(url)
+      Ok(NodeResolution::CommonJs(url))
     }
   } else if url_str.ends_with(".mjs") || url_str.ends_with(".d.mts") {
-    NodeResolution::Esm(url)
+    Ok(NodeResolution::Esm(url))
+  } else if url_str.ends_with(".ts") {
+    Err(generic_error(format!(
+      "TypeScript files are not supported in npm packages: {}",
+      url
+    )))
   } else {
-    NodeResolution::CommonJs(url)
-  })
+    Ok(NodeResolution::CommonJs(url))
+  }
 }
 
 fn finalize_resolution(
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index ea577d412e..288500ce44 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -314,6 +314,14 @@ itest!(types_no_types_entry {
   exit_code: 0,
 });
 
+itest!(typescript_file_in_package {
+  args: "run npm/typescript_file_in_package/main.ts",
+  output: "npm/typescript_file_in_package/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/registry/@denotest/typescript-file/1.0.0/index.ts b/cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/index.ts
new file mode 100644
index 0000000000..44b441a1e2
--- /dev/null
+++ b/cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/index.ts
@@ -0,0 +1,4 @@
+// this should not work because we don't support typescript files in npm packages
+export function getValue(): 5 {
+  return 5;
+}
\ No newline at end of file
diff --git a/cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/package.json
new file mode 100644
index 0000000000..e899f4100a
--- /dev/null
+++ b/cli/tests/testdata/npm/registry/@denotest/typescript-file/1.0.0/package.json
@@ -0,0 +1,5 @@
+{
+  "name": "@denotest/typescript-file",
+  "version": "1.0.0",
+  "main": "./index.ts"
+}
diff --git a/cli/tests/testdata/npm/typescript_file_in_package/main.out b/cli/tests/testdata/npm/typescript_file_in_package/main.out
new file mode 100644
index 0000000000..ba53f77255
--- /dev/null
+++ b/cli/tests/testdata/npm/typescript_file_in_package/main.out
@@ -0,0 +1,6 @@
+Download http://localhost:4545/npm/registry/@denotest/typescript-file
+Download http://localhost:4545/npm/registry/@denotest/typescript-file/1.0.0.tgz
+error: Could not resolve 'npm:@denotest/typescript-file'.
+
+Caused by:
+    TypeScript files are not supported in npm packages: file:///[WILDCARD]/@denotest/typescript-file/1.0.0/index.ts
diff --git a/cli/tests/testdata/npm/typescript_file_in_package/main.ts b/cli/tests/testdata/npm/typescript_file_in_package/main.ts
new file mode 100644
index 0000000000..aefc38ebe6
--- /dev/null
+++ b/cli/tests/testdata/npm/typescript_file_in_package/main.ts
@@ -0,0 +1,5 @@
+// We don't support typescript files in npm packages because we don't
+// want to encourage people distributing npm packages that aren't JavaScript.
+import { getValue } from "npm:@denotest/typescript-file";
+
+console.log(getValue());