fix(lsp): don't write lockfile during cache-on-save (#30733)
Some checks are pending
ci / publish canary (push) Blocked by required conditions
ci / test release macos-x86_64 (push) Blocked by required conditions
ci / test debug windows-x86_64 (push) Blocked by required conditions
ci / test release windows-x86_64 (push) Blocked by required conditions
ci / build libs (push) Blocked by required conditions
ci / pre-build (push) Waiting to run
ci / test debug linux-aarch64 (push) Blocked by required conditions
ci / test release linux-aarch64 (push) Blocked by required conditions
ci / test debug macos-aarch64 (push) Blocked by required conditions
ci / test release macos-aarch64 (push) Blocked by required conditions
ci / bench release linux-x86_64 (push) Blocked by required conditions
ci / lint debug linux-x86_64 (push) Blocked by required conditions
ci / lint debug macos-x86_64 (push) Blocked by required conditions
ci / lint debug windows-x86_64 (push) Blocked by required conditions
ci / test debug linux-x86_64 (push) Blocked by required conditions
ci / test release linux-x86_64 (push) Blocked by required conditions
ci / test debug macos-x86_64 (push) Blocked by required conditions

This commit is contained in:
Nayeem Rahman 2025-09-15 21:01:05 +01:00 committed by GitHub
parent 73853545f1
commit a6663b0ce3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 43 additions and 29 deletions

View file

