Repository: denoland/deno
Pull request: perf(lsp): drop jsx import source cache

Both initialization and some other operations call `dispatch_cache_jsx_import_source`, which in turn calls cache, which in turn triggers `post_cache` and invalidates all the work performed in `initialization`/`did_change_workspace_folders`.

Based on the comment in https://github.com/denoland/deno/pull/32476, we agreed to just drop it.

- Closes #21770

Changed files:
filename: cli/lsp/language_server.rs
patch: @@ -44,8 +44,6 @@ use deno_runtime::deno_tls::rustls::RootCertStore;
 use deno_semver::jsr::JsrPackageReqReference;
 use indexmap::IndexSet;
 use log::error;
-use node_resolver::NodeResolutionKind;
-use node_resolver::ResolutionMode;
 use serde::Deserialize;
 use serde_json::from_value;
 use tokio::sync::OnceCell;
@@ -1195,42 +1193,6 @@ impl Inner {
     ));
   }
 
-  #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
-  fn dispatch_cache_jsx_import_sources(&self) {
-    for specifier_config in self
-      .compiler_options_resolver
-      .entries()
-      .filter_map(|(_, d)| d.jsx_import_source_config.as_ref())
-      .flat_map(|c| c.import_source.iter().chain(c.import_source_types.iter()))
-    {
-      let referrer = specifier_config.base.clone();
-      let specifier = format!("{}/jsx-runtime", &specifier_config.specifier);
-      self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
-        spawn(async move {
-          let specifier = {
-            let inner = ls.inner.read().await;
-            let scoped_resolver =
-              inner.resolver.get_scoped_resolver(Some(&referrer));
-            let resolver = scoped_resolver.as_cli_resolver();
-            let Ok(specifier) = resolver.resolve(
-              &specifier,
-              &referrer,
-              deno_graph::Position::zeroed(),
-              ResolutionMode::Import,
-              NodeResolutionKind::Types,
-            ) else {
-              return;
-            };
-            specifier
-          };
-          if let Err(err) = ls.cache(vec![specifier], referrer, false).await {
-            lsp_warn!("{:#}", err);
-          }
-        });
-      }));
-    }
-  }
-
   #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
   async fn refresh_resolver(&mut self) {
     self.resolver = Arc::new(
@@ -1609,7 +1571,6 @@ impl Inner {
     self.update_cache();
     self.refresh_resolver().await;
     self.refresh_compiler_options_resolver();
-    self.dispatch_cache_jsx_import_sources();
     self.refresh_linter_resolver();
     self.refresh_documents_config();
     self.send_diagnostics_update();
@@ -1674,11 +1635,6 @@ impl Inner {
       self.update_cache();
       self.refresh_resolver().await;
       self.refresh_compiler_options_resolver();
-      // Don't cache anything if only a lockfile has changed, or it can
-      // retrigger this notification and cause an infinite loop.
-      if changed_deno_json {
-        self.dispatch_cache_jsx_import_sources();
-      }
       self.refresh_linter_resolver();
       self.refresh_documents_config();
       self.project_changed(
@@ -3994,7 +3950,6 @@ impl Inner {
     self.update_cache();
     self.refresh_resolver().await;
     self.refresh_compiler_options_resolver();
-    self.dispatch_cache_jsx_import_sources();
     self.refresh_linter_resolver();
     self.refresh_documents_config();
 
@@ -4231,7 +4186,6 @@ impl Inner {
     self.refresh_config_tree().await;
     self.refresh_resolver().await;
     self.refresh_compiler_options_resolver();
-    self.dispatch_cache_jsx_import_sources();
     self.refresh_linter_resolver();
     self.refresh_documents_config();
     self.send_diagnostics_update();

filename: tests/integration/lsp_tests.rs
patch: @@ -14275,146 +14275,6 @@ export function B() {
   client.shutdown();
 }
 
-#[ignore = "https://github.com/denoland/deno/issues/21770"]
-#[test(timeout = 300, fork_with_suffix = "_tsgo")]
-fn lsp_jsx_import_source_config_file_automatic_cache(use_tsgo: bool) {
-  let context = TestContextBuilder::new()
-    .use_http_server()
-    .use_temp_cwd()
-    .build();
-  let temp_dir = context.temp_dir();
-  temp_dir.write(
-    "deno.json",
-    json!({
-      "compilerOptions": {
-        "jsx": "react-jsx",
-        "jsxImportSource": "http://localhost:4545/jsx",
-      },
-    })
-    .to_string(),
-  );
-  let mut client = context.new_lsp_command().set_use_tsgo(use_tsgo).build();
-  client.initialize_default();
-  let mut diagnostics = client.did_open(json!({
-    "textDocument": {
-      "uri": url_to_uri(&temp_dir.url().join("file.tsx").unwrap()).unwrap(),
-      "languageId": "typescriptreact",
-      "version": 1,
-      "text": "
-        export function Foo() {
-          return <div></div>;
-        }
-      ",
-    },
-  }));
-  // The caching is done on an asynchronous task spawned after init, so there's
-  // a chance it wasn't done in time and we need to wait for another batch of
-  // diagnostics.
-  let mut version = 1;
-  while !diagnostics.all().is_empty() {
-    std::thread::sleep(std::time::Duration::from_millis(50));
-    // The post-cache diagnostics update triggers inconsistently on CI for some
-    // reason. Force it with this notification.
-    version += 1;
-    client.write_notification(
-      "textDocument/didChange",
-      json!({
-        "textDocument": {
-          "uri": url_to_uri(&temp_dir.url().join("file.tsx").unwrap()).unwrap(),
-          "version": version,
-        },
-        "contentChanges": [
-          {
-            "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 },
-            },
-            "text": "",
-          },
-        ],
-      }),
-    );
-    diagnostics = client.read_diagnostics();
-  }
-  assert_eq!(diagnostics.all(), vec![]);
-  client.shutdown();
-}
-
-#[ignore = "https://github.com/denoland/deno/issues/21770"]
-#[test(timeout = 300, fork_with_suffix = "_tsgo")]
-fn lsp_jsx_import_source_package_json_automatic_cache(use_tsgo: bool) {
-  let context = TestContextBuilder::new()
-    .use_http_server()
-    .use_temp_cwd()
-    .build();
-  let temp_dir = context.temp_dir();
-  temp_dir.write(
-    "deno.json",
-    json!({
-      "compilerOptions": {
-        "jsx": "react-jsx",
-        "jsxImportSource": "preact",
-      },
-      "nodeModulesDir": false,
-    })
-    .to_string(),
-  );
-  temp_dir.write(
-    "package.json",
-    json!({
-      "dependencies": {
-        "preact": "^10.19.6",
-      },
-    })
-    .to_string(),
-  );
-  let mut client = context.new_lsp_command().set_use_tsgo(use_tsgo).build();
-  client.initialize_default();
-  let mut diagnostics = client.did_open(json!({
-    "textDocument": {
-      "uri": url_to_uri(&temp_dir.url().join("file.tsx").unwrap()).unwrap(),
-      "languageId": "typescriptreact",
-      "version": 1,
-      "text": "
-        export function Foo() {
-          return <div></div>;
-        }
-      ",
-    },
-  }));
-  // The caching is done on an asynchronous task spawned after init, so there's
-  // a chance it wasn't done in time and we need to wait for another batch of
-  // diagnostics.
-  let mut version = 1;
-  while !diagnostics.all().is_empty() {
-    std::thread::sleep(std::time::Duration::from_millis(50));
-    // The post-cache diagnostics update triggers inconsistently on CI for some
-    // reason. Force it with this notification.
-    version += 1;
-    client.write_notification(
-      "textDocument/didChange",
-      json!({
-        "textDocument": {
-          "uri": url_to_uri(&temp_dir.url().join("file.tsx").unwrap()).unwrap(),
-          "version": version,
-        },
-        "contentChanges": [
-          {
-            "range": {
-              "start": { "line": 0, "character": 0 },
-              "end": { "line": 0, "character": 0 },
-            },
-            "text": "",
-          },
-        ],
-      }),
-    );
-    diagnostics = client.read_diagnostics();
-  }
-  assert_eq!(json!(diagnostics.all()), json!([]));
-  client.shutdown();
-}
-
 #[test(timeout = 300, fork_with_suffix = "_tsgo")]
 fn lsp_jsx_import_source_byonm_preact(use_tsgo: bool) {
   let context = TestContextBuilder::new()