diff --git a/changelog_unreleased/api/14576.md b/changelog_unreleased/api/14576.md new file mode 100644 index 000000000..ac8a525e0 --- /dev/null +++ b/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. diff --git a/src/common/load-builtin-plugins.js b/src/common/load-builtin-plugins.js deleted file mode 100644 index 79b48d354..000000000 --- a/src/common/load-builtin-plugins.js +++ /dev/null @@ -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; diff --git a/src/common/load-plugins.js b/src/common/load-plugins.js deleted file mode 100644 index 7b13e5a21..000000000 --- a/src/common/load-plugins.js +++ /dev/null @@ -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 }; diff --git a/src/index.js b/src/index.js index b5ab88ce1..14ff57ef0 100644 --- a/src/index.js +++ b/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); }; } diff --git a/src/main/plugins/index.js b/src/main/plugins/index.js new file mode 100644 index 000000000..72e55ed0f --- /dev/null +++ b/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"; diff --git a/src/main/plugins/load-builtin-plugins.js b/src/main/plugins/load-builtin-plugins.js new file mode 100644 index 000000000..79f778fee --- /dev/null +++ b/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; diff --git a/src/main/plugins/load-plugin.js b/src/main/plugins/load-plugin.js new file mode 100644 index 000000000..80e603c01 --- /dev/null +++ b/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 }; diff --git a/src/main/plugins/load-plugins.js b/src/main/plugins/load-plugins.js new file mode 100644 index 000000000..a2bac1bd1 --- /dev/null +++ b/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; diff --git a/src/main/plugins/search-plugins.js b/src/main/plugins/search-plugins.js new file mode 100644 index 000000000..1b020d779 --- /dev/null +++ b/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 }; diff --git a/src/standalone.js b/src/standalone.js index 631e15f70..7a8b4d16c 100644 --- a/src/standalone.js +++ b/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), }; diff --git a/src/utils/is-directory.js b/src/utils/is-directory.js new file mode 100644 index 000000000..e87b6c6cd --- /dev/null +++ b/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; diff --git a/tests/integration/__tests__/plugin-cache.js b/tests/integration/__tests__/plugin-cache.js new file mode 100644 index 000000000..8fdc62dbe --- /dev/null +++ b/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"]); +}); diff --git a/tests/unit/builtin-plugins.js b/tests/unit/builtin-plugins.js index 427d32e83..7fe8fe8c8 100644 --- a/tests/unit/builtin-plugins.js +++ b/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 () => {