@ -555,7 +555,7 @@ pub struct WorkspaceSettings {
/// Cache local modules and their dependencies on `textDocument/didSave` /// Cache local modules and their dependencies on `textDocument/didSave`
/// notifications corresponding to them. /// notifications corresponding to them.
#[serde(default)] #[serde(default = "default_to_true")]
pub cache_on_save: bool, pub cache_on_save: bool,
/// Override the default stores used to validate certificates. This overrides /// Override the default stores used to validate certificates. This overrides
@ -632,7 +632,7 @@ impl Default for WorkspaceSettings {
disable_paths: vec![], disable_paths: vec![],
enable_paths: None, enable_paths: None,
cache: None, cache: None,
cache_on_save: false, cache_on_save: true,
certificate_stores: None, certificate_stores: None,
config: None, config: None,
import_map: None, import_map: None,
@ -1433,7 +1433,7 @@ impl ConfigData {
is_package_manager_subcommand: false, is_package_manager_subcommand: false,
frozen_lockfile: None, frozen_lockfile: None,
lock_arg: None, lock_arg: None,
lockfile_skip_write: false, lockfile_skip_write: true,
node_modules_dir: Some(resolve_node_modules_dir_mode( node_modules_dir: Some(resolve_node_modules_dir_mode(
&member_dir.workspace, &member_dir.workspace,
byonm, byonm,
@ -2146,7 +2146,7 @@ mod tests {
disable_paths: vec![], disable_paths: vec![],
enable_paths: None, enable_paths: None,
cache: None, cache: None,
cache_on_save: false, cache_on_save: true,
certificate_stores: None, certificate_stores: None,
config: None, config: None,
import_map: None, import_map: None,

View file

@ -377,6 +377,7 @@ impl LanguageServer {
specifiers: Vec<ModuleSpecifier>, specifiers: Vec<ModuleSpecifier>,
referrer: ModuleSpecifier, referrer: ModuleSpecifier,
force_global_cache: bool, force_global_cache: bool,
lockfile_skip_write: bool,
) -> LspResult<Option<Value>> { ) -> LspResult<Option<Value>> {
async fn create_graph_for_caching( async fn create_graph_for_caching(
factory: CliFactory, factory: CliFactory,
@ -439,6 +440,7 @@ impl LanguageServer {
specifiers, specifiers,
referrer, referrer,
force_global_cache, force_global_cache,
lockfile_skip_write,
); );
match prepare_cache_result { match prepare_cache_result {
@ -458,7 +460,12 @@ impl LanguageServer {
// now get the lock back to update with the new information // now get the lock back to update with the new information
*self.did_change_batch_queue.borrow_mut() = None; *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); self.performance.measure(mark);
} }
Err(err) => { Err(err) => {
@ -1254,7 +1261,9 @@ impl Inner {
}; };
specifier 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); lsp_warn!("{:#}", err);
} }
}); });
@ -1451,7 +1460,7 @@ impl Inner {
self.task_queue.queue_task(Box::new(|ls: LanguageServer| { self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
spawn(async move { spawn(async move {
if let Err(err) = ls if let Err(err) = ls
.cache(vec![], module.specifier.as_ref().clone(), false) .cache(vec![], module.specifier.as_ref().clone(), false, true)
.await .await
{ {
lsp_warn!( lsp_warn!(
@ -4085,7 +4094,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
serde_json::from_value(json!(params.arguments)) serde_json::from_value(json!(params.arguments))
.map_err(|err| LspError::invalid_params(err.to_string()))?; .map_err(|err| LspError::invalid_params(err.to_string()))?;
self self
.cache(specifiers, referrer, options.force_global_cache) .cache(specifiers, referrer, options.force_global_cache, false)
.await .await
} else if params.command == "deno.reloadImportRegistries" { } else if params.command == "deno.reloadImportRegistries" {
*self.did_change_batch_queue.borrow_mut() = None; *self.did_change_batch_queue.borrow_mut() = None;
@ -4694,6 +4703,7 @@ impl Inner {
specifiers: Vec<ModuleSpecifier>, specifiers: Vec<ModuleSpecifier>,
referrer: ModuleSpecifier, referrer: ModuleSpecifier,
force_global_cache: bool, force_global_cache: bool,
lockfile_skip_write: bool,
) -> Result<PrepareCacheResult, AnyError> { ) -> Result<PrepareCacheResult, AnyError> {
let config_data = self.config.tree.data_for_specifier(&referrer); let config_data = self.config.tree.data_for_specifier(&referrer);
let scope = config_data.map(|d| d.scope.clone()); 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 { let mut cli_factory = CliFactory::from_flags(Arc::new(Flags {
internal: InternalFlags { internal: InternalFlags {
cache_path: Some(self.cache.deno_dir().root.clone()), cache_path: Some(self.cache.deno_dir().root.clone()),
lockfile_skip_write,
..Default::default() ..Default::default()
}, },
ca_stores: workspace_settings.certificate_stores.clone(), ca_stores: workspace_settings.certificate_stores.clone(),
@ -4776,9 +4787,22 @@ impl Inner {
} }
#[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))] #[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
async fn post_cache(&mut self) { 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.resolver.did_cache();
self.refresh_dep_info(); 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.project_changed(vec![], ProjectScopesChange::Config);
self.ts_server.cleanup_semantic_cache(self.snapshot()).await; self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
self.send_diagnostics_update(); self.send_diagnostics_update();

View file

@ -6586,7 +6586,7 @@ fn lsp_cache_on_save() {
.use_temp_cwd() .use_temp_cwd()
.build(); .build();
let temp_dir = context.temp_dir(); let temp_dir = context.temp_dir();
temp_dir.write( let file = temp_dir.source_file(
"file.ts", "file.ts",
r#" r#"
import { printHello } from "http://localhost:4545/subdir/print_hello.ts"; 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(); let mut client = context.new_lsp_command().build();
client.initialize_default(); client.initialize_default();
client.change_configuration(json!({
"deno": {
"enable": true,
"cacheOnSave": true,
},
}));
let diagnostics = client.did_open(json!({ let diagnostics = client.did_open_file(&file);
"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"),
}
}));
assert_eq!( assert_eq!(
diagnostics.messages_with_source("deno"), diagnostics.messages_with_source("deno"),
serde_json::from_value(json!({ serde_json::from_value(json!({
"uri": url_to_uri(&temp_dir.url().join("file.ts").unwrap()).unwrap(), "uri": file.uri(),
"diagnostics": [{ "diagnostics": [{
"range": { "range": {
"start": { "line": 1, "character": 33 }, "start": { "line": 1, "character": 33 },
@ -6630,10 +6617,13 @@ fn lsp_cache_on_save() {
.unwrap() .unwrap()
); );
client.did_save(json!({ 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(); 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(); client.shutdown();
} }