From 10578286deb00afd326f7529e27413dace3c7f31 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 22 Dec 2023 19:12:34 +0000 Subject: [PATCH 1/4] feat(lsp): cache jsxImportSource automatically --- cli/lsp/language_server.rs | 74 +++++++++++++++++++++++++++--- cli/tests/integration/lsp_tests.rs | 41 +++++++++++++++++ 2 files changed, 108 insertions(+), 7 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index e730e145fedc1e..38881d4e7ed797 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use base64::Engine; use deno_ast::MediaType; use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; @@ -28,8 +29,13 @@ use std::collections::HashMap; use std::collections::HashSet; use std::env; use std::fmt::Write as _; +use std::future::Future; use std::path::PathBuf; +use std::pin::Pin; use std::sync::Arc; +use tokio::sync::mpsc::unbounded_channel; +use tokio::sync::mpsc::UnboundedReceiver; +use tokio::sync::mpsc::UnboundedSender; use tokio_util::sync::CancellationToken; use tower_lsp::jsonrpc::Error as LspError; use tower_lsp::jsonrpc::Result as LspResult; @@ -177,6 +183,12 @@ pub struct StateSnapshot { pub npm: Option, } +type OutsideLockTaskFn = Box< + dyn (FnOnce(LanguageServer) -> Pin>>) + + Send + + Sync, +>; + #[derive(Debug)] pub struct Inner { /// Cached versions of "fixed" assets that can either be inlined in Rust or @@ -217,6 +229,9 @@ pub struct Inner { maybe_testing_server: Option, /// Services used for dealing with npm related functionality. npm: LspNpmServices, + /// This is moved out to its own task after initializing. + outside_lock_task_rx: Option>, + outside_lock_task_tx: UnboundedSender, /// A collection of measurements which instrument that performance of the LSP. performance: Arc, /// A memoized version of fixable diagnostic codes retrieved from TypeScript. @@ -484,6 +499,7 @@ impl Inner { diagnostics_state.clone(), ); let assets = Assets::new(ts_server.clone()); + let (outside_lock_task_tx, outside_lock_task_rx) = unbounded_channel(); Self { assets, @@ -510,6 +526,8 @@ impl Inner { node_resolver: None, resolver: None, }, + outside_lock_task_rx: Some(outside_lock_task_rx), + outside_lock_task_tx, performance, ts_fixable_diagnostics: Default::default(), ts_server, @@ -1023,6 +1041,44 @@ impl Inner { self.lint_options = lint_options; self.fmt_options = fmt_options; self.recreate_http_client_and_dependents().await?; + if let Some(config_file) = self.config.maybe_config_file() { + 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 cache_params = lsp_custom::CacheParams { + referrer: TextDocumentIdentifier { + uri: config_file.specifier.clone(), + }, + uris: vec![TextDocumentIdentifier { + uri: Url::parse(&format!( + "data:application/typescript;base64,{}", + base64::engine::general_purpose::STANDARD.encode( + format!("import '{jsx_import_source}/jsx-runtime';") + ) + )) + .unwrap(), + }], + }; + self + .outside_lock_task_tx + .send(Box::new(|ls: LanguageServer| { + Box::pin(async move { + if let Err(err) = + ls.cache_request(Some(json!(cache_params))).await + { + lsp_warn!("{}", err); + } + }) + })) + .unwrap(); + } + } + } + } + } } Ok(()) @@ -3245,7 +3301,7 @@ impl tower_lsp::LanguageServer for LanguageServer { self.refresh_configuration().await; - { + let mut outside_lock_task_rx = { let mut ls = self.0.write().await; init_log_file(ls.config.log_file()); if let Err(err) = ls.update_tsconfig().await { @@ -3254,9 +3310,15 @@ impl tower_lsp::LanguageServer for LanguageServer { ls.refresh_documents_config().await; ls.diagnostics_server.invalidate_all(); ls.send_diagnostics_update(); - } + ls.outside_lock_task_rx.take().unwrap() + }; - lsp_log!("Server ready."); + let ls = self.clone(); + spawn(async move { + while let Some(task_fn) = outside_lock_task_rx.recv().await { + task_fn(ls.clone()).await; + } + }); if upgrade_check_enabled() { // spawn to avoid lsp send/sync requirement, but also just @@ -3279,6 +3341,8 @@ impl tower_lsp::LanguageServer for LanguageServer { } }); } + + lsp_log!("Server ready."); } async fn shutdown(&self) -> LspResult<()> { @@ -3591,10 +3655,6 @@ impl Inner { let referrer = self .url_map .normalize_url(¶ms.referrer.uri, LspUrlKind::File); - if !self.is_diagnosable(&referrer) { - return Ok(None); - } - let mark = self.performance.mark_with_args("lsp.cache", ¶ms); let roots = if !params.uris.is_empty() { params diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 0e8bbd2afdd82c..cbb5e60ced1967 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -9468,6 +9468,47 @@ export function B() { client.shutdown(); } +#[test] +fn lsp_jsx_import_source_config_file_automatic_cache() { + 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().build(); + client.initialize_default(); + let mut diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.tsx").unwrap(), + "languageId": "typescriptreact", + "version": 1, + "text": " + export function Foo() { + return
; + } + ", + }, + })); + // 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. + if !diagnostics.all().is_empty() { + diagnostics = client.read_diagnostics(); + } + assert_eq!(diagnostics.all(), vec![]); + client.shutdown(); +} + #[derive(Debug, Clone, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] struct TestData { From 59ccbd3535bc504db9d7e0ba7a8e37fd6e0a522c Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 29 Dec 2023 08:56:49 +0000 Subject: [PATCH 2/4] fix test hang --- cli/tests/integration/lsp_tests.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 3eaa949407860b..e1e29b57b3f234 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -9503,8 +9503,22 @@ fn lsp_jsx_import_source_config_file_automatic_cache() { // 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. - if !diagnostics.all().is_empty() { - diagnostics = client.read_diagnostics(); + 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. + diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.tsx").unwrap(), + "languageId": "typescriptreact", + "version": 1, + "text": " + export function Foo() { + return
; + } + ", + }, + })); } assert_eq!(diagnostics.all(), vec![]); client.shutdown(); From 2b2ac75d19c71bf93dcbda73636d9fc203713423 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 29 Dec 2023 09:27:48 +0000 Subject: [PATCH 3/4] LanguageServerTaskQueue struct --- cli/lsp/language_server.rs | 75 ++++++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 19bc1a90833105..32173c95fb63c4 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -183,12 +183,45 @@ pub struct StateSnapshot { pub npm: Option, } -type OutsideLockTaskFn = Box< +type LanguageServerTaskFn = Box< dyn (FnOnce(LanguageServer) -> Pin>>) + Send + Sync, >; +#[derive(Debug)] +struct LanguageServerTaskQueue { + task_tx: UnboundedSender, + /// This is moved out to its own task after initializing. + task_rx: Option>, +} + +impl Default for LanguageServerTaskQueue { + fn default() -> Self { + let (task_tx, task_rx) = unbounded_channel(); + Self { + task_tx, + task_rx: Some(task_rx), + } + } +} + +impl LanguageServerTaskQueue { + fn queue_task(&self, task_fn: LanguageServerTaskFn) -> bool { + self.task_tx.send(task_fn).is_ok() + } + + /// Panics if called more than once. + fn start(&mut self, ls: LanguageServer) { + let mut task_rx = self.task_rx.take().unwrap(); + spawn(async move { + while let Some(task_fn) = task_rx.recv().await { + task_fn(ls.clone()).await; + } + }); + } +} + #[derive(Debug)] pub struct Inner { /// Cached versions of "fixed" assets that can either be inlined in Rust or @@ -208,6 +241,7 @@ pub struct Inner { /// on disk or "open" within the client. pub documents: Documents, http_client: Arc, + task_queue: LanguageServerTaskQueue, /// Handles module registries, which allow discovery of modules module_registries: ModuleRegistry, /// The path to the module registries cache @@ -229,9 +263,6 @@ pub struct Inner { maybe_testing_server: Option, /// Services used for dealing with npm related functionality. npm: LspNpmServices, - /// This is moved out to its own task after initializing. - outside_lock_task_rx: Option>, - outside_lock_task_tx: UnboundedSender, /// A collection of measurements which instrument that performance of the LSP. performance: Arc, /// A memoized version of fixable diagnostic codes retrieved from TypeScript. @@ -499,7 +530,6 @@ impl Inner { diagnostics_state.clone(), ); let assets = Assets::new(ts_server.clone()); - let (outside_lock_task_tx, outside_lock_task_rx) = unbounded_channel(); Self { assets, @@ -516,6 +546,7 @@ impl Inner { maybe_import_map_uri: None, maybe_package_json: None, fmt_options: Default::default(), + task_queue: Default::default(), lint_options: Default::default(), maybe_testing_server: None, module_registries, @@ -526,8 +557,6 @@ impl Inner { node_resolver: None, resolver: None, }, - outside_lock_task_rx: Some(outside_lock_task_rx), - outside_lock_task_tx, performance, ts_fixable_diagnostics: Default::default(), ts_server, @@ -1062,18 +1091,15 @@ impl Inner { .unwrap(), }], }; - self - .outside_lock_task_tx - .send(Box::new(|ls: LanguageServer| { - Box::pin(async move { - if let Err(err) = - ls.cache_request(Some(json!(cache_params))).await - { - lsp_warn!("{}", err); - } - }) - })) - .unwrap(); + self.task_queue.queue_task(Box::new(|ls: LanguageServer| { + Box::pin(async move { + if let Err(err) = + ls.cache_request(Some(json!(cache_params))).await + { + lsp_warn!("{}", err); + } + }) + })); } } } @@ -3304,7 +3330,7 @@ impl tower_lsp::LanguageServer for LanguageServer { self.refresh_configuration().await; - let mut outside_lock_task_rx = { + { let mut ls = self.0.write().await; init_log_file(ls.config.log_file()); if let Err(err) = ls.update_tsconfig().await { @@ -3313,16 +3339,9 @@ impl tower_lsp::LanguageServer for LanguageServer { ls.refresh_documents_config().await; ls.diagnostics_server.invalidate_all(); ls.send_diagnostics_update(); - ls.outside_lock_task_rx.take().unwrap() + ls.task_queue.start(self.clone()); }; - let ls = self.clone(); - spawn(async move { - while let Some(task_fn) = outside_lock_task_rx.recv().await { - task_fn(ls.clone()).await; - } - }); - if upgrade_check_enabled() { // spawn to avoid lsp send/sync requirement, but also just // to ensure this initialized method returns quickly From a61e5a5b0f00079af2cdfbb23a6ef48074496582 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 2 Jan 2024 05:51:11 +0000 Subject: [PATCH 4/4] spawn for individual tasks, add comment --- cli/lsp/language_server.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 812147a84e6716..754ccd680d1369 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -29,9 +29,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::env; use std::fmt::Write as _; -use std::future::Future; use std::path::PathBuf; -use std::pin::Pin; use std::sync::Arc; use tokio::sync::mpsc::unbounded_channel; use tokio::sync::mpsc::UnboundedReceiver; @@ -183,12 +181,11 @@ pub struct StateSnapshot { pub npm: Option, } -type LanguageServerTaskFn = Box< - dyn (FnOnce(LanguageServer) -> Pin>>) - + Send - + Sync, ->; +type LanguageServerTaskFn = Box; +/// Used to queue tasks from inside of the language server lock that must be +/// commenced from outside of it. For example, queue a request to cache a module +/// after having loaded a config file which references it. #[derive(Debug)] struct LanguageServerTaskQueue { task_tx: UnboundedSender, @@ -216,7 +213,7 @@ impl LanguageServerTaskQueue { let mut task_rx = self.task_rx.take().unwrap(); spawn(async move { while let Some(task_fn) = task_rx.recv().await { - task_fn(ls.clone()).await; + task_fn(ls.clone()); } }); } @@ -1092,13 +1089,13 @@ impl Inner { }], }; self.task_queue.queue_task(Box::new(|ls: LanguageServer| { - Box::pin(async move { + spawn(async move { if let Err(err) = ls.cache_request(Some(json!(cache_params))).await { lsp_warn!("{}", err); } - }) + }); })); } }