Fix plugin loading cache (#14576)

changelog_unreleased/api/14576.md

@@ -0,0 +1,3 @@
+#### Fix plugin loading cache (#14576 by @fisker)
+
+Plugin instances are incorrectly memoized, check this [issue](https://github.com/prettier/prettier/issues/13235) for details.

src/common/load-builtin-plugins.js

@@ -1,23 +0,0 @@
-let builtinPlugins;
-
-async function loadBuiltinPlugins() {
-  builtinPlugins ??= await Promise.all([
-    import("../plugins/estree.js"),
-    import("../plugins/babel.js"),
-    import("../plugins/flow.js"),
-    import("../plugins/typescript.js"),
-    import("../plugins/acorn.js"),
-    import("../plugins/meriyah.js"),
-    import("../plugins/angular.js"),
-    import("../plugins/postcss.js"),
-    import("../plugins/graphql.js"),
-    import("../plugins/markdown.js"),
-    import("../plugins/glimmer.js"),
-    import("../plugins/html.js"),
-    import("../plugins/yaml.js"),
-  ]);
-
-  return builtinPlugins;
-}
-
-export default loadBuiltinPlugins;

src/common/load-plugins.js

@@ -1,128 +0,0 @@
-import fs from "node:fs/promises";
-import path from "node:path";
-import { fileURLToPath, pathToFileURL } from "node:url";
-import fastGlob from "fast-glob";
-import mem, { memClear } from "mem";
-import partition from "../utils/partition.js";
-import importFromDirectory from "../utils/import-from-directory.js";
-import thirdParty from "./third-party.js";
-
-const __dirname = path.dirname(fileURLToPath(import.meta.url));
-
-const importPlugin = async (name, directory) => {
-  const module = await importFromDirectory(name, directory);
-  const plugin = module.default ?? module;
-  return { name, ...plugin };
-};
-
-const memoizedLoad = mem(load, { cacheKey: JSON.stringify });
-const memoizedSearch = mem(findPluginsInNodeModules);
-const clearCache = () => {
-  memClear(memoizedLoad);
-  memClear(memoizedSearch);
-};
-
-async function load(plugins, pluginSearchDirs) {
-  plugins ??= [];
-
-  if (pluginSearchDirs === false) {
-    pluginSearchDirs = [];
-  } else {
-    pluginSearchDirs ??= [];
-
-    // unless pluginSearchDirs are provided, auto-load plugins from node_modules that are parent to Prettier
-    if (pluginSearchDirs.length === 0) {
-      const autoLoadDir = thirdParty.findParentDir(__dirname, "node_modules");
-      if (autoLoadDir) {
-        pluginSearchDirs = [autoLoadDir];
-      }
-    }
-  }
-
-  const [externalPluginNames, externalPluginInstances] = partition(
-    plugins,
-    (plugin) => typeof plugin === "string"
-  );
-
-  const externalManualLoadPlugins = await Promise.all(
-    externalPluginNames.map(async (name) => {
-      try {
-        // try local files
-        const module = await import(pathToFileURL(path.resolve(name)).href);
-        const plugin = module.default ?? module;
-        return { name, ...plugin };
-      } catch {
-        // try node modules
-        return importPlugin(name, process.cwd());
-      }
-    })
-  );
-
-  const externalAutoLoadPlugins = (
-    await Promise.all(
-      pluginSearchDirs.map(async (pluginSearchDir) => {
-        const resolvedPluginSearchDir = path.resolve(
-          process.cwd(),
-          pluginSearchDir
-        );
-
-        const nodeModulesDir = path.resolve(
-          resolvedPluginSearchDir,
-          "node_modules"
-        );
-
-        // In some fringe cases (ex: files "mounted" as virtual directories), the
-        // isDirectory(resolvedPluginSearchDir) check might be false even though
-        // the node_modules actually exists.
-        if (
-          !(await isDirectory(nodeModulesDir)) &&
-          !(await isDirectory(resolvedPluginSearchDir))
-        ) {
-          throw new Error(
-            `${pluginSearchDir} does not exist or is not a directory`
-          );
-        }
-
-        const pluginNames = await memoizedSearch(nodeModulesDir);
-
-        return Promise.all(
-          pluginNames.map((name) => importPlugin(name, nodeModulesDir))
-        );
-      })
-    )
-  ).flat();
-
-  return [
-    ...externalManualLoadPlugins,
-    ...externalAutoLoadPlugins,
-    ...externalPluginInstances,
-  ];
-}
-
-async function findPluginsInNodeModules(nodeModulesDir) {
-  const pluginPackageJsonPaths = await fastGlob(
-    [
-      "prettier-plugin-*/package.json",
-      "@*/prettier-plugin-*/package.json",
-      "@prettier/plugin-*/package.json",
-    ],
-    {
-      cwd: nodeModulesDir,
-    }
-  );
-  return pluginPackageJsonPaths.map(path.dirname);
-}
-
-async function isDirectory(dir) {
-  let stat;
-
-  try {
-    stat = await fs.stat(dir);
-  } catch {
-    return false;
-  }
-
-  return stat.isDirectory();
-}
-
-export { memoizedLoad as loadPlugins, clearCache };

src/index.js

@@ -4,9 +4,11 @@ import * as core from "./main/core.js";
 import { getSupportInfo as getSupportInfoWithoutPlugins } from "./main/support.js";
 import getFileInfoWithoutPlugins from "./common/get-file-info.js";
 import {
+  loadBuiltinPlugins,
   loadPlugins,
+  searchPlugins,
   clearCache as clearPluginCache,
-} from "./common/load-plugins.js";
+} from "./main/plugins/index.js";
 import {
   resolveConfig,
   resolveConfigFile,
@@ -20,30 +22,34 @@ import normalizeOptions from "./main/normalize-options.js";
 import arrayify from "./utils/arrayify.js";
 import partition from "./utils/partition.js";
 import isNonEmptyArray from "./utils/is-non-empty-array.js";
-import loadBuiltinPlugins from "./common/load-builtin-plugins.js";
 
 /**
  * @param {*} fn
- * @param {*} optsArgIdx
+ * @param {number} [optionsArgumentIndex]
  * @returns {*}
  */
 function withPlugins(
   fn,
-  optsArgIdx = 1 // Usually `opts` is the 2nd argument
+  optionsArgumentIndex = 1 // Usually `options` is the 2nd argument
 ) {
   return async (...args) => {
-    const opts = args[optsArgIdx] || {};
-    const plugins = (
-      await Promise.all([
-        loadBuiltinPlugins(),
-        loadPlugins(opts.plugins, opts.pluginSearchDirs),
-      ])
-    ).flat();
+    const options = args[optionsArgumentIndex] ?? {};
+    const { plugins = [], pluginSearchDirs } = options;
 
-    args[optsArgIdx] = {
-      ...opts,
-      plugins,
+    args[optionsArgumentIndex] = {
+      ...options,
+      plugins: (
+        await Promise.all([
+          loadBuiltinPlugins(),
+          // TODO: standalone version allow `plugins` to be `prettierPlugins` which is an object, should allow that too
+          loadPlugins(plugins),
+          options.pluginSearchDirs === false
+            ? []
+            : searchPlugins(pluginSearchDirs),
+        ])
+      ).flat(),
     };
+
     return fn(...args);
   };
 }

src/main/plugins/index.js

@@ -0,0 +1,10 @@
+import { clearCache as clearPluginLoadCache } from "./load-plugin.js";
+import { clearCache as clearPluginSearchCache } from "./search-plugins.js";
+
+export function clearCache() {
+  clearPluginLoadCache();
+  clearPluginSearchCache();
+}
+export { default as loadBuiltinPlugins } from "./load-builtin-plugins.js";
+export { default as loadPlugins } from "./load-plugins.js";
+export { searchPlugins } from "./search-plugins.js";

src/main/plugins/load-builtin-plugins.js

@@ -0,0 +1,23 @@
+let builtinPlugins;
+
+async function loadBuiltinPlugins() {
+  builtinPlugins ??= await Promise.all([
+    import("../../plugins/estree.js"),
+    import("../../plugins/babel.js"),
+    import("../../plugins/flow.js"),
+    import("../../plugins/typescript.js"),
+    import("../../plugins/acorn.js"),
+    import("../../plugins/meriyah.js"),
+    import("../../plugins/angular.js"),
+    import("../../plugins/postcss.js"),
+    import("../../plugins/graphql.js"),
+    import("../../plugins/markdown.js"),
+    import("../../plugins/glimmer.js"),
+    import("../../plugins/html.js"),
+    import("../../plugins/yaml.js"),
+  ]);
+
+  return builtinPlugins;
+}
+
+export default loadBuiltinPlugins;

src/main/plugins/load-plugin.js

@@ -0,0 +1,41 @@
+import { pathToFileURL } from "node:url";
+import path from "node:path";
+import mem, { memClear } from "mem";
+import importFromDirectory from "../../utils/import-from-directory.js";
+
+function normalizePlugin(pluginInstanceOfPluginModule, name) {
+  const plugin =
+    pluginInstanceOfPluginModule.default ?? pluginInstanceOfPluginModule;
+  return { name, ...plugin };
+}
+
+const loadPluginFromDirectory = mem(
+  async (name, directory) =>
+    normalizePlugin(await importFromDirectory(name, directory), name),
+  { cacheKey: JSON.stringify }
+);
+
+const importPlugin = mem(async (name) => {
+  try {
+    // try local files
+    return await import(pathToFileURL(path.resolve(name)).href);
+  } catch {
+    // try node modules
+    return importFromDirectory(name, process.cwd());
+  }
+});
+
+async function loadPlugin(plugin) {
+  if (typeof plugin === "string") {
+    return normalizePlugin(await importPlugin(plugin), plugin);
+  }
+
+  return plugin;
+}
+
+function clearCache() {
+  memClear(loadPluginFromDirectory);
+  memClear(importPlugin);
+}
+
+export { loadPlugin, loadPluginFromDirectory, clearCache };

src/main/plugins/load-plugins.js

@@ -0,0 +1,7 @@
+import { loadPlugin } from "./load-plugin.js";
+
+function loadPlugins(plugins = []) {
+  return Promise.all(plugins.map((plugin) => loadPlugin(plugin)));
+}
+
+export default loadPlugins;

src/main/plugins/search-plugins.js

@@ -0,0 +1,76 @@
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import mem, { memClear } from "mem";
+import fastGlob from "fast-glob";
+import isDirectory from "../../utils/is-directory.js";
+import thirdParty from "../../common/third-party.js";
+import { loadPluginFromDirectory } from "./load-plugin.js";
+
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+
+const findNodeModules = mem((directory) =>
+  thirdParty.findParentDir(directory, "node_modules")
+);
+
+const findPluginsInNodeModules = mem(async (nodeModulesDir) => {
+  const pluginPackageJsonPaths = await fastGlob(
+    [
+      "prettier-plugin-*/package.json",
+      "@*/prettier-plugin-*/package.json",
+      "@prettier/plugin-*/package.json",
+    ],
+    {
+      cwd: nodeModulesDir,
+    }
+  );
+  return pluginPackageJsonPaths.map(path.dirname);
+});
+
+const searchPluginsInDirectory = mem(async (directory) => {
+  const absolutePath = path.resolve(process.cwd(), directory);
+  const nodeModulesDir = path.join(absolutePath, "node_modules");
+
+  // In some fringe cases (ex: files "mounted" as virtual directories), the
+  // isDirectory(absolutePath) check might be false even though
+  // the node_modules actually exists.
+  if (
+    !(await isDirectory(nodeModulesDir)) &&
+    !(await isDirectory(absolutePath))
+  ) {
+    throw new Error(`${directory} does not exist or is not a directory`);
+  }
+
+  const pluginNames = await findPluginsInNodeModules(nodeModulesDir);
+
+  return Promise.all(
+    pluginNames.map((name) => loadPluginFromDirectory(name, nodeModulesDir))
+  );
+});
+
+const searchPlugins = mem(
+  async (directories = []) => {
+    // unless pluginSearchDirs are provided, auto-load plugins from node_modules that are parent to Prettier
+    if (directories.length === 0) {
+      const nodeModulesDirectory = findNodeModules(__dirname);
+      if (nodeModulesDirectory) {
+        directories = [nodeModulesDirectory];
+      }
+    }
+
+    return (
+      await Promise.all(
+        directories.map((directory) => searchPluginsInDirectory(directory))
+      )
+    ).flat();
+  },
+  { cacheKey: JSON.stringify }
+);
+
+function clearCache() {
+  memClear(searchPlugins);
+  memClear(searchPluginsInDirectory);
+  memClear(findPluginsInNodeModules);
+  memClear(findNodeModules);
+}
+
+export { searchPlugins, clearCache };

src/standalone.js

@@ -3,16 +3,16 @@ import { getSupportInfo as getSupportInfoWithoutPlugins } from "./main/support.j
 
 function withPlugins(
   fn,
-  optsArgIdx = 1 // Usually `opts` is the 2nd argument
+  optionsArgumentIndex = 1 // Usually `options` is the 2nd argument
 ) {
   // Returns Promises to consistent with functions in `index.js`
   // eslint-disable-next-line require-await
   return async (...args) => {
-    const opts = args[optsArgIdx] || {};
-    const plugins = opts.plugins || [];
+    const options = args[optionsArgumentIndex] ?? {};
+    const plugins = options.plugins ?? [];
 
-    args[optsArgIdx] = {
-      ...opts,
+    args[optionsArgumentIndex] = {
+      ...options,
       plugins: Array.isArray(plugins) ? plugins : Object.values(plugins),
     };
 

src/utils/is-directory.js

@@ -0,0 +1,15 @@
+import fs from "node:fs/promises";
+
+async function isDirectory(directory) {
+  let stat;
+
+  try {
+    stat = await fs.stat(directory);
+  } catch {
+    return false;
+  }
+
+  return stat.isDirectory();
+}
+
+export default isDirectory;

tests/integration/__tests__/plugin-cache.js

@@ -0,0 +1,21 @@
+import prettier from "../../config/prettier-entry.js";
+
+// #13235
+test("Plugin instance should not cached", async () => {
+  expect([
+    await prettier.format(".", {
+      plugins: [
+        { parsers: { baz: { parse: () => ({}), astFormat: "baz-ast" } } },
+        { printers: { "baz-ast": { print: () => "1" } } },
+      ],
+      parser: "baz",
+    }),
+    await prettier.format(".", {
+      plugins: [
+        { parsers: { baz: { parse: () => ({}), astFormat: "baz-ast" } } },
+        { printers: { "baz-ast": { print: () => "2" } } },
+      ],
+      parser: "baz",
+    }),
+  ]).toEqual(["1", "2"]);
+});

tests/unit/builtin-plugins.js

@@ -1,4 +1,4 @@
-import loadBuiltinPlugins from "../../src/common/load-builtin-plugins.js";
+import loadBuiltinPlugins from "../../src/main/plugins/load-builtin-plugins.js";
 
 // Snapshot to prevent mistakes
 test("builtin parsers", async () => {