refactor: Combine ImportMapResolver and JsxResolver (#16508)

These resolvers are used in `deno_graph` APIs. Combining them removes a
lot of code and
unblocks me on https://github.com/denoland/deno/pull/16157

cli/lsp/documents.rs

@@ -19,8 +19,7 @@ use crate::node::NodeResolutionMode;
 use crate::npm::NpmPackageReference;
 use crate::npm::NpmPackageReq;
 use crate::npm::NpmPackageResolver;
-use crate::resolver::ImportMapResolver;
-use crate::resolver::JsxResolver;
+use crate::resolver::CliResolver;
 use crate::text_encoding;
 
 use deno_ast::MediaType;
@@ -707,10 +706,9 @@ pub struct Documents {
   /// Any imports to the context supplied by configuration files. This is like
   /// the imports into the a module graph in CLI.
   imports: Arc<HashMap<ModuleSpecifier, GraphImport>>,
-  /// The optional import map that should be used when resolving dependencies.
-  maybe_import_map: Option<ImportMapResolver>,
-  /// The optional JSX resolver, which is used when JSX imports are configured.
-  maybe_jsx_resolver: Option<JsxResolver>,
+  /// A resolver that takes into account currently loaded import map and JSX
+  /// settings.
+  maybe_resolver: Option<CliResolver>,
   /// The npm package requirements.
   npm_reqs: HashSet<NpmPackageReq>,
   /// Resolves a specifier to its final redirected to specifier.
@@ -726,8 +724,7 @@ impl Documents {
       open_docs: HashMap::default(),
       file_system_docs: Default::default(),
       imports: Default::default(),
-      maybe_import_map: None,
-      maybe_jsx_resolver: None,
+      maybe_resolver: None,
       npm_reqs: HashSet::new(),
       specifier_resolver: Arc::new(SpecifierResolver::new(location)),
     }
@@ -1049,11 +1046,10 @@ impl Documents {
     maybe_config_file: Option<&ConfigFile>,
   ) {
     // TODO(@kitsonk) update resolved dependencies?
-    self.maybe_import_map = maybe_import_map.map(ImportMapResolver::new);
-    self.maybe_jsx_resolver = maybe_config_file.and_then(|cf| {
-      cf.to_maybe_jsx_import_source_config()
-        .map(|cfg| JsxResolver::new(cfg, self.maybe_import_map.clone()))
-    });
+    let maybe_jsx_config =
+      maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config());
+    self.maybe_resolver =
+      CliResolver::maybe_new(maybe_jsx_config, maybe_import_map);
     self.imports = Arc::new(
       if let Some(Ok(Some(imports))) =
         maybe_config_file.map(|cf| cf.to_maybe_imports())
@@ -1125,11 +1121,7 @@ impl Documents {
   }
 
   fn get_maybe_resolver(&self) -> Option<&dyn deno_graph::source::Resolver> {
-    if self.maybe_jsx_resolver.is_some() {
-      self.maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
-    } else {
-      self.maybe_import_map.as_ref().map(|im| im.as_resolver())
-    }
+    self.maybe_resolver.as_ref().map(|r| r.as_graph_resolver())
   }
 
   fn resolve_dependency(

cli/main.rs

@@ -70,8 +70,7 @@ use crate::file_watcher::ResolutionResult;
 use crate::graph_util::graph_lock_or_exit;
 use crate::graph_util::graph_valid;
 use crate::proc_state::ProcState;
-use crate::resolver::ImportMapResolver;
-use crate::resolver::JsxResolver;
+use crate::resolver::CliResolver;
 use crate::tools::check;
 
 use args::CliOptions;
@@ -361,19 +360,12 @@ async fn create_graph_and_maybe_check(
   );
   let maybe_locker = lockfile::as_maybe_locker(ps.lockfile.clone());
   let maybe_imports = ps.options.to_maybe_imports()?;
-  let maybe_import_map_resolver =
-    ps.maybe_import_map.clone().map(ImportMapResolver::new);
-  let maybe_jsx_resolver = ps
-    .options
-    .to_maybe_jsx_import_source_config()
-    .map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
-  let maybe_resolver = if maybe_jsx_resolver.is_some() {
-    maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
-  } else {
-    maybe_import_map_resolver
-      .as_ref()
-      .map(|im| im.as_resolver())
-  };
+  let maybe_cli_resolver = CliResolver::maybe_new(
+    ps.options.to_maybe_jsx_import_source_config(),
+    ps.maybe_import_map.clone(),
+  );
+  let maybe_graph_resolver =
+    maybe_cli_resolver.as_ref().map(|r| r.as_graph_resolver());
   let analyzer = ps.parsed_source_cache.as_analyzer();
   let graph = Arc::new(
     deno_graph::create_graph(
@@ -382,7 +374,7 @@ async fn create_graph_and_maybe_check(
       deno_graph::GraphOptions {
         is_dynamic: false,
         imports: maybe_imports,
-        resolver: maybe_resolver,
+        resolver: maybe_graph_resolver,
         locker: maybe_locker,
         module_analyzer: Some(&*analyzer),
         reporter: None,

cli/proc_state.rs

@@ -28,8 +28,7 @@ use crate::npm::NpmPackageReference;
 use crate::npm::NpmPackageResolver;
 use crate::npm::NpmRegistryApi;
 use crate::progress_bar::ProgressBar;
-use crate::resolver::ImportMapResolver;
-use crate::resolver::JsxResolver;
+use crate::resolver::CliResolver;
 use crate::tools::check;
 
 use deno_ast::MediaType;
@@ -49,6 +48,7 @@ use deno_graph::create_graph;
 use deno_graph::source::CacheInfo;
 use deno_graph::source::LoadFuture;
 use deno_graph::source::Loader;
+use deno_graph::source::Resolver;
 use deno_graph::ModuleKind;
 use deno_graph::Resolved;
 use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
@@ -88,7 +88,7 @@ pub struct Inner {
   pub shared_array_buffer_store: SharedArrayBufferStore,
   pub compiled_wasm_module_store: CompiledWasmModuleStore,
   pub parsed_source_cache: ParsedSourceCache,
-  maybe_resolver: Option<Arc<dyn deno_graph::source::Resolver + Send + Sync>>,
+  maybe_resolver: Option<Arc<CliResolver>>,
   maybe_file_watcher_reporter: Option<FileWatcherReporter>,
   pub node_analysis_cache: NodeAnalysisCache,
   pub npm_cache: NpmCache,
@@ -193,22 +193,11 @@ impl ProcState {
     let maybe_inspector_server =
       cli_options.resolve_inspector_server().map(Arc::new);
 
-    let maybe_import_map_resolver =
-      maybe_import_map.clone().map(ImportMapResolver::new);
-    let maybe_jsx_resolver = cli_options
-      .to_maybe_jsx_import_source_config()
-      .map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
-    let maybe_resolver: Option<
-      Arc<dyn deno_graph::source::Resolver + Send + Sync>,
-    > = if let Some(jsx_resolver) = maybe_jsx_resolver {
-      // the JSX resolver offloads to the import map if present, otherwise uses
-      // the default Deno explicit import resolution.
-      Some(Arc::new(jsx_resolver))
-    } else if let Some(import_map_resolver) = maybe_import_map_resolver {
-      Some(Arc::new(import_map_resolver))
-    } else {
-      None
-    };
+    let maybe_cli_resolver = CliResolver::maybe_new(
+      cli_options.to_maybe_jsx_import_source_config(),
+      maybe_import_map.clone(),
+    );
+    let maybe_resolver = maybe_cli_resolver.map(Arc::new);
 
     let maybe_file_watcher_reporter =
       maybe_sender.map(|sender| FileWatcherReporter {
@@ -335,12 +324,8 @@ impl ProcState {
     );
     let maybe_locker = as_maybe_locker(self.lockfile.clone());
     let maybe_imports = self.options.to_maybe_imports()?;
-    let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
-      if let Some(resolver) = &self.maybe_resolver {
-        Some(resolver.as_ref())
-      } else {
-        None
-      };
+    let maybe_resolver =
+      self.maybe_resolver.as_ref().map(|r| r.as_graph_resolver());
 
     struct ProcStateLoader<'a> {
       inner: &'a mut cache::FetchCacher,
@@ -571,13 +556,7 @@ impl ProcState {
       deno_core::resolve_url_or_path(referrer).unwrap()
     };
 
-    let maybe_resolver: Option<&dyn deno_graph::source::Resolver> =
-      if let Some(resolver) = &self.maybe_resolver {
-        Some(resolver.as_ref())
-      } else {
-        None
-      };
-    if let Some(resolver) = &maybe_resolver {
+    if let Some(resolver) = &self.maybe_resolver {
       resolver.resolve(specifier, &referrer).to_result()
     } else {
       deno_core::resolve_import(specifier, referrer.as_str())
@@ -640,20 +619,14 @@ impl ProcState {
     loader: &mut dyn Loader,
   ) -> Result<deno_graph::ModuleGraph, AnyError> {
     let maybe_locker = as_maybe_locker(self.lockfile.clone());
-    let maybe_import_map_resolver =
-      self.maybe_import_map.clone().map(ImportMapResolver::new);
     let maybe_imports = self.options.to_maybe_imports()?;
-    let maybe_jsx_resolver = self
-      .options
-      .to_maybe_jsx_import_source_config()
-      .map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
-    let maybe_resolver = if maybe_jsx_resolver.is_some() {
-      maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
-    } else {
-      maybe_import_map_resolver
-        .as_ref()
-        .map(|im| im.as_resolver())
-    };
+
+    let maybe_cli_resolver = CliResolver::maybe_new(
+      self.options.to_maybe_jsx_import_source_config(),
+      self.maybe_import_map.clone(),
+    );
+    let maybe_graph_resolver =
+      maybe_cli_resolver.as_ref().map(|r| r.as_graph_resolver());
     let analyzer = self.parsed_source_cache.as_analyzer();
 
     let graph = create_graph(
@@ -662,7 +635,7 @@ impl ProcState {
       deno_graph::GraphOptions {
         is_dynamic: false,
         imports: maybe_imports,
-        resolver: maybe_resolver,
+        resolver: maybe_graph_resolver,
         locker: maybe_locker,
         module_analyzer: Some(&*analyzer),
         reporter: None,

cli/resolver.rs

@@ -4,71 +4,59 @@ use deno_core::resolve_import;
 use deno_core::ModuleSpecifier;
 use deno_graph::source::ResolveResponse;
 use deno_graph::source::Resolver;
+use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE;
 use import_map::ImportMap;
 use std::sync::Arc;
 
 use crate::args::config_file::JsxImportSourceConfig;
 
-/// Wraps an import map to be used when building a deno_graph module graph.
-/// This is done to avoid having `import_map` be a direct dependency of
-/// `deno_graph`.
-#[derive(Debug, Clone)]
-pub struct ImportMapResolver(Arc<ImportMap>);
+/// A resolver that takes care of resolution, taking into account loaded
+/// import map, JSX settings.
+#[derive(Debug, Clone, Default)]
+pub struct CliResolver {
+  maybe_import_map: Option<Arc<ImportMap>>,
+  maybe_default_jsx_import_source: Option<String>,
+  maybe_jsx_import_source_module: Option<String>,
+}
 
-impl ImportMapResolver {
-  pub fn new(import_map: Arc<ImportMap>) -> Self {
-    Self(import_map)
+impl CliResolver {
+  pub fn maybe_new(
+    maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
+    maybe_import_map: Option<Arc<ImportMap>>,
+  ) -> Option<Self> {
+    if maybe_jsx_import_source_config.is_some() || maybe_import_map.is_some() {
+      Some(Self {
+        maybe_import_map,
+        maybe_default_jsx_import_source: maybe_jsx_import_source_config
+          .as_ref()
+          .and_then(|c| c.default_specifier.clone()),
+        maybe_jsx_import_source_module: maybe_jsx_import_source_config
+          .map(|c| c.module),
+      })
+    } else {
+      None
+    }
   }
 
-  pub fn as_resolver(&self) -> &dyn Resolver {
+  pub fn with_import_map(import_map: Arc<ImportMap>) -> Self {
+    Self::maybe_new(None, Some(import_map)).unwrap()
+  }
+
+  pub fn as_graph_resolver(&self) -> &dyn Resolver {
     self
   }
 }
 
-impl Resolver for ImportMapResolver {
-  fn resolve(
-    &self,
-    specifier: &str,
-    referrer: &ModuleSpecifier,
-  ) -> ResolveResponse {
-    match self.0.resolve(specifier, referrer) {
-      Ok(resolved_specifier) => ResolveResponse::Specifier(resolved_specifier),
-      Err(err) => ResolveResponse::Err(err.into()),
-    }
-  }
-}
-
-#[derive(Debug, Default, Clone)]
-pub struct JsxResolver {
-  default_jsx_import_source: Option<String>,
-  jsx_import_source_module: String,
-  maybe_import_map_resolver: Option<ImportMapResolver>,
-}
-
-impl JsxResolver {
-  pub fn new(
-    jsx_import_source_config: JsxImportSourceConfig,
-    maybe_import_map_resolver: Option<ImportMapResolver>,
-  ) -> Self {
-    Self {
-      default_jsx_import_source: jsx_import_source_config.default_specifier,
-      jsx_import_source_module: jsx_import_source_config.module,
-      maybe_import_map_resolver,
-    }
-  }
-
-  pub fn as_resolver(&self) -> &dyn Resolver {
-    self
-  }
-}
-
-impl Resolver for JsxResolver {
+impl Resolver for CliResolver {
   fn default_jsx_import_source(&self) -> Option<String> {
-    self.default_jsx_import_source.clone()
+    self.maybe_default_jsx_import_source.clone()
   }
 
   fn jsx_import_source_module(&self) -> &str {
-    self.jsx_import_source_module.as_str()
+    self
+      .maybe_jsx_import_source_module
+      .as_deref()
+      .unwrap_or(DEFAULT_JSX_IMPORT_SOURCE_MODULE)
   }
 
   fn resolve(
@@ -76,12 +64,18 @@ impl Resolver for JsxResolver {
     specifier: &str,
     referrer: &ModuleSpecifier,
   ) -> ResolveResponse {
-    self.maybe_import_map_resolver.as_ref().map_or_else(
-      || match resolve_import(specifier, referrer.as_str()) {
+    if let Some(import_map) = &self.maybe_import_map {
+      match import_map.resolve(specifier, referrer) {
+        Ok(resolved_specifier) => {
+          ResolveResponse::Specifier(resolved_specifier)
+        }
+        Err(err) => ResolveResponse::Err(err.into()),
+      }
+    } else {
+      match resolve_import(specifier, referrer.as_str()) {
         Ok(specifier) => ResolveResponse::Specifier(specifier),
         Err(err) => ResolveResponse::Err(err.into()),
-      },
-      |r| r.resolve(specifier, referrer),
-    )
+      }
+    }
   }
 }

cli/standalone.rs

@@ -6,7 +6,7 @@ use crate::file_fetcher::get_source_from_data_url;
 use crate::ops;
 use crate::proc_state::ProcState;
 use crate::version;
-use crate::ImportMapResolver;
+use crate::CliResolver;
 use deno_core::anyhow::anyhow;
 use deno_core::anyhow::Context;
 use deno_core::error::type_error;
@@ -125,7 +125,7 @@ fn u64_from_bytes(arr: &[u8]) -> Result<u64, AnyError> {
 
 struct EmbeddedModuleLoader {
   eszip: eszip::EszipV2,
-  maybe_import_map_resolver: Option<ImportMapResolver>,
+  maybe_import_map_resolver: Option<CliResolver>,
 }
 
 impl ModuleLoader for EmbeddedModuleLoader {
@@ -231,7 +231,7 @@ pub async fn run(
     eszip,
     maybe_import_map_resolver: metadata.maybe_import_map.map(
       |(base, source)| {
-        ImportMapResolver::new(Arc::new(
+        CliResolver::with_import_map(Arc::new(
           parse_from_json(&base, &source).unwrap().import_map,
         ))
       },

cli/tools/vendor/test.rs

@@ -21,7 +21,7 @@ use deno_graph::ModuleKind;
 use import_map::ImportMap;
 
 use crate::cache::ParsedSourceCache;
-use crate::resolver::ImportMapResolver;
+use crate::resolver::CliResolver;
 
 use super::build::VendorEnvironment;
 
@@ -265,14 +265,14 @@ async fn build_test_graph(
   analyzer: &dyn deno_graph::ModuleAnalyzer,
 ) -> ModuleGraph {
   let resolver =
-    original_import_map.map(|m| ImportMapResolver::new(Arc::new(m)));
+    original_import_map.map(|m| CliResolver::with_import_map(Arc::new(m)));
   deno_graph::create_graph(
     roots,
     &mut loader,
     deno_graph::GraphOptions {
       is_dynamic: false,
       imports: None,
-      resolver: resolver.as_ref().map(|im| im.as_resolver()),
+      resolver: resolver.as_ref().map(|r| r.as_graph_resolver()),
       locker: None,
       module_analyzer: Some(analyzer),
       reporter: None,