Refactor config search (#15822)

src/config/editorconfig/index.js

@@ -2,26 +2,19 @@ import path from "node:path";
 
 import editorconfig from "editorconfig";
 
-import findProjectRootWithoutCache from "../find-project-root.js";
+import {
+  clearFindProjectRootCache,
+  findProjectRoot,
+} from "../find-project-root.js";
 import editorConfigToPrettier from "./editorconfig-to-prettier.js";
 
-const projectRootCache = new Map();
 const editorconfigCache = new Map();
 
 function clearEditorconfigCache() {
-  projectRootCache.clear();
+  clearFindProjectRootCache();
   editorconfigCache.clear();
 }
 
-// TODO: Improve this cache
-function findProjectRoot(directory, { shouldCache }) {
-  if (!shouldCache || !projectRootCache.has(directory)) {
-    // Even if `shouldCache` is false, we still cache the result, so we can use it when `shouldCache` is true
-    projectRootCache.set(directory, findProjectRootWithoutCache(directory));
-  }
-  return projectRootCache.get(directory);
-}
-
 async function loadEditorconfigInternal(file, { shouldCache }) {
   const directory = path.dirname(file);
   const root = await findProjectRoot(directory, { shouldCache });

src/config/find-project-root.js

@@ -1,29 +1,34 @@
 // Simple version of `find-project-root`
 // https://github.com/kirstein/find-project-root/blob/master/index.js
 
-import path from "node:path";
-
-import iterateDirectoryUp from "iterate-directory-up";
+import * as path from "node:path";
 
 import isDirectory from "../utils/is-directory.js";
+import Searcher from "./searcher.js";
 
 const MARKERS = [".git", ".hg"];
+let searcher;
+const searchOptions = {
+  names: MARKERS,
+  filter: ({ path: directory }) =>
+    isDirectory(directory, { allowSymlinks: false }),
+};
 
 /**
  * Find the directory contains a version control system directory
  * @param {string} startDirectory
+ * @param {{shouldCache?: boolean}} options
  * @returns {Promise<string | undefined>}
  */
-async function findProjectRoot(startDirectory) {
-  for (const directory of iterateDirectoryUp(startDirectory)) {
-    for (const name of MARKERS) {
-      const directoryPath = path.join(directory, name);
+async function findProjectRoot(startDirectory, options) {
+  searcher ??= new Searcher(searchOptions);
+  const mark = await searcher.search(startDirectory, options);
+
+  return mark ? path.dirname(mark) : undefined;
+}
 
-      if (await isDirectory(directoryPath, { allowSymlinks: false })) {
-        return directory;
-      }
-    }
-  }
+function clearFindProjectRootCache() {
+  searcher?.clearCache();
 }
 
-export default findProjectRoot;
+export { clearFindProjectRootCache, findProjectRoot };

src/config/prettier-config/config-searcher.js

@@ -1,8 +1,5 @@
-import path from "node:path";
-
-import iterateDirectoryUp from "iterate-directory-up";
-
 import isFile from "../../utils/is-file.js";
+import Searcher from "../searcher.js";
 import { loadConfigFromPackageJson } from "./loaders.js";
 
 const CONFIG_FILE_NAMES = [
@@ -21,101 +18,24 @@ const CONFIG_FILE_NAMES = [
   ".prettierrc.toml",
 ];
 
-async function isPackageJsonFileWithPrettierConfig(file) {
-  try {
-    return Boolean(await loadConfigFromPackageJson(file));
-  } catch {
+async function filter({ name, path: file }) {
+  if (!(await isFile(file))) {
     return false;
   }
-}
-
-async function searchConfigInDirectory(directory) {
-  for (const fileName of CONFIG_FILE_NAMES) {
-    const file = path.join(directory, fileName);
-
-    if (!(await isFile(file))) {
-      continue;
-    }
 
-    if (
-      fileName !== "package.json" ||
-      (await isPackageJsonFileWithPrettierConfig(file))
-    ) {
-      return file;
+  if (name === "package.json") {
+    try {
+      return Boolean(await loadConfigFromPackageJson(file));
+    } catch {
+      return false;
     }
   }
-}
-
-function createCachedSearchConfigInDirectory() {
-  const cache = new Map();
 
-  return async function (fileOrDirectory) {
-    if (cache.has(fileOrDirectory)) {
-      return cache.get(fileOrDirectory);
-    }
-
-    const promise = searchConfigInDirectory(fileOrDirectory);
-    cache.set(fileOrDirectory, promise);
-    const result = await promise;
-    cache.set(fileOrDirectory, result);
-    return result;
-  };
+  return true;
 }
 
-/** @type {Map} */ // @ts-expect-error -- intentionally not add the `get` method
-const noopMap = { has: () => false, set() {} };
-class ConfigSearcher {
-  #shouldCache;
-  #stopDirectory;
-  #searchedFilesStore;
-  #searchConfigInDirectory;
-  constructor({ stopDirectory, shouldCache }) {
-    this.#shouldCache = shouldCache;
-    this.#stopDirectory = stopDirectory;
-    this.#searchConfigInDirectory = shouldCache
-      ? createCachedSearchConfigInDirectory()
-      : searchConfigInDirectory;
-    this.#searchedFilesStore = shouldCache ? new Map() : noopMap;
-  }
-
-  async #searchConfigInDirectories(startDirectory) {
-    const store = this.#searchedFilesStore;
-    for (const directory of iterateDirectoryUp(
-      startDirectory,
-      this.#stopDirectory,
-    )) {
-      // The parent directory may already searched
-      if (store.has(directory)) {
-        return store.get(directory);
-      }
-
-      const file = await this.#searchConfigInDirectory(directory);
-      if (file) {
-        return file;
-      }
-    }
-  }
-
-  async search(startDirectory) {
-    const store = this.#searchedFilesStore;
-    if (store.has(startDirectory)) {
-      return store.get(startDirectory);
-    }
-
-    const configFile = await this.#searchConfigInDirectories(startDirectory);
-
-    // Directories between startDirectory and configFile directory should has the same result
-    if (this.#shouldCache) {
-      for (const directory of iterateDirectoryUp(
-        startDirectory,
-        configFile ? path.dirname(configFile) : startDirectory,
-      )) {
-        store.set(directory, configFile);
-      }
-    }
-
-    return configFile;
-  }
+function getSearcher(stopDirectory) {
+  return new Searcher({ names: CONFIG_FILE_NAMES, filter, stopDirectory });
 }
 
-export default ConfigSearcher;
+export default getSearcher;

src/config/prettier-config/index.js

@@ -1,7 +1,7 @@
 import path from "node:path";
 
 import mockable from "../../common/mockable.js";
-import ConfigSearcher from "./config-searcher.js";
+import getConfigSearcher from "./config-searcher.js";
 import loadConfig from "./load-config.js";
 
 const loadCache = new Map();
@@ -28,37 +28,35 @@ function loadPrettierConfig(configFile, { shouldCache }) {
 }
 
 /**
- * @param {{shouldCache?: boolean, stopDirectory?: string}} param0
+ * @param {string} stopDirectory
  */
-function getSearchFunction({ shouldCache, stopDirectory }) {
+function getSearchFunction(stopDirectory) {
   stopDirectory = stopDirectory ? path.resolve(stopDirectory) : undefined;
-  const searchCacheKey = JSON.stringify({ shouldCache, stopDirectory });
 
-  if (!searchCache.has(searchCacheKey)) {
-    const searcher = new ConfigSearcher({ shouldCache, stopDirectory });
+  if (!searchCache.has(stopDirectory)) {
+    const searcher = getConfigSearcher(stopDirectory);
     const searchFunction = searcher.search.bind(searcher);
-
-    searchCache.set(searchCacheKey, searchFunction);
+    searchCache.set(stopDirectory, searchFunction);
   }
 
-  return searchCache.get(searchCacheKey);
+  return searchCache.get(stopDirectory);
 }
 
 /**
  * @param {string} startDirectory
- * @param {{shouldCache?: boolean, stopDirectory?: string}} options
+ * @param {{shouldCache?: boolean}} options
  * @returns {Promise<string>}
  */
 function searchPrettierConfig(startDirectory, options = {}) {
   startDirectory = startDirectory
     ? path.resolve(startDirectory)
     : process.cwd();
 
-  options.stopDirectory = mockable.getPrettierConfigSearchStopDirectory();
+  const stopDirectory = mockable.getPrettierConfigSearchStopDirectory();
 
-  const search = getSearchFunction(options);
+  const search = getSearchFunction(stopDirectory);
 
-  return search(startDirectory);
+  return search(startDirectory, { shouldCache: options.shouldCache });
 }
 
 export { clearPrettierConfigCache, loadPrettierConfig, searchPrettierConfig };

src/config/searcher.js

@@ -0,0 +1,72 @@
+import path from "node:path";
+
+import iterateDirectoryUp from "iterate-directory-up";
+
+class Searcher {
+  #names;
+  #filter;
+  #stopDirectory;
+  #cache = new Map();
+
+  /**
+   * @param {{
+   *   names: string[],
+   *   filter: (fileOrDirectory: {name: string, path: string}) => Promise<boolean>,
+   *   stopDirectory?: string,
+   * }} param0
+   */
+  constructor({ names, filter, stopDirectory }) {
+    this.#names = names;
+    this.#filter = filter;
+    this.#stopDirectory = stopDirectory;
+  }
+
+  async #searchInDirectory(directory, shouldCache) {
+    const cache = this.#cache;
+    if (shouldCache && cache.has(directory)) {
+      return cache.get(directory);
+    }
+
+    for (const name of this.#names) {
+      const fileOrDirectory = path.join(directory, name);
+
+      if (await this.#filter({ name, path: fileOrDirectory })) {
+        return fileOrDirectory;
+      }
+    }
+  }
+
+  async search(startDirectory, { shouldCache }) {
+    const cache = this.#cache;
+    if (shouldCache && cache.has(startDirectory)) {
+      return cache.get(startDirectory);
+    }
+
+    const searchedDirectories = [];
+    let result;
+    for (const directory of iterateDirectoryUp(
+      startDirectory,
+      this.#stopDirectory,
+    )) {
+      searchedDirectories.push(directory);
+      result = await this.#searchInDirectory(directory, shouldCache);
+
+      if (result) {
+        break;
+      }
+    }
+
+    // Always cache the result, so we can use it when `useCache` is set to true
+    for (const directory of searchedDirectories) {
+      cache.set(directory, result);
+    }
+
+    return result;
+  }
+
+  clearCache() {
+    this.#cache.clear();
+  }
+}
+
+export default Searcher;