Repository: denoland/deno
Pull request: fix(lsp): resolve jsx import source with types mode

Blocked on https://github.com/denoland/deno_graph/pull/513.

Fixes jsx import sources with alternate resolutions for types. Also fixes panic from trying to cache npm specifiers as roots under byonm.

Changed files:
filename: Cargo.lock
patch: @@ -1586,9 +1586,9 @@ dependencies = [
 
 [[package]]
 name = "deno_graph"
-version = "0.81.2"
+version = "0.81.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "beccc9d4007be8b85299f696a412f38912fc304cf9519092b47531319ef8a92f"
+checksum = "450d75c29d99fd7325dd19a1ed7c3afb18ec04d1f4a4762350a29cbe041647c3"
 dependencies = [
  "anyhow",
  "async-trait",

filename: cli/Cargo.toml
patch: @@ -69,7 +69,7 @@ deno_config = { version = "=0.30.1", features = ["workspace", "sync"] }
 deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
 deno_doc = { version = "0.146.0", features = ["html", "syntect"] }
 deno_emit = "=0.44.0"
-deno_graph = { version = "=0.81.2" }
+deno_graph = { version = "=0.81.3" }
 deno_lint = { version = "=0.63.1", features = ["docs"] }
 deno_lockfile.workspace = true
 deno_npm = "=0.23.1"

filename: cli/lsp/language_server.rs
patch: @@ -956,31 +956,27 @@ impl Inner {
       .refresh(&self.config.settings, &self.workspace_files, &file_fetcher)
       .await;
     for config_file in self.config.tree.config_files() {
-      if let Ok((compiler_options, _)) = config_file.to_compiler_options() {
-        if let Some(compiler_options_obj) = compiler_options.as_object() {
-          if let Some(jsx_import_source) =
-            compiler_options_obj.get("jsxImportSource")
-          {
-            if let Some(jsx_import_source) = jsx_import_source.as_str() {
-              let specifiers = vec![Url::parse(&format!(
-                "data:application/typescript;base64,{}",
-                base64::engine::general_purpose::STANDARD
-                  .encode(format!("import '{jsx_import_source}/jsx-runtime';"))
-              ))
-              .unwrap()];
-              let referrer = config_file.specifier.clone();
-              self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
-                spawn(async move {
-                  if let Err(err) = ls.cache(specifiers, referrer, false).await
-                  {
-                    lsp_warn!("{:#}", err);
-                  }
-                });
-              }));
+      (|| {
+        let compiler_options = config_file.to_compiler_options().ok()?.0;
+        let compiler_options_obj = compiler_options.as_object()?;
+        let jsx_import_source = compiler_options_obj.get("jsxImportSource")?;
+        let jsx_import_source = jsx_import_source.as_str()?;
+        let referrer = config_file.specifier.clone();
+        let specifier = Url::parse(&format!(
+          "data:application/typescript;base64,{}",
+          base64::engine::general_purpose::STANDARD
+            .encode(format!("import '{jsx_import_source}/jsx-runtime';"))
+        ))
+        .unwrap();
+        self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
+          spawn(async move {
+            if let Err(err) = ls.cache(vec![specifier], referrer, false).await {
+              lsp_warn!("{:#}", err);
             }
-          }
-        }
-      }
+          });
+        }));
+        Some(())
+      })();
     }
   }
 
@@ -3533,19 +3529,22 @@ impl Inner {
     force_global_cache: bool,
   ) -> Result<PrepareCacheResult, AnyError> {
     let config_data = self.config.tree.data_for_specifier(&referrer);
+    let byonm = config_data.map(|d| d.byonm).unwrap_or(false);
     let mut roots = if !specifiers.is_empty() {
       specifiers
     } else {
       vec![referrer.clone()]
     };
 
-    // always include the npm packages since resolution of one npm package
-    // might affect the resolution of other npm packages
-    if let Some(npm_reqs) = self
+    if byonm {
+      roots.retain(|s| s.scheme() != "npm");
+    } else if let Some(npm_reqs) = self
       .documents
       .npm_reqs_by_scope()
       .get(&config_data.map(|d| d.scope.as_ref().clone()))
     {
+      // always include the npm packages since resolution of one npm package
+      // might affect the resolution of other npm packages
       roots.extend(
         npm_reqs
           .iter()

filename: tests/integration/lsp_tests.rs
patch: @@ -11724,6 +11724,66 @@ fn lsp_jsx_import_source_config_file_automatic_cache() {
   client.shutdown();
 }
 
+#[test]
+fn lsp_jsx_import_source_byonm_preact() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .add_npm_env_vars()
+    .build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write(
+    "deno.json",
+    json!({
+      "compilerOptions": {
+        "jsx": "react-jsx",
+        "jsxImportSource": "npm:preact@^10.19.6"
+      },
+      "unstable": ["byonm"],
+    })
+    .to_string(),
+  );
+  temp_dir.write(
+    "package.json",
+    json!({
+      "dependencies": {
+        "preact": "^10.19.6",
+      },
+    })
+    .to_string(),
+  );
+  let file = source_file(temp_dir.path().join("file.tsx"), r#"<div></div>;"#);
+  context.run_npm("install");
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let diagnostics = client.did_open_file(&file);
+  assert_eq!(json!(diagnostics.all()), json!([]));
+  let res = client.write_request(
+    "textDocument/hover",
+    json!({
+      "textDocument": { "uri": file.uri() },
+      "position": { "line": 0, "character": 1 },
+    }),
+  );
+  assert_eq!(
+    res,
+    json!({
+      "contents": [
+        {
+          "language": "typescript",
+          "value": "(property) JSXInternal.IntrinsicElements.div: JSXInternal.HTMLAttributes<HTMLDivElement>",
+        },
+        "",
+      ],
+      "range": {
+        "start": { "line": 0, "character": 1 },
+        "end": { "line": 0, "character": 4 },
+      },
+    }),
+  );
+  client.shutdown();
+}
+
 #[test]
 fn lsp_jsx_import_source_types_pragma() {
   let context = TestContextBuilder::new()

filename: tests/util/server/src/lsp.rs
patch: @@ -1301,6 +1301,8 @@ impl SourceFile {
     let lang = match path.as_path().extension().unwrap().to_str().unwrap() {
       "js" => "javascript",
       "ts" | "d.ts" => "typescript",
+      "jsx" => "javascriptreact",
+      "tsx" => "typescriptreact",
       "json" => "json",
       "md" => "markdown",
       other => panic!("unsupported file extension: {other}"),