From bb203e3fa58e4a7c3e1de8f78fc617284af004ed Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Mon, 1 Aug 2022 10:48:37 +0800 Subject: [PATCH] Simplify plugin loading (#13138) --- .github/workflows/dev-test.yml | 12 ---- .github/workflows/prod-test.yml | 7 --- package.json | 1 - src/common/load-plugins.js | 55 +++++++------------ src/common/require-from.js | 9 +++ src/common/resolve.js | 20 ------- .../__tests__/plugin-resolution.js | 13 +++++ yarn.lock | 5 +- 8 files changed, 43 insertions(+), 79 deletions(-) create mode 100644 src/common/require-from.js delete mode 100644 src/common/resolve.js diff --git a/.github/workflows/dev-test.yml b/.github/workflows/dev-test.yml index 51ee1b19e..d700579ba 100644 --- a/.github/workflows/dev-test.yml +++ b/.github/workflows/dev-test.yml @@ -66,15 +66,3 @@ jobs: if: ${{ matrix.ENABLE_CODE_COVERAGE }} with: fail_ci_if_error: true - - # #8073 test - - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) - run: yarn c8 yarn test "tests/integration/__tests__/(config|plugin)" - env: - PRETTIER_FALLBACK_RESOLVE: true - - - name: Upload Coverage (PRETTIER_FALLBACK_RESOLVE) - uses: codecov/codecov-action@v3.1.0 - if: ${{ matrix.ENABLE_CODE_COVERAGE }} - with: - fail_ci_if_error: true diff --git a/.github/workflows/prod-test.yml b/.github/workflows/prod-test.yml index 81c6e7a28..5ab46e10d 100644 --- a/.github/workflows/prod-test.yml +++ b/.github/workflows/prod-test.yml @@ -110,13 +110,6 @@ jobs: - name: Run Format Tests (standalone) run: yarn test:dist-standalone - # #8073 test - - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) - run: yarn test "tests/integration/__tests__/(config|plugin)" - env: - NODE_ENV: production - PRETTIER_FALLBACK_RESOLVE: true - cli: name: CLI runs-on: ubuntu-latest diff --git a/package.json b/package.json index ccc55d679..a6b9a994d 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,6 @@ "remark-footnotes": "2.0.0", "remark-math": "3.0.1", "remark-parse": "8.0.3", - "resolve": "1.22.1", "sdbm": "2.0.0", "semver": "7.3.7", "string-width": "5.0.1", diff --git a/src/common/load-plugins.js b/src/common/load-plugins.js index 9b99926c7..37f7eb180 100644 --- a/src/common/load-plugins.js +++ b/src/common/load-plugins.js @@ -1,16 +1,13 @@ -import { createRequire } from "node:module"; import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath } from "node:url"; import fastGlob from "fast-glob"; import mem, { memClear } from "mem"; import partition from "../utils/partition.js"; -import uniqByKey from "../utils/uniq-by-key.js"; import thirdParty from "./third-party.js"; -import resolve from "./resolve.js"; +import requireFrom from "./require-from.js"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); -const require = createRequire(import.meta.url); const memoizedLoad = mem(load, { cacheKey: JSON.stringify }); const memoizedSearch = mem(findPluginsInNodeModules); @@ -43,25 +40,20 @@ async function load(plugins, pluginSearchDirs) { (plugin) => typeof plugin === "string" ); - const externalManualLoadPluginInfos = externalPluginNames.map( - (pluginName) => { - let requirePath; - try { - // try local files - requirePath = resolve(path.resolve(process.cwd(), pluginName)); - } catch { - // try node modules - requirePath = resolve(pluginName, { paths: [process.cwd()] }); - } - - return { - name: pluginName, - requirePath, - }; + const externalManualLoadPlugins = externalPluginNames.map((name) => { + let plugin; + try { + // try local files + plugin = requireFrom(path.resolve(name), process.cwd()); + } catch { + // try node modules + plugin = requireFrom(name, process.cwd()); } - ); - const externalAutoLoadPluginInfos = ( + return { name, ...plugin }; + }); + + const externalAutoLoadPlugins = ( await Promise.all( pluginSearchDirs.map(async (pluginSearchDir) => { const resolvedPluginSearchDir = path.resolve( @@ -88,28 +80,19 @@ async function load(plugins, pluginSearchDirs) { const pluginNames = await memoizedSearch(nodeModulesDir); - return pluginNames.map((pluginName) => ({ - name: pluginName, - requirePath: resolve(pluginName, { - paths: [resolvedPluginSearchDir], - }), + return pluginNames.map((name) => ({ + name, + ...requireFrom(name, nodeModulesDir), })); }) ) ).flat(); - const externalPlugins = [ - ...uniqByKey( - [...externalManualLoadPluginInfos, ...externalAutoLoadPluginInfos], - "requirePath" - ).map((externalPluginInfo) => ({ - name: externalPluginInfo.name, - ...require(externalPluginInfo.requirePath), - })), + return [ + ...externalManualLoadPlugins, + ...externalAutoLoadPlugins, ...externalPluginInstances, ]; - - return externalPlugins; } async function findPluginsInNodeModules(nodeModulesDir) { diff --git a/src/common/require-from.js b/src/common/require-from.js new file mode 100644 index 000000000..a2f8fcef2 --- /dev/null +++ b/src/common/require-from.js @@ -0,0 +1,9 @@ +import { createRequire } from "node:module"; +import path from "node:path"; + +function requireFrom(id, directory) { + const require = createRequire(path.join(directory, "noop.js")); + return require(id); +} + +export default requireFrom; diff --git a/src/common/resolve.js b/src/common/resolve.js deleted file mode 100644 index b40cf7496..000000000 --- a/src/common/resolve.js +++ /dev/null @@ -1,20 +0,0 @@ -import { createRequire } from "node:module"; - -const require = createRequire(import.meta.url); - -let { resolve } = require; - -// In the VS Code and Atom extensions `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if (resolve.length === 1 || process.env.PRETTIER_FALLBACK_RESOLVE) { - // @ts-expect-error - resolve = (id, options) => { - let basedir; - if (options && options.paths && options.paths.length === 1) { - basedir = options.paths[0]; - } - - return require("resolve").sync(id, { basedir }); - }; -} - -export default resolve; diff --git a/tests/integration/__tests__/plugin-resolution.js b/tests/integration/__tests__/plugin-resolution.js index 18dea6461..1b4dec6b3 100644 --- a/tests/integration/__tests__/plugin-resolution.js +++ b/tests/integration/__tests__/plugin-resolution.js @@ -178,6 +178,19 @@ describe("loads --plugin by relative path to its directory without leading ./ (a }); }); +describe("loads --plugin by package name", () => { + runPrettier("plugins/automatic", [ + "file.txt", + "--parser=foobar", + "--plugin=@user/prettier-plugin-foobar", + ]).test({ + stdout: "foobar+contents" + EOL, + stderr: "", + status: 0, + write: [], + }); +}); + describe("loads --plugin by filename without leading ./ and ext, should resolve to file, not package", () => { runPrettier("plugins/automatic", [ "file.txt", diff --git a/yarn.lock b/yarn.lock index a9511d933..965ead0ee 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6846,7 +6846,6 @@ __metadata: remark-footnotes: 2.0.0 remark-math: 3.0.1 remark-parse: 8.0.3 - resolve: 1.22.1 rimraf: 3.0.2 rollup-plugin-license: 2.8.1 sdbm: 2.0.0 @@ -7177,7 +7176,7 @@ __metadata: languageName: node linkType: hard -"resolve@npm:1.22.1, resolve@npm:^1.10.0, resolve@npm:^1.20.0, resolve@npm:^1.22.0": +"resolve@npm:^1.10.0, resolve@npm:^1.20.0, resolve@npm:^1.22.0": version: 1.22.1 resolution: "resolve@npm:1.22.1" dependencies: @@ -7203,7 +7202,7 @@ __metadata: languageName: node linkType: hard -"resolve@patch:resolve@1.22.1#~builtin, resolve@patch:resolve@^1.10.0#~builtin, resolve@patch:resolve@^1.20.0#~builtin, resolve@patch:resolve@^1.22.0#~builtin": +"resolve@patch:resolve@^1.10.0#~builtin, resolve@patch:resolve@^1.20.0#~builtin, resolve@patch:resolve@^1.22.0#~builtin": version: 1.22.1 resolution: "resolve@patch:resolve@npm%3A1.22.1#~builtin::version=1.22.1&hash=07638b" dependencies: