fix(cli): handle extensionless imports better (#13548)

Fixes #13526

cli/tests/integration/run_tests.rs

@@ -1488,6 +1488,12 @@ itest!(import_file_with_colon {
   http_server: true,
 });
 
+itest!(import_extensionless {
+  args: "run --quiet --reload import_extensionless.ts",
+  output: "import_extensionless.ts.out",
+  http_server: true,
+});
+
 itest!(classic_workers_event_loop {
   args:
     "run --enable-testing-features-do-not-use classic_workers_event_loop.js",

cli/tests/testdata/import_extensionless.ts

@@ -0,0 +1,3 @@
+import { printHello3 } from "http://localhost:4545/v1/extensionless";
+
+printHello3();

cli/tests/testdata/import_extensionless.ts.out

@@ -0,0 +1,2 @@
+[WILDCARD]
+Hello

cli/tsc.rs

@@ -141,6 +141,30 @@ fn hash_url(specifier: &ModuleSpecifier, media_type: &MediaType) -> String {
   )
 }
 
+/// If the provided URLs derivable tsc media type doesn't match the media type,
+/// we will add an extension to the output.  This is to avoid issues with
+/// specifiers that don't have extensions, that tsc refuses to emit because they
+/// think a `.js` version exists, when it doesn't.
+fn maybe_remap_specifier(
+  specifier: &ModuleSpecifier,
+  media_type: &MediaType,
+) -> Option<String> {
+  let path = if specifier.scheme() == "file" {
+    if let Ok(path) = specifier.to_file_path() {
+      path
+    } else {
+      PathBuf::from(specifier.path())
+    }
+  } else {
+    PathBuf::from(specifier.path())
+  };
+  if path.extension().is_none() {
+    Some(format!("{}{}", specifier, media_type.as_ts_extension()))
+  } else {
+    None
+  }
+}
+
 /// tsc only supports `.ts`, `.tsx`, `.d.ts`, `.js`, or `.jsx` as root modules
 /// and so we have to detect the apparent media type based on extensions it
 /// supports.
@@ -235,13 +259,13 @@ pub(crate) struct Response {
 
 #[derive(Debug)]
 struct State {
-  data_url_map: HashMap<String, ModuleSpecifier>,
   hash_data: Vec<Vec<u8>>,
   emitted_files: Vec<EmittedFile>,
   graph_data: Arc<RwLock<GraphData>>,
   maybe_config_specifier: Option<ModuleSpecifier>,
   maybe_tsbuildinfo: Option<String>,
   maybe_response: Option<RespondArgs>,
+  remapped_specifiers: HashMap<String, ModuleSpecifier>,
   root_map: HashMap<String, ModuleSpecifier>,
 }
 
@@ -252,16 +276,16 @@ impl State {
     maybe_config_specifier: Option<ModuleSpecifier>,
     maybe_tsbuildinfo: Option<String>,
     root_map: HashMap<String, ModuleSpecifier>,
-    data_url_map: HashMap<String, ModuleSpecifier>,
+    remapped_specifiers: HashMap<String, ModuleSpecifier>,
   ) -> Self {
     State {
-      data_url_map,
       hash_data,
       emitted_files: Default::default(),
       graph_data,
       maybe_config_specifier,
       maybe_tsbuildinfo,
       maybe_response: None,
+      remapped_specifiers,
       root_map,
     }
   }
@@ -335,7 +359,7 @@ fn op_emit(state: &mut State, args: Value) -> Result<Value, AnyError> {
         let specifiers = specifiers
           .iter()
           .map(|s| {
-            if let Some(data_specifier) = state.data_url_map.get(s) {
+            if let Some(data_specifier) = state.remapped_specifiers.get(s) {
               data_specifier.clone()
             } else if let Some(remapped_specifier) = state.root_map.get(s) {
               remapped_specifier.clone()
@@ -423,10 +447,10 @@ fn op_load(state: &mut State, args: Value) -> Result<Value, AnyError> {
     media_type = MediaType::from(&v.specifier);
     maybe_source
   } else {
-    let specifier = if let Some(data_specifier) =
+    let specifier = if let Some(remapped_specifier) =
-      state.data_url_map.get(&v.specifier)
+      state.remapped_specifiers.get(&v.specifier)
     {
-      data_specifier.clone()
+      remapped_specifier.clone()
     } else if let Some(remapped_specifier) = state.root_map.get(&v.specifier) {
       remapped_specifier.clone()
     } else {
@@ -465,20 +489,20 @@ pub struct ResolveArgs {
   pub specifiers: Vec<String>,
 }
 
-fn op_resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
+fn op_resolve(state: &mut State, args: ResolveArgs) -> Result<Value, AnyError> {
-  let v: ResolveArgs = serde_json::from_value(args)
-    .context("Invalid request from JavaScript for \"op_resolve\".")?;
   let mut resolved: Vec<(String, String)> = Vec::new();
-  let referrer = if let Some(data_specifier) = state.data_url_map.get(&v.base) {
+  let referrer = if let Some(remapped_specifier) =
-    data_specifier.clone()
+    state.remapped_specifiers.get(&args.base)
-  } else if let Some(remapped_base) = state.root_map.get(&v.base) {
+  {
+    remapped_specifier.clone()
+  } else if let Some(remapped_base) = state.root_map.get(&args.base) {
     remapped_base.clone()
   } else {
-    normalize_specifier(&v.base).context(
+    normalize_specifier(&args.base).context(
       "Error converting a string module specifier for \"op_resolve\".",
     )?
   };
-  for specifier in &v.specifiers {
+  for specifier in &args.specifiers {
     if specifier.starts_with("asset:///") {
       resolved.push((
         specifier.clone(),
@@ -528,10 +552,23 @@ fn op_resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
           let specifier_str = match specifier.scheme() {
             "data" | "blob" => {
               let specifier_str = hash_url(&specifier, media_type);
-              state.data_url_map.insert(specifier_str.clone(), specifier);
+              state
+                .remapped_specifiers
+                .insert(specifier_str.clone(), specifier);
               specifier_str
             }
-            _ => specifier.to_string(),
+            _ => {
+              if let Some(specifier_str) =
+                maybe_remap_specifier(&specifier, media_type)
+              {
+                state
+                  .remapped_specifiers
+                  .insert(specifier_str.clone(), specifier);
+                specifier_str
+              } else {
+                specifier.to_string()
+              }
+            }
           };
           (specifier_str, media_type.as_ts_extension().into())
         }
@@ -573,14 +610,14 @@ pub(crate) fn exec(request: Request) -> Result<Response, AnyError> {
   // extensions and remap any that are unacceptable to tsc and add them to the
   // op state so when requested, we can remap to the original specifier.
   let mut root_map = HashMap::new();
-  let mut data_url_map = HashMap::new();
+  let mut remapped_specifiers = HashMap::new();
   let root_names: Vec<String> = request
     .root_names
     .iter()
     .map(|(s, mt)| match s.scheme() {
       "data" | "blob" => {
         let specifier_str = hash_url(s, mt);
-        data_url_map.insert(specifier_str.clone(), s.clone());
+        remapped_specifiers.insert(specifier_str.clone(), s.clone());
         specifier_str
       }
       _ => {
@@ -605,7 +642,7 @@ pub(crate) fn exec(request: Request) -> Result<Response, AnyError> {
       request.maybe_config_specifier.clone(),
       request.maybe_tsbuildinfo.clone(),
       root_map,
-      data_url_map,
+      remapped_specifiers,
     ));
   }
 
@@ -981,7 +1018,10 @@ mod tests {
     .await;
     let actual = op_resolve(
       &mut state,
-      json!({ "base": "https://deno.land/x/a.ts", "specifiers": [ "./b.ts" ]}),
+      ResolveArgs {
+        base: "https://deno.land/x/a.ts".to_string(),
+        specifiers: vec!["./b.ts".to_string()],
+      },
     )
     .expect("should have invoked op");
     assert_eq!(actual, json!([["https://deno.land/x/b.ts", ".ts"]]));
@@ -997,8 +1037,12 @@ mod tests {
     .await;
     let actual = op_resolve(
       &mut state,
-      json!({ "base": "https://deno.land/x/a.ts", "specifiers": [ "./bad.ts" ]}),
+      ResolveArgs {
-    ).expect("should have not errored");
+        base: "https://deno.land/x/a.ts".to_string(),
+        specifiers: vec!["./bad.ts".to_string()],
+      },
+    )
+    .expect("should have not errored");
     assert_eq!(
       actual,
       json!([["deno:///missing_dependency.d.ts", ".d.ts"]])

test_util/src/lib.rs

@@ -840,6 +840,15 @@ async fn main_server(
       );
       Ok(res)
     }
+    (_, "/v1/extensionless") => {
+      let mut res =
+        Response::new(Body::from(r#"export * from "/subdir/mod1.ts";"#));
+      res.headers_mut().insert(
+        "content-type",
+        HeaderValue::from_static("application/typescript"),
+      );
+      Ok(res)
+    }
     (_, "/subdir/no_js_ext@1.0.0") => {
       let mut res = Response::new(Body::from(
         r#"import { printHello } from "./mod2.ts";