From a6663b0ce3bced56fbd9138e02ef473669d6c114 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 15 Sep 2025 21:01:05 +0100 Subject: [PATCH] fix(lsp): don't write lockfile during cache-on-save (#30733) --- cli/lsp/config.rs | 8 +++---- cli/lsp/language_server.rs | 38 +++++++++++++++++++++++++++------- tests/integration/lsp_tests.rs | 26 +++++++---------------- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index e5569b03a7..7989d30c4e 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -555,7 +555,7 @@ pub struct WorkspaceSettings { /// Cache local modules and their dependencies on `textDocument/didSave` /// notifications corresponding to them. - #[serde(default)] + #[serde(default = "default_to_true")] pub cache_on_save: bool, /// Override the default stores used to validate certificates. This overrides @@ -632,7 +632,7 @@ impl Default for WorkspaceSettings { disable_paths: vec![], enable_paths: None, cache: None, - cache_on_save: false, + cache_on_save: true, certificate_stores: None, config: None, import_map: None, @@ -1433,7 +1433,7 @@ impl ConfigData { is_package_manager_subcommand: false, frozen_lockfile: None, lock_arg: None, - lockfile_skip_write: false, + lockfile_skip_write: true, node_modules_dir: Some(resolve_node_modules_dir_mode( &member_dir.workspace, byonm, @@ -2146,7 +2146,7 @@ mod tests { disable_paths: vec![], enable_paths: None, cache: None, - cache_on_save: false, + cache_on_save: true, certificate_stores: None, config: None, import_map: None, diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 6d60ad621a..a54ec95295 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -377,6 +377,7 @@ impl LanguageServer { specifiers: Vec, referrer: ModuleSpecifier, force_global_cache: bool, + lockfile_skip_write: bool, ) -> LspResult> { async fn create_graph_for_caching( factory: CliFactory, @@ -439,6 +440,7 @@ impl LanguageServer { specifiers, referrer, force_global_cache, + lockfile_skip_write, ); match prepare_cache_result { @@ -458,7 +460,12 @@ impl LanguageServer { // now get the lock back to update with the new information *self.did_change_batch_queue.borrow_mut() = None; - self.inner.write().await.post_cache().await; + self + .inner + .write() + .await + .post_cache(!lockfile_skip_write) + .await; self.performance.measure(mark); } Err(err) => { @@ -1254,7 +1261,9 @@ impl Inner { }; specifier }; - if let Err(err) = ls.cache(vec![specifier], referrer, false).await { + if let Err(err) = + ls.cache(vec![specifier], referrer, false, true).await + { lsp_warn!("{:#}", err); } }); @@ -1451,7 +1460,7 @@ impl Inner { self.task_queue.queue_task(Box::new(|ls: LanguageServer| { spawn(async move { if let Err(err) = ls - .cache(vec![], module.specifier.as_ref().clone(), false) + .cache(vec![], module.specifier.as_ref().clone(), false, true) .await { lsp_warn!( @@ -4085,7 +4094,7 @@ impl tower_lsp::LanguageServer for LanguageServer { serde_json::from_value(json!(params.arguments)) .map_err(|err| LspError::invalid_params(err.to_string()))?; self - .cache(specifiers, referrer, options.force_global_cache) + .cache(specifiers, referrer, options.force_global_cache, false) .await } else if params.command == "deno.reloadImportRegistries" { *self.did_change_batch_queue.borrow_mut() = None; @@ -4694,6 +4703,7 @@ impl Inner { specifiers: Vec, referrer: ModuleSpecifier, force_global_cache: bool, + lockfile_skip_write: bool, ) -> Result { let config_data = self.config.tree.data_for_specifier(&referrer); let scope = config_data.map(|d| d.scope.clone()); @@ -4725,6 +4735,7 @@ impl Inner { let mut cli_factory = CliFactory::from_flags(Arc::new(Flags { internal: InternalFlags { cache_path: Some(self.cache.deno_dir().root.clone()), + lockfile_skip_write, ..Default::default() }, ca_stores: workspace_settings.certificate_stores.clone(), @@ -4776,9 +4787,22 @@ impl Inner { } #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))] - async fn post_cache(&mut self) { - self.resolver.did_cache(); - self.refresh_dep_info(); + async fn post_cache(&mut self, did_write_lockfile: bool) { + if did_write_lockfile { + // Most of the refresh steps will happen in `did_change_watched_files()`, + // since the lockfile was written. + self.resolver.did_cache(); + self.refresh_dep_info(); + } else { + self.refresh_config_tree().await; + self.update_cache(); + self.refresh_resolver().await; + self.refresh_compiler_options_resolver(); + self.refresh_linter_resolver(); + self.refresh_documents_config(); + self.resolver.did_cache(); + self.refresh_dep_info(); + } self.project_changed(vec![], ProjectScopesChange::Config); self.ts_server.cleanup_semantic_cache(self.snapshot()).await; self.send_diagnostics_update(); diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 869d65ef93..4719c183df 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -6586,7 +6586,7 @@ fn lsp_cache_on_save() { .use_temp_cwd() .build(); let temp_dir = context.temp_dir(); - temp_dir.write( + let file = temp_dir.source_file( "file.ts", r#" import { printHello } from "http://localhost:4545/subdir/print_hello.ts"; @@ -6595,25 +6595,12 @@ fn lsp_cache_on_save() { ); let mut client = context.new_lsp_command().build(); client.initialize_default(); - client.change_configuration(json!({ - "deno": { - "enable": true, - "cacheOnSave": true, - }, - })); - let diagnostics = client.did_open(json!({ - "textDocument": { - "uri": url_to_uri(&temp_dir.url().join("file.ts").unwrap()).unwrap(), - "languageId": "typescript", - "version": 1, - "text": temp_dir.read_to_string("file.ts"), - } - })); + let diagnostics = client.did_open_file(&file); assert_eq!( diagnostics.messages_with_source("deno"), serde_json::from_value(json!({ - "uri": url_to_uri(&temp_dir.url().join("file.ts").unwrap()).unwrap(), + "uri": file.uri(), "diagnostics": [{ "range": { "start": { "line": 1, "character": 33 }, @@ -6630,10 +6617,13 @@ fn lsp_cache_on_save() { .unwrap() ); client.did_save(json!({ - "textDocument": { "uri": url_to_uri(&temp_dir.url().join("file.ts").unwrap()).unwrap() }, + "textDocument": { "uri": file.uri() }, })); client.handle_refresh_diagnostics_request(); - assert_eq!(client.read_diagnostics().all(), vec![]); + assert_eq!(json!(client.read_diagnostics().all()), json!([])); + + // Lockfiles should not be written from cache-on-save. + assert!(!temp_dir.path().join("deno.lock").exists()); client.shutdown(); }