fix(lsp): improved cjs tracking (#23374)

Our cjs tracking was a bit broken. It was marking stuff as esm that was
actually cjs leading to type checking errors.

4 files changed, 155 insertions, 86 deletions

diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs
index 1dd18f599..e7ef048cf 100644
--- a/cli/lsp/documents.rs
+++ b/cli/lsp/documents.rs
@@ -12,6 +12,7 @@ use super::tsc::AssetDocument;
 use crate::args::package_json;
 use crate::cache::HttpCache;
 use crate::jsr::JsrCacheResolver;
+use crate::lsp::logging::lsp_warn;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
@@ -43,7 +44,6 @@ use deno_semver::jsr::JsrPackageReqReference;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
 use indexmap::IndexMap;
-use once_cell::sync::Lazy;
 use package_json::PackageJsonDepsProvider;
 use std::borrow::Cow;
 use std::collections::BTreeSet;
@@ -57,36 +57,6 @@ use std::sync::atomic::Ordering;
 use std::sync::Arc;
 use tower_lsp::lsp_types as lsp;
 
-static JS_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([(
-    "content-type".to_string(),
-    "application/javascript".to_string(),
-  )])
-  .into_iter()
-  .collect()
-});
-
-static JSX_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([("content-type".to_string(), "text/jsx".to_string())])
-    .into_iter()
-    .collect()
-});
-
-static TS_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([(
-    "content-type".to_string(),
-    "application/typescript".to_string(),
-  )])
-  .into_iter()
-  .collect()
-});
-
-static TSX_HEADERS: Lazy<HashMap<String, String>> = Lazy::new(|| {
-  ([("content-type".to_string(), "text/tsx".to_string())])
-    .into_iter()
-    .collect()
-});
-
 pub const DOCUMENT_SCHEMES: [&str; 5] =
   ["data", "blob", "file", "http", "https"];
 
@@ -103,18 +73,6 @@ pub enum LanguageId {
 }
 
 impl LanguageId {
-  pub fn as_media_type(&self) -> MediaType {
-    match self {
-      LanguageId::JavaScript => MediaType::JavaScript,
-      LanguageId::Jsx => MediaType::Jsx,
-      LanguageId::TypeScript => MediaType::TypeScript,
-      LanguageId::Tsx => MediaType::Tsx,
-      LanguageId::Json => MediaType::Json,
-      LanguageId::JsonC => MediaType::Json,
-      LanguageId::Markdown | LanguageId::Unknown => MediaType::Unknown,
-    }
-  }
-
   pub fn as_extension(&self) -> Option<&'static str> {
     match self {
       LanguageId::JavaScript => Some("js"),
@@ -128,13 +86,15 @@ impl LanguageId {
     }
   }
 
-  fn as_headers(&self) -> Option<&HashMap<String, String>> {
+  pub fn as_content_type(&self) -> Option<&'static str> {
     match self {
-      Self::JavaScript => Some(&JS_HEADERS),
-      Self::Jsx => Some(&JSX_HEADERS),
-      Self::TypeScript => Some(&TS_HEADERS),
-      Self::Tsx => Some(&TSX_HEADERS),
-      _ => None,
+      LanguageId::JavaScript => Some("application/javascript"),
+      LanguageId::Jsx => Some("text/jsx"),
+      LanguageId::TypeScript => Some("application/typescript"),
+      LanguageId::Tsx => Some("text/tsx"),
+      LanguageId::Json | LanguageId::JsonC => Some("application/json"),
+      LanguageId::Markdown => Some("text/markdown"),
+      LanguageId::Unknown => None,
     }
   }
 
@@ -291,6 +251,7 @@ pub struct Document {
   // so having a mutex to hold it is ok
   maybe_navigation_tree: Mutex<Option<Arc<tsc::NavigationTree>>>,
   maybe_parsed_source: Option<ParsedSourceResult>,
+  media_type: MediaType,
   specifier: ModuleSpecifier,
   text_info: SourceTextInfo,
 }
@@ -302,15 +263,23 @@ impl Document {
     maybe_headers: Option<HashMap<String, String>>,
     text_info: SourceTextInfo,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Arc<Self> {
     // we only ever do `Document::new` on disk resources that are supposed to
     // be diagnosable, unlike `Document::open`, so it is safe to unconditionally
     // parse the module.
+    let media_type = resolve_media_type(
+      &specifier,
+      maybe_headers.as_ref(),
+      None,
+      maybe_node_resolver,
+    );
     let (maybe_parsed_source, maybe_module) = parse_and_analyze_module(
       &specifier,
       text_info.clone(),
       maybe_headers.as_ref(),
+      media_type,
       resolver,
       npm_resolver,
     );
@@ -328,6 +297,7 @@ impl Document {
       maybe_navigation_tree: Mutex::new(None),
       maybe_parsed_source: maybe_parsed_source
         .filter(|_| specifier.scheme() == "file"),
+      media_type,
       text_info,
       specifier,
     })
@@ -336,12 +306,27 @@ impl Document {
   fn maybe_with_new_resolver(
     &self,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Self>> {
-    let parsed_source_result = match &self.maybe_parsed_source {
+    let mut parsed_source_result = match &self.maybe_parsed_source {
       Some(parsed_source_result) => parsed_source_result.clone(),
       None => return None, // nothing to change
     };
+    let media_type = resolve_media_type(
+      &self.specifier,
+      self.maybe_headers.as_ref(),
+      self.maybe_language_id,
+      maybe_node_resolver,
+    );
+    // reparse if the media type has changed
+    if let Ok(parsed_source) = &parsed_source_result {
+      if parsed_source.media_type() != media_type {
+        parsed_source_result =
+          parse_source(&self.specifier, self.text_info.clone(), media_type);
+      }
+    }
+
     let maybe_module = Some(analyze_module(
       &self.specifier,
       &parsed_source_result,
@@ -363,27 +348,37 @@ impl Document {
       maybe_headers: self.maybe_headers.clone(),
       maybe_language_id: self.maybe_language_id,
       maybe_lsp_version: self.maybe_lsp_version,
+      media_type,
       text_info: self.text_info.clone(),
       specifier: self.specifier.clone(),
     }))
   }
 
+  #[allow(clippy::too_many_arguments)]
   fn open(
     specifier: ModuleSpecifier,
     version: i32,
     language_id: LanguageId,
+    maybe_headers: Option<HashMap<String, String>>,
     content: Arc<str>,
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Arc<Self> {
-    let maybe_headers = language_id.as_headers();
     let text_info = SourceTextInfo::new(content);
+    let media_type = resolve_media_type(
+      &specifier,
+      None,
+      Some(language_id),
+      maybe_node_resolver,
+    );
     let (maybe_parsed_source, maybe_module) = if language_id.is_diagnosable() {
       parse_and_analyze_module(
         &specifier,
         text_info.clone(),
-        maybe_headers,
+        maybe_headers.as_ref(),
+        media_type,
         resolver,
         npm_resolver,
       )
@@ -400,11 +395,12 @@ impl Document {
       line_index,
       maybe_language_id: Some(language_id),
       maybe_lsp_version: Some(version),
-      maybe_headers: maybe_headers.map(ToOwned::to_owned),
+      maybe_headers,
       maybe_module,
       maybe_navigation_tree: Mutex::new(None),
       maybe_parsed_source: maybe_parsed_source
         .filter(|_| specifier.scheme() == "file"),
+      media_type,
       text_info,
       specifier,
     })
@@ -434,20 +430,18 @@ impl Document {
       }
     }
     let text_info = SourceTextInfo::from_string(content);
+    let media_type = self.media_type;
     let (maybe_parsed_source, maybe_module) = if self
       .maybe_language_id
       .as_ref()
       .map(|li| li.is_diagnosable())
       .unwrap_or(false)
     {
-      let maybe_headers = self
-        .maybe_language_id
-        .as_ref()
-        .and_then(|li| li.as_headers());
       parse_and_analyze_module(
         &self.specifier,
         text_info.clone(),
-        maybe_headers,
+        self.maybe_headers.as_ref(),
+        media_type,
         resolver,
         npm_resolver,
       )
@@ -477,6 +471,7 @@ impl Document {
         .filter(|_| self.specifier.scheme() == "file"),
       maybe_lsp_version: Some(version),
       maybe_navigation_tree: Mutex::new(None),
+      media_type,
     }))
   }
 
@@ -494,6 +489,7 @@ impl Document {
       maybe_parsed_source: self.maybe_parsed_source.clone(),
       maybe_lsp_version: self.maybe_lsp_version,
       maybe_navigation_tree: Mutex::new(None),
+      media_type: self.media_type,
     })
   }
 
@@ -554,18 +550,7 @@ impl Document {
   }
 
   pub fn media_type(&self) -> MediaType {
-    if let Some(Ok(module)) = &self.maybe_module {
-      return module.media_type;
-    }
-    let specifier_media_type = MediaType::from_specifier(&self.specifier);
-    if specifier_media_type != MediaType::Unknown {
-      return specifier_media_type;
-    }
-
-    self
-      .maybe_language_id
-      .map(|id| id.as_media_type())
-      .unwrap_or(MediaType::Unknown)
+    self.media_type
   }
 
   pub fn maybe_language_id(&self) -> Option<LanguageId> {
@@ -629,6 +614,39 @@ impl Document {
   }
 }
 
+fn resolve_media_type(
+  specifier: &ModuleSpecifier,
+  maybe_headers: Option<&HashMap<String, String>>,
+  maybe_language_id: Option<LanguageId>,
+  maybe_node_resolver: Option<&CliNodeResolver>,
+) -> MediaType {
+  if let Some(node_resolver) = maybe_node_resolver {
+    if node_resolver.in_npm_package(specifier) {
+      match node_resolver.url_to_node_resolution(specifier.clone()) {
+        Ok(resolution) => {
+          let (_, media_type) =
+            NodeResolution::into_specifier_and_media_type(Some(resolution));
+          return media_type;
+        }
+        Err(err) => {
+          lsp_warn!("Node resolution failed for '{}': {}", specifier, err);
+        }
+      }
+    }
+  }
+
+  if maybe_headers.is_some() {
+    return MediaType::from_specifier_and_headers(specifier, maybe_headers);
+  }
+
+  // LanguageId is a subset of MediaType, so get its content type and
+  // also use the specifier to inform its media type
+  MediaType::from_specifier_and_content_type(
+    specifier,
+    maybe_language_id.and_then(|id| id.as_content_type()),
+  )
+}
+
 pub fn to_lsp_range(range: &deno_graph::Range) -> lsp::Range {
   lsp::Range {
     start: lsp::Position {
@@ -712,6 +730,7 @@ impl FileSystemDocuments {
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Document>> {
     let fs_version = if specifier.scheme() == "data" {
@@ -724,7 +743,13 @@ impl FileSystemDocuments {
       != fs_version
     {
       // attempt to update the file on the file system
-      self.refresh_document(cache, resolver, specifier, npm_resolver)
+      self.refresh_document(
+        cache,
+        resolver,
+        specifier,
+        maybe_node_resolver,
+        npm_resolver,
+      )
     } else {
       file_system_doc
     }
@@ -737,6 +762,7 @@ impl FileSystemDocuments {
     cache: &Arc<dyn HttpCache>,
     resolver: &dyn deno_graph::source::Resolver,
     specifier: &ModuleSpecifier,
+    maybe_node_resolver: Option<&CliNodeResolver>,
     npm_resolver: &dyn deno_graph::source::NpmResolver,
   ) -> Option<Arc<Document>> {
     let doc = if specifier.scheme() == "file" {
@@ -751,6 +777,7 @@ impl FileSystemDocuments {
         None,
         SourceTextInfo::from_string(content),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     } else if specifier.scheme() == "data" {
@@ -764,6 +791,7 @@ impl FileSystemDocuments {
         None,
         SourceTextInfo::from_string(source),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     } else {
@@ -791,6 +819,7 @@ impl FileSystemDocuments {
         maybe_headers,
         SourceTextInfo::from_string(content),
         resolver,
+        maybe_node_resolver,
         npm_resolver,
       )
     };
@@ -837,6 +866,8 @@ 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<IndexMap<ModuleSpecifier, GraphImport>>,
+  /// Resolver for node_modules.
+  maybe_node_resolver: Option<Arc<CliNodeResolver>>,
   /// A resolver that takes into account currently loaded import map and JSX
   /// settings.
   resolver: Arc<CliGraphResolver>,
@@ -861,6 +892,7 @@ impl Documents {
       open_docs: HashMap::default(),
       file_system_docs: Default::default(),
       imports: Default::default(),
+      maybe_node_resolver: None,
       resolver: Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
         node_resolver: None,
         npm_resolver: None,
@@ -905,9 +937,14 @@ impl Documents {
       specifier.clone(),
       version,
       language_id,
+      // todo(dsherret): don't we want to pass in the headers from
+      // the cache for remote modules here in order to get the
+      // x-typescript-types?
+      None,
       content,
       &self.cache,
       resolver,
+      self.maybe_node_resolver.as_deref(),
       npm_resolver,
     );
 
@@ -1102,6 +1139,7 @@ impl Documents {
         &self.cache,
         self.get_resolver(),
         &specifier,
+        self.maybe_node_resolver.as_deref(),
         self.get_npm_resolver(),
       )
     }
@@ -1281,6 +1319,7 @@ impl Documents {
   ) {
     let config_data = config.tree.root_data();
     let config_file = config_data.and_then(|d| d.config_file.as_deref());
+    self.maybe_node_resolver = node_resolver.clone();
     self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
       node_resolver,
       npm_resolver,
@@ -1352,9 +1391,11 @@ impl Documents {
         if !config.specifier_enabled(doc.specifier()) {
           continue;
         }
-        if let Some(new_doc) =
-          doc.maybe_with_new_resolver(resolver, npm_resolver)
-        {
+        if let Some(new_doc) = doc.maybe_with_new_resolver(
+          resolver,
+          self.maybe_node_resolver.as_deref(),
+          npm_resolver,
+        ) {
           *doc = new_doc;
         }
       }
@@ -1362,9 +1403,11 @@ impl Documents {
         if !config.specifier_enabled(doc.specifier()) {
           continue;
         }
-        if let Some(new_doc) =
-          doc.maybe_with_new_resolver(resolver, npm_resolver)
-        {
+        if let Some(new_doc) = doc.maybe_with_new_resolver(
+          resolver,
+          self.maybe_node_resolver.as_deref(),
+          npm_resolver,
+        ) {
           *doc.value_mut() = new_doc;
         }
       }
@@ -1385,6 +1428,7 @@ impl Documents {
             &self.cache,
             resolver,
             specifier,
+            self.maybe_node_resolver.as_deref(),
             npm_resolver,
           );
         }
@@ -1628,10 +1672,11 @@ fn parse_and_analyze_module(
   specifier: &ModuleSpecifier,
   text_info: SourceTextInfo,
   maybe_headers: Option<&HashMap<String, String>>,
+  media_type: MediaType,
   resolver: &dyn deno_graph::source::Resolver,
   npm_resolver: &dyn deno_graph::source::NpmResolver,
 ) -> (Option<ParsedSourceResult>, Option<ModuleResult>) {
-  let parsed_source_result = parse_source(specifier, text_info, maybe_headers);
+  let parsed_source_result = parse_source(specifier, text_info, media_type);
   let module_result = analyze_module(
     specifier,
     &parsed_source_result,
@@ -1645,12 +1690,12 @@ fn parse_and_analyze_module(
 fn parse_source(
   specifier: &ModuleSpecifier,
   text_info: SourceTextInfo,
-  maybe_headers: Option<&HashMap<String, String>>,
+  media_type: MediaType,
 ) -> ParsedSourceResult {
   deno_ast::parse_module(deno_ast::ParseParams {
     specifier: specifier.clone(),
     text_info,
-    media_type: MediaType::from_specifier_and_headers(specifier, maybe_headers),
+    media_type,
     capture_tokens: true,
     scope_analysis: true,
     maybe_syntax: None,
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 2e0726a79..13da932f7 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -3996,6 +3996,7 @@ struct LoadResponse {
   data: Arc<str>,
   script_kind: i32,
   version: Option<String>,
+  is_cjs: bool,
 }
 
 #[op2]
@@ -4018,6 +4019,10 @@ fn op_load<'s>(
         data: doc.text(),
         script_kind: crate::tsc::as_ts_script_kind(doc.media_type()),
         version: state.script_version(&specifier),
+        is_cjs: matches!(
+          doc.media_type(),
+          MediaType::Cjs | MediaType::Cts | MediaType::Dcts
+        ),
       })
     };
 
diff --git a/cli/resolver.rs b/cli/resolver.rs
index fc326e1b1..d5a85e001 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -223,6 +223,13 @@ impl CliNodeResolver {
     Ok(specifier)
   }
 
+  pub fn url_to_node_resolution(
+    &self,
+    specifier: ModuleSpecifier,
+  ) -> Result<NodeResolution, AnyError> {
+    self.node_resolver.url_to_node_resolution(specifier)
+  }
+
   fn handle_node_resolve_result(
     &self,
     result: Result<Option<NodeResolution>, AnyError>,
diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
index be9962814..b76c95aa5 100644
--- a/cli/tsc/99_main_compiler.js
+++ b/cli/tsc/99_main_compiler.js
@@ -135,12 +135,16 @@ delete Object.prototype.__proto__;
     #cache = new Set();
 
     /** @param {[string, ts.Extension]} param */
-    add([specifier, ext]) {
+    maybeAdd([specifier, ext]) {
       if (ext === ".cjs" || ext === ".d.cts" || ext === ".cts") {
         this.#cache.add(specifier);
       }
     }
 
+    add(specifier) {
+      this.#cache.add(specifier);
+    }
+
     /** @param specifier {string} */
     has(specifier) {
       return this.#cache.has(specifier);
@@ -603,22 +607,30 @@ delete Object.prototype.__proto__;
         return sourceFile;
       }
 
-      /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; }} */
+      /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; isCjs: boolean }} */
       const fileInfo = ops.op_load(specifier);
       if (!fileInfo) {
         return undefined;
       }
-      const { data, scriptKind, version } = fileInfo;
+      let { data, scriptKind, version, isCjs } = fileInfo;
       assert(
         data != null,
         `"data" is unexpectedly null for "${specifier}".`,
       );
+
+      // use the cache for non-lsp
+      if (isCjs == null) {
+        isCjs = isCjsCache.has(specifier);
+      } else if (isCjs) {
+        isCjsCache.add(specifier);
+      }
+
       sourceFile = ts.createSourceFile(
         specifier,
         data,
         {
           ...getCreateSourceFileOptions(languageVersion),
-          impliedNodeFormat: isCjsCache.has(specifier)
+          impliedNodeFormat: isCjs
             ? ts.ModuleKind.CommonJS
             : ts.ModuleKind.ESNext,
           // no need to parse docs for `deno check`
@@ -688,7 +700,7 @@ delete Object.prototype.__proto__;
             base: containingFilePath,
           })?.[0];
           if (resolved) {
-            isCjsCache.add(resolved);
+            isCjsCache.maybeAdd(resolved);
             return {
               primary: true,
               resolvedFileName: resolved[0],
@@ -723,7 +735,7 @@ delete Object.prototype.__proto__;
       if (resolved) {
         const result = resolved.map((item) => {
           if (item) {
-            isCjsCache.add(item);
+            isCjsCache.maybeAdd(item);
             const [resolvedFileName, extension] = item;
             return {
               resolvedFileName,