feat: allow specification of import map in config file (denoland#13739)

Closes: denoland#12800
kt3k · Feb 22, 2022 · ee27b9d · ee27b9d
1 parent 6613a31
commit ee27b9d
Show file tree

Hide file tree

Showing 7 changed files with 335 additions and 56 deletions.
diff --git a/cli/config_file.rs b/cli/config_file.rs
@@ -227,6 +227,56 @@ pub fn json_merge(a: &mut Value, b: &Value) {
   }
 }
 
+/// Based on an optional command line import map path and an optional
+/// configuration file, return a resolved module specifier to an import map.
+pub fn resolve_import_map_specifier(
+  maybe_import_map_path: Option<&str>,
+  maybe_config_file: Option<&ConfigFile>,
+) -> Result<Option<ModuleSpecifier>, AnyError> {
+  if let Some(import_map_path) = maybe_import_map_path {
+    if let Some(config_file) = &maybe_config_file {
+      if config_file.to_import_map_path().is_some() {
+        log::warn!("{} the configuration file \"{}\" contains an entry for \"importMap\" that is being ignored.", crate::colors::yellow("Warning"), config_file.specifier);
+      }
+    }
+    let specifier = deno_core::resolve_url_or_path(import_map_path)
+      .context(format!("Bad URL (\"{}\") for import map.", import_map_path))?;
+    return Ok(Some(specifier));
+  } else if let Some(config_file) = &maybe_config_file {
+    // when the import map is specifier in a config file, it needs to be
+    // resolved relative to the config file, versus the CWD like with the flag
+    // and with config files, we support both local and remote config files,
+    // so we have treat them differently.
+    if let Some(import_map_path) = config_file.to_import_map_path() {
+      let specifier =
+          // with local config files, it might be common to specify an import
+          // map like `"importMap": "import-map.json"`, which is resolvable if
+          // the file is resolved like a file path, so we will coerce the config
+          // file into a file path if possible and join the import map path to
+          // the file path.
+          if let Ok(config_file_path) = config_file.specifier.to_file_path() {
+            let import_map_file_path = config_file_path
+              .parent()
+              .ok_or_else(|| {
+                anyhow!("Bad config file specifier: {}", config_file.specifier)
+              })?
+              .join(&import_map_path);
+            ModuleSpecifier::from_file_path(import_map_file_path).unwrap()
+          // otherwise if the config file is remote, we have no choice but to
+          // use "import resolution" with the config file as the base.
+          } else {
+            deno_core::resolve_import(&import_map_path, config_file.specifier.as_str())
+              .context(format!(
+                "Bad URL (\"{}\") for import map.",
+                import_map_path
+              ))?
+          };
+      return Ok(Some(specifier));
+    }
+  }
+  Ok(None)
+}
+
 fn parse_compiler_options(
   compiler_options: &HashMap<String, Value>,
   maybe_specifier: Option<ModuleSpecifier>,
@@ -476,6 +526,7 @@ pub struct FmtConfig {
 #[serde(rename_all = "camelCase")]
 pub struct ConfigFileJson {
   pub compiler_options: Option<Value>,
+  pub import_map: Option<String>,
   pub lint: Option<Value>,
   pub fmt: Option<Value>,
 }
@@ -583,6 +634,10 @@ impl ConfigFile {
     }
   }
 
+  pub fn to_import_map_path(&self) -> Option<String> {
+    self.json.import_map.clone()
+  }
+
   pub fn to_lint_config(&self) -> Result<Option<LintConfig>, AnyError> {
     if let Some(config) = self.json.lint.clone() {
       let lint_config: SerializedLintConfig = serde_json::from_value(config)
@@ -914,4 +969,80 @@ mod tests {
     let err = discover_from(&d, &mut checked).unwrap_err();
     assert!(err.to_string().contains("Unable to parse config file"));
   }
+
+  #[cfg(not(windows))]
+  #[test]
+  fn resolve_import_map_config_file() {
+    let config_text = r#"{
+      "importMap": "import_map.json"
+    }"#;
+    let config_specifier =
+      ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap();
+    let config_file = ConfigFile::new(config_text, &config_specifier).unwrap();
+    let actual = resolve_import_map_specifier(None, Some(&config_file));
+    assert!(actual.is_ok());
+    let actual = actual.unwrap();
+    assert_eq!(
+      actual,
+      Some(ModuleSpecifier::parse("file:///deno/import_map.json").unwrap())
+    );
+  }
+
+  #[test]
+  fn resolve_import_map_config_file_remote() {
+    let config_text = r#"{
+      "importMap": "./import_map.json"
+    }"#;
+    let config_specifier =
+      ModuleSpecifier::parse("https://example.com/deno.jsonc").unwrap();
+    let config_file = ConfigFile::new(config_text, &config_specifier).unwrap();
+    let actual = resolve_import_map_specifier(None, Some(&config_file));
+    assert!(actual.is_ok());
+    let actual = actual.unwrap();
+    assert_eq!(
+      actual,
+      Some(
+        ModuleSpecifier::parse("https://example.com/import_map.json").unwrap()
+      )
+    );
+  }
+
+  #[test]
+  fn resolve_import_map_flags_take_precedence() {
+    let config_text = r#"{
+      "importMap": "import_map.json"
+    }"#;
+    let config_specifier =
+      ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap();
+    let config_file = ConfigFile::new(config_text, &config_specifier).unwrap();
+    let actual =
+      resolve_import_map_specifier(Some("import-map.json"), Some(&config_file));
+    let import_map_path =
+      std::env::current_dir().unwrap().join("import-map.json");
+    let expected_specifier =
+      ModuleSpecifier::from_file_path(&import_map_path).unwrap();
+    assert!(actual.is_ok());
+    let actual = actual.unwrap();
+    assert_eq!(actual, Some(expected_specifier));
+  }
+
+  #[test]
+  fn resolve_import_map_none() {
+    let config_text = r#"{}"#;
+    let config_specifier =
+      ModuleSpecifier::parse("file:///deno/deno.jsonc").unwrap();
+    let config_file = ConfigFile::new(config_text, &config_specifier).unwrap();
+    let actual = resolve_import_map_specifier(None, Some(&config_file));
+    assert!(actual.is_ok());
+    let actual = actual.unwrap();
+    assert_eq!(actual, None);
+  }
+
+  #[test]
+  fn resolve_import_map_no_config() {
+    let actual = resolve_import_map_specifier(None, None);
+    assert!(actual.is_ok());
+    let actual = actual.unwrap();
+    assert_eq!(actual, None);
+  }
 }
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
@@ -465,29 +465,65 @@ impl Inner {
   pub async fn update_import_map(&mut self) -> Result<(), AnyError> {
     let mark = self.performance.mark("update_import_map", None::<()>);
     self.maybe_cache_server = None;
-    let maybe_import_map = self.config.get_workspace_settings().import_map;
-    if let Some(import_map_str) = &maybe_import_map {
-      lsp_log!("Setting import map from: \"{}\"", import_map_str);
-      let import_map_url = if let Ok(url) = Url::from_file_path(import_map_str)
-      {
-        Ok(url)
+    let maybe_import_map_url = if let Some(import_map_str) =
+      self.config.get_workspace_settings().import_map
+    {
+      lsp_log!(
+        "Setting import map from workspace settings: \"{}\"",
+        import_map_str
+      );
+      if let Some(config_file) = &self.maybe_config_file {
+        if let Some(import_map_path) = config_file.to_import_map_path() {
+          lsp_log!("Warning: Import map \"{}\" configured in \"{}\" being ignored due to an import map being explicitly configured in workspace settings.", import_map_path, config_file.specifier);
+        }
+      }
+      if let Ok(url) = Url::from_file_path(&import_map_str) {
+        Some(url)
       } else if import_map_str.starts_with("data:") {
-        Url::parse(import_map_str).map_err(|_| {
-          anyhow!("Bad data url for import map: {:?}", import_map_str)
-        })
+        Some(Url::parse(&import_map_str).map_err(|_| {
+          anyhow!("Bad data url for import map: {}", import_map_str)
+        })?)
       } else if let Some(root_uri) = &self.root_uri {
         let root_path = fs_util::specifier_to_file_path(root_uri)?;
-        let import_map_path = root_path.join(import_map_str);
-        Url::from_file_path(import_map_path).map_err(|_| {
-          anyhow!("Bad file path for import map: {:?}", import_map_str)
-        })
+        let import_map_path = root_path.join(&import_map_str);
+        Some(Url::from_file_path(import_map_path).map_err(|_| {
+          anyhow!("Bad file path for import map: {}", import_map_str)
+        })?)
       } else {
-        Err(anyhow!(
+        return Err(anyhow!(
           "The path to the import map (\"{}\") is not resolvable.",
           import_map_str
-        ))
-      }?;
-
+        ));
+      }
+    } else if let Some(config_file) = &self.maybe_config_file {
+      if let Some(import_map_path) = config_file.to_import_map_path() {
+        lsp_log!(
+          "Setting import map from configuration file: \"{}\"",
+          import_map_path
+        );
+        let specifier =
+          if let Ok(config_file_path) = config_file.specifier.to_file_path() {
+            let import_map_file_path = config_file_path
+              .parent()
+              .ok_or_else(|| {
+                anyhow!("Bad config file specifier: {}", config_file.specifier)
+              })?
+              .join(&import_map_path);
+            ModuleSpecifier::from_file_path(import_map_file_path).unwrap()
+          } else {
+            deno_core::resolve_import(
+              &import_map_path,
+              config_file.specifier.as_str(),
+            )?
+          };
+        Some(specifier)
+      } else {
+        None
+      }
+    } else {
+      None
+    };
+    if let Some(import_map_url) = maybe_import_map_url {
       let import_map_json = if import_map_url.scheme() == "data" {
         get_source_from_data_url(&import_map_url)?.0
       } else {
@@ -508,6 +544,7 @@ impl Inner {
       self.maybe_import_map_uri = Some(import_map_url);
       self.maybe_import_map = Some(Arc::new(import_map));
     } else {
+      self.maybe_import_map_uri = None;
       self.maybe_import_map = None;
     }
     self.performance.measure(mark);
@@ -854,15 +891,15 @@ impl Inner {
     if let Err(err) = self.update_cache() {
       self.client.show_message(MessageType::WARNING, err).await;
     }
-    if let Err(err) = self.update_import_map().await {
-      self.client.show_message(MessageType::WARNING, err).await;
-    }
     if let Err(err) = self.update_registries().await {
       self.client.show_message(MessageType::WARNING, err).await;
     }
     if let Err(err) = self.update_config_file() {
       self.client.show_message(MessageType::WARNING, err).await;
     }
+    if let Err(err) = self.update_import_map().await {
+      self.client.show_message(MessageType::WARNING, err).await;
+    }
     if let Err(err) = self.update_tsconfig().await {
       self.client.show_message(MessageType::WARNING, err).await;
     }
@@ -889,15 +926,6 @@ impl Inner {
       .map(|f| self.url_map.normalize_url(&f.uri))
       .collect();
 
-    // if the current import map has changed, we need to reload it
-    if let Some(import_map_uri) = &self.maybe_import_map_uri {
-      if changes.iter().any(|uri| import_map_uri == uri) {
-        if let Err(err) = self.update_import_map().await {
-          self.client.show_message(MessageType::WARNING, err).await;
-        }
-        touched = true;
-      }
-    }
     // if the current tsconfig has changed, we need to reload it
     if let Some(config_file) = &self.maybe_config_file {
       if changes.iter().any(|uri| config_file.specifier == *uri) {
@@ -910,6 +938,16 @@ impl Inner {
         touched = true;
       }
     }
+    // if the current import map, or config file has changed, we need to reload
+    // reload the import map
+    if let Some(import_map_uri) = &self.maybe_import_map_uri {
+      if changes.iter().any(|uri| import_map_uri == uri) || touched {
+        if let Err(err) = self.update_import_map().await {
+          self.client.show_message(MessageType::WARNING, err).await;
+        }
+        touched = true;
+      }
+    }
     if touched {
       self.documents.update_config(
         self.maybe_import_map.clone(),

diff --git a/cli/main.rs b/cli/main.rs
@@ -803,9 +803,14 @@ async fn bundle_command(
         })
         .collect();
 
-      if let Some(import_map) = ps.flags.import_map_path.as_ref() {
-        paths_to_watch
-          .push(fs_util::resolve_from_cwd(std::path::Path::new(import_map))?);
+      if let Ok(Some(import_map_path)) =
+        config_file::resolve_import_map_specifier(
+          ps.flags.import_map_path.as_deref(),
+          ps.maybe_config_file.as_ref(),
+        )
+        .map(|ms| ms.map(|ref s| s.to_file_path().ok()).flatten())
+      {
+        paths_to_watch.push(import_map_path);
       }
 
       Ok((paths_to_watch, graph, ps))
@@ -1047,9 +1052,14 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<i32, AnyError> {
         paths_to_watch.extend(watch_paths);
       }
 
-      if let Some(import_map) = ps.flags.import_map_path.as_ref() {
-        paths_to_watch
-          .push(fs_util::resolve_from_cwd(std::path::Path::new(import_map))?);
+      if let Ok(Some(import_map_path)) =
+        config_file::resolve_import_map_specifier(
+          ps.flags.import_map_path.as_deref(),
+          ps.maybe_config_file.as_ref(),
+        )
+        .map(|ms| ms.map(|ref s| s.to_file_path().ok()).flatten())
+      {
+        paths_to_watch.push(import_map_path);
       }
 
       Ok((paths_to_watch, main_module, ps))

diff --git a/cli/proc_state.rs b/cli/proc_state.rs
@@ -151,26 +151,26 @@ impl ProcState {
 
     let maybe_config_file = crate::config_file::discover(&flags)?;
 
-    let maybe_import_map: Option<Arc<ImportMap>> =
-      match flags.import_map_path.as_ref() {
-        None => None,
-        Some(import_map_url) => {
-          let import_map_specifier =
-            deno_core::resolve_url_or_path(import_map_url).context(format!(
-              "Bad URL (\"{}\") for import map.",
-              import_map_url
-            ))?;
-          let file = file_fetcher
-            .fetch(&import_map_specifier, &mut Permissions::allow_all())
-            .await
-            .context(format!(
-              "Unable to load '{}' import map",
-              import_map_specifier
-            ))?;
-          let import_map =
-            import_map_from_text(&import_map_specifier, &file.source)?;
-          Some(Arc::new(import_map))
-        }
+    let maybe_import_map_specifier =
+      crate::config_file::resolve_import_map_specifier(
+        flags.import_map_path.as_deref(),
+        maybe_config_file.as_ref(),
+      )?;
+
+    let maybe_import_map =
+      if let Some(import_map_specifier) = maybe_import_map_specifier {
+        let file = file_fetcher
+          .fetch(&import_map_specifier, &mut Permissions::allow_all())
+          .await
+          .context(format!(
+            "Unable to load '{}' import map",
+            import_map_specifier
+          ))?;
+        let import_map =
+          import_map_from_text(&import_map_specifier, &file.source)?;
+        Some(Arc::new(import_map))
+      } else {
+        None
       };
 
     let maybe_inspect_host = flags.inspect.or(flags.inspect_brk);
@@ -597,7 +597,7 @@ impl SourceMapGetter for ProcState {
     if let Ok(specifier) = resolve_url(file_name) {
       self.file_fetcher.get_source(&specifier).map(|out| {
         // Do NOT use .lines(): it skips the terminating empty line.
-        // (due to internally using .split_terminator() instead of .split())
+        // (due to internally using_terminator() instead of .split())
         let lines: Vec<&str> = out.source.split('\n').collect();
         if line_number >= lines.len() {
           format!(