Fix prettier errors around Zed's settings.json/keymap.json files (#3191)

Kirill Bulatov created

Deals with https://github.com/zed-industries/community/issues/2191

Fix Zed starting too many prettier installations in the beginning, and
not being able to format the config files.

Release Notes:

- Fixed prettier not being able to format Zed's config files and spawning excessive prettier installations

Change summary

crates/prettier/src/prettier_server.js |   9 +
crates/project/src/project.rs          | 143 ++++++++++++++++++++-------
crates/project/src/worktree.rs         |  12 +-
3 files changed, 118 insertions(+), 46 deletions(-)

Detailed changes

crates/prettier/src/prettier_server.js 🔗

@@ -55,8 +55,11 @@ async function handleBuffer(prettier) {
         }
         // allow concurrent request handling by not `await`ing the message handling promise (async function)
         handleMessage(message, prettier).catch(e => {
-            sendResponse({ id: message.id, ...makeError(`error during message handling: ${e}`) });
-        });
+            const errorMessage = message;
+            if ((errorMessage.params || {}).text !== undefined) {
+                errorMessage.params.text = "..snip..";
+            }
+            sendResponse({ id: message.id, ...makeError(`error during message '${JSON.stringify(errorMessage)}' handling: ${e}`) }); });
     }
 }
 
@@ -172,7 +175,7 @@ async function handleMessage(message, prettier) {
         sendResponse({ id, result: null });
     } else if (method === 'initialize') {
         sendResponse({
-            id,
+            id: id || 0,
             result: {
                 "capabilities": {}
             }

crates/project/src/project.rs 🔗

@@ -162,12 +162,20 @@ pub struct Project {
     copilot_log_subscription: Option<lsp::Subscription>,
     current_lsp_settings: HashMap<Arc<str>, LspSettings>,
     node: Option<Arc<dyn NodeRuntime>>,
+    #[cfg(not(any(test, feature = "test-support")))]
+    default_prettier: Option<DefaultPrettier>,
     prettier_instances: HashMap<
         (Option<WorktreeId>, PathBuf),
         Shared<Task<Result<Arc<Prettier>, Arc<anyhow::Error>>>>,
     >,
 }
 
+#[cfg(not(any(test, feature = "test-support")))]
+struct DefaultPrettier {
+    installation_process: Option<Shared<Task<()>>>,
+    installed_plugins: HashSet<&'static str>,
+}
+
 struct DelayedDebounced {
     task: Option<Task<()>>,
     cancel_channel: Option<oneshot::Sender<()>>,
@@ -677,6 +685,8 @@ impl Project {
                 copilot_log_subscription: None,
                 current_lsp_settings: settings::get::<ProjectSettings>(cx).lsp.clone(),
                 node: Some(node),
+                #[cfg(not(any(test, feature = "test-support")))]
+                default_prettier: None,
                 prettier_instances: HashMap::default(),
             }
         })
@@ -776,6 +786,8 @@ impl Project {
                 copilot_log_subscription: None,
                 current_lsp_settings: settings::get::<ProjectSettings>(cx).lsp.clone(),
                 node: None,
+                #[cfg(not(any(test, feature = "test-support")))]
+                default_prettier: None,
                 prettier_instances: HashMap::default(),
             };
             for worktree in worktrees {
@@ -8497,7 +8509,7 @@ impl Project {
 
     #[cfg(any(test, feature = "test-support"))]
     fn install_default_formatters(
-        &self,
+        &mut self,
         _worktree: Option<WorktreeId>,
         _new_language: &Language,
         _language_settings: &LanguageSettings,
@@ -8508,7 +8520,7 @@ impl Project {
 
     #[cfg(not(any(test, feature = "test-support")))]
     fn install_default_formatters(
-        &self,
+        &mut self,
         worktree: Option<WorktreeId>,
         new_language: &Language,
         language_settings: &LanguageSettings,
@@ -8537,51 +8549,108 @@ impl Project {
             return Task::ready(Ok(()));
         };
 
+        let mut plugins_to_install = prettier_plugins;
+        let (mut install_success_tx, mut install_success_rx) =
+            futures::channel::mpsc::channel::<HashSet<&'static str>>(1);
+        let new_installation_process = cx
+            .spawn(|this, mut cx| async move {
+                if let Some(installed_plugins) = install_success_rx.next().await {
+                    this.update(&mut cx, |this, _| {
+                        let default_prettier =
+                            this.default_prettier
+                                .get_or_insert_with(|| DefaultPrettier {
+                                    installation_process: None,
+                                    installed_plugins: HashSet::default(),
+                                });
+                        if !installed_plugins.is_empty() {
+                            log::info!("Installed new prettier plugins: {installed_plugins:?}");
+                            default_prettier.installed_plugins.extend(installed_plugins);
+                        }
+                    })
+                }
+            })
+            .shared();
+        let previous_installation_process =
+            if let Some(default_prettier) = &mut self.default_prettier {
+                plugins_to_install
+                    .retain(|plugin| !default_prettier.installed_plugins.contains(plugin));
+                if plugins_to_install.is_empty() {
+                    return Task::ready(Ok(()));
+                }
+                std::mem::replace(
+                    &mut default_prettier.installation_process,
+                    Some(new_installation_process.clone()),
+                )
+            } else {
+                None
+            };
+
         let default_prettier_dir = util::paths::DEFAULT_PRETTIER_DIR.as_path();
         let already_running_prettier = self
             .prettier_instances
             .get(&(worktree, default_prettier_dir.to_path_buf()))
             .cloned();
-
         let fs = Arc::clone(&self.fs);
-        cx.background()
-            .spawn(async move {
-                let prettier_wrapper_path = default_prettier_dir.join(prettier::PRETTIER_SERVER_FILE);
-                // method creates parent directory if it doesn't exist
-                fs.save(&prettier_wrapper_path, &text::Rope::from(prettier::PRETTIER_SERVER_JS), text::LineEnding::Unix).await
-                .with_context(|| format!("writing {} file at {prettier_wrapper_path:?}", prettier::PRETTIER_SERVER_FILE))?;
-
-                let packages_to_versions = future::try_join_all(
-                    prettier_plugins
-                        .iter()
-                        .chain(Some(&"prettier"))
-                        .map(|package_name| async {
-                            let returned_package_name = package_name.to_string();
-                            let latest_version = node.npm_package_latest_version(package_name)
-                                .await
-                                .with_context(|| {
-                                    format!("fetching latest npm version for package {returned_package_name}")
-                                })?;
-                            anyhow::Ok((returned_package_name, latest_version))
-                        }),
-                )
-                .await
-                .context("fetching latest npm versions")?;
+        cx.spawn(|this, mut cx| async move {
+            if let Some(previous_installation_process) = previous_installation_process {
+                previous_installation_process.await;
+            }
+            let mut everything_was_installed = false;
+            this.update(&mut cx, |this, _| {
+                match &mut this.default_prettier {
+                    Some(default_prettier) => {
+                        plugins_to_install
+                            .retain(|plugin| !default_prettier.installed_plugins.contains(plugin));
+                        everything_was_installed = plugins_to_install.is_empty();
+                    },
+                    None => this.default_prettier = Some(DefaultPrettier { installation_process: Some(new_installation_process), installed_plugins: HashSet::default() }),
+                }
+            });
+            if everything_was_installed {
+                return Ok(());
+            }
 
-                log::info!("Fetching default prettier and plugins: {packages_to_versions:?}");
-                let borrowed_packages = packages_to_versions.iter().map(|(package, version)| {
-                    (package.as_str(), version.as_str())
-                }).collect::<Vec<_>>();
-                node.npm_install_packages(default_prettier_dir, &borrowed_packages).await.context("fetching formatter packages")?;
+            cx.background()
+                .spawn(async move {
+                    let prettier_wrapper_path = default_prettier_dir.join(prettier::PRETTIER_SERVER_FILE);
+                    // method creates parent directory if it doesn't exist
+                    fs.save(&prettier_wrapper_path, &text::Rope::from(prettier::PRETTIER_SERVER_JS), text::LineEnding::Unix).await
+                    .with_context(|| format!("writing {} file at {prettier_wrapper_path:?}", prettier::PRETTIER_SERVER_FILE))?;
 
-                if !prettier_plugins.is_empty() {
-                    if let Some(prettier) = already_running_prettier {
-                        prettier.await.map_err(|e| anyhow::anyhow!("Default prettier startup await failure: {e:#}"))?.clear_cache().await.context("clearing default prettier cache after plugins install")?;
+                    let packages_to_versions = future::try_join_all(
+                        plugins_to_install
+                            .iter()
+                            .chain(Some(&"prettier"))
+                            .map(|package_name| async {
+                                let returned_package_name = package_name.to_string();
+                                let latest_version = node.npm_package_latest_version(package_name)
+                                    .await
+                                    .with_context(|| {
+                                        format!("fetching latest npm version for package {returned_package_name}")
+                                    })?;
+                                anyhow::Ok((returned_package_name, latest_version))
+                            }),
+                    )
+                    .await
+                    .context("fetching latest npm versions")?;
+
+                    log::info!("Fetching default prettier and plugins: {packages_to_versions:?}");
+                    let borrowed_packages = packages_to_versions.iter().map(|(package, version)| {
+                        (package.as_str(), version.as_str())
+                    }).collect::<Vec<_>>();
+                    node.npm_install_packages(default_prettier_dir, &borrowed_packages).await.context("fetching formatter packages")?;
+                    let installed_packages = !plugins_to_install.is_empty();
+                    install_success_tx.try_send(plugins_to_install).ok();
+
+                    if !installed_packages {
+                        if let Some(prettier) = already_running_prettier {
+                            prettier.await.map_err(|e| anyhow::anyhow!("Default prettier startup await failure: {e:#}"))?.clear_cache().await.context("clearing default prettier cache after plugins install")?;
+                        }
                     }
-                }
 
-                anyhow::Ok(())
-            })
+                    anyhow::Ok(())
+                }).await
+        })
     }
 }
 

crates/project/src/worktree.rs 🔗

@@ -2662,12 +2662,12 @@ impl language::File for File {
 
 impl language::LocalFile for File {
     fn abs_path(&self, cx: &AppContext) -> PathBuf {
-        self.worktree
-            .read(cx)
-            .as_local()
-            .unwrap()
-            .abs_path
-            .join(&self.path)
+        let worktree_path = &self.worktree.read(cx).as_local().unwrap().abs_path;
+        if self.path.as_ref() == Path::new("") {
+            worktree_path.to_path_buf()
+        } else {
+            worktree_path.join(&self.path)
+        }
     }
 
     fn load(&self, cx: &AppContext) -> Task<Result<String>> {