From c00689dad8541f47226f2bbb640f342fe9231de3 Mon Sep 17 00:00:00 2001 From: fisker Date: Fri, 17 Apr 2020 17:43:13 +0800 Subject: [PATCH 01/13] Use `resolve` insteadof `require.resolve` --- package.json | 1 + src/common/load-plugins.js | 10 ++++++---- src/common/resolve.js | 12 ------------ src/config/resolve-config.js | 7 ++++--- yarn.lock | 7 +++++++ 5 files changed, 18 insertions(+), 19 deletions(-) delete mode 100644 src/common/resolve.js diff --git a/package.json b/package.json index 40a9f6e28f13..71a7bd0a67bb 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "regexp-util": "1.2.2", "remark-math": "1.0.6", "remark-parse": "5.0.0", + "resolve": "1.16.0", "semver": "7.3.2", "srcset": "2.0.1", "string-width": "4.2.0", diff --git a/src/common/load-plugins.js b/src/common/load-plugins.js index 7cc20403c440..caf6d44a2ba4 100644 --- a/src/common/load-plugins.js +++ b/src/common/load-plugins.js @@ -5,10 +5,10 @@ const partition = require("lodash/partition"); const fs = require("fs"); const globby = require("globby"); const path = require("path"); +const resolve = require("resolve"); const thirdParty = require("./third-party"); const internalPlugins = require("./internal-plugins"); const mem = require("mem"); -const resolve = require("./resolve"); const memoizedLoad = mem(load, { cacheKey: JSON.stringify }); const memoizedSearch = mem(findPluginsInNodeModules); @@ -43,10 +43,10 @@ function load(plugins, pluginSearchDirs) { let requirePath; try { // try local files - requirePath = resolve(path.resolve(process.cwd(), pluginName)); + requirePath = resolve.sync(path.resolve(process.cwd(), pluginName)); } catch (_) { // try node modules - requirePath = resolve(pluginName, { paths: [process.cwd()] }); + requirePath = resolve.sync(pluginName, { basedir: process.cwd() }); } return { @@ -82,7 +82,9 @@ function load(plugins, pluginSearchDirs) { return memoizedSearch(nodeModulesDir).map((pluginName) => ({ name: pluginName, - requirePath: resolve(pluginName, { paths: [resolvedPluginSearchDir] }), + requirePath: resolve.sync(pluginName, { + basedir: resolvedPluginSearchDir, + }), })); }) .reduce((a, b) => a.concat(b), []); diff --git a/src/common/resolve.js b/src/common/resolve.js deleted file mode 100644 index 183b59429766..000000000000 --- a/src/common/resolve.js +++ /dev/null @@ -1,12 +0,0 @@ -"use strict"; - -let { resolve } = eval("require"); - -// In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if (resolve.length === 1) { - const Module = eval("require")("module"); - const createRequire = Module.createRequire || Module.createRequireFromPath; - resolve = createRequire(__dirname).resolve; -} - -module.exports = resolve; diff --git a/src/config/resolve-config.js b/src/config/resolve-config.js index c32067876d34..f56c43de0800 100644 --- a/src/config/resolve-config.js +++ b/src/config/resolve-config.js @@ -2,12 +2,12 @@ const thirdParty = require("../common/third-party"); const minimatch = require("minimatch"); +const resolve = require("resolve"); const path = require("path"); const mem = require("mem"); const resolveEditorConfig = require("./resolve-config-editorconfig"); const loadToml = require("../utils/load-toml"); -const resolve = require("../common/resolve"); const getExplorerMemoized = mem( (opts) => { @@ -17,8 +17,9 @@ const getExplorerMemoized = mem( transform: (result) => { if (result && result.config) { if (typeof result.config === "string") { - const dir = path.dirname(result.filepath); - const modulePath = resolve(result.config, { paths: [dir] }); + const modulePath = resolve.sync(result.config, { + basedir: path.dirname(result.filepath), + }); result.config = eval("require")(modulePath); } diff --git a/yarn.lock b/yarn.lock index 333476bc5cba..70cfa4668c06 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6463,6 +6463,13 @@ resolve@1.1.7: resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.1.7.tgz#203114d82ad2c5ed9e8e0411b3932875e889e97b" integrity sha1-IDEU2CrSxe2ejgQRs5ModeiJ6Xs= +resolve@1.16.0: + version "1.16.0" + resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.16.0.tgz#063dc704fa3413e13ac1d0d1756a7cbfe95dd1a7" + integrity sha512-LarL/PIKJvc09k1jaeT4kQb/8/7P+qV4qSnN2K80AES+OHdfZELAKVOBjxsvtToT/uLOfFbvYvKfZmV8cee7nA== + dependencies: + path-parse "^1.0.6" + resolve@^1.1.6, resolve@^1.10.0, resolve@^1.11.0, resolve@^1.12.0, resolve@^1.13.1, resolve@^1.14.2, resolve@^1.15.1, resolve@^1.3.2: version "1.15.1" resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.15.1.tgz#27bdcdeffeaf2d6244b95bb0f9f4b4653451f3e8" From 157e90ef7a05ac65ef3796f77df3350c8f72adf3 Mon Sep 17 00:00:00 2001 From: fisker Date: Fri, 17 Apr 2020 18:03:29 +0800 Subject: [PATCH 02/13] Use `resolve` as fallback --- src/common/load-plugins.js | 10 ++++------ src/common/resolve.js | 19 +++++++++++++++++++ src/config/resolve-config.js | 7 +++---- 3 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 src/common/resolve.js diff --git a/src/common/load-plugins.js b/src/common/load-plugins.js index caf6d44a2ba4..7cc20403c440 100644 --- a/src/common/load-plugins.js +++ b/src/common/load-plugins.js @@ -5,10 +5,10 @@ const partition = require("lodash/partition"); const fs = require("fs"); const globby = require("globby"); const path = require("path"); -const resolve = require("resolve"); const thirdParty = require("./third-party"); const internalPlugins = require("./internal-plugins"); const mem = require("mem"); +const resolve = require("./resolve"); const memoizedLoad = mem(load, { cacheKey: JSON.stringify }); const memoizedSearch = mem(findPluginsInNodeModules); @@ -43,10 +43,10 @@ function load(plugins, pluginSearchDirs) { let requirePath; try { // try local files - requirePath = resolve.sync(path.resolve(process.cwd(), pluginName)); + requirePath = resolve(path.resolve(process.cwd(), pluginName)); } catch (_) { // try node modules - requirePath = resolve.sync(pluginName, { basedir: process.cwd() }); + requirePath = resolve(pluginName, { paths: [process.cwd()] }); } return { @@ -82,9 +82,7 @@ function load(plugins, pluginSearchDirs) { return memoizedSearch(nodeModulesDir).map((pluginName) => ({ name: pluginName, - requirePath: resolve.sync(pluginName, { - basedir: resolvedPluginSearchDir, - }), + requirePath: resolve(pluginName, { paths: [resolvedPluginSearchDir] }), })); }) .reduce((a, b) => a.concat(b), []); diff --git a/src/common/resolve.js b/src/common/resolve.js new file mode 100644 index 000000000000..7f4caeb390f2 --- /dev/null +++ b/src/common/resolve.js @@ -0,0 +1,19 @@ +"use strict"; + +const resolve = require("resolve"); + +const { resolve: nativeResolve } = eval("require"); + +// In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. +if (nativeResolve.length === 1) { + module.exports = (id, options) => { + let baseDir; + if (options && options.paths && options.paths.length === 1) { + baseDir = options.paths[0]; + } + + return resolve.sync(id, { baseDir }); + }; +} else { + module.exports = nativeResolve; +} diff --git a/src/config/resolve-config.js b/src/config/resolve-config.js index f56c43de0800..c32067876d34 100644 --- a/src/config/resolve-config.js +++ b/src/config/resolve-config.js @@ -2,12 +2,12 @@ const thirdParty = require("../common/third-party"); const minimatch = require("minimatch"); -const resolve = require("resolve"); const path = require("path"); const mem = require("mem"); const resolveEditorConfig = require("./resolve-config-editorconfig"); const loadToml = require("../utils/load-toml"); +const resolve = require("../common/resolve"); const getExplorerMemoized = mem( (opts) => { @@ -17,9 +17,8 @@ const getExplorerMemoized = mem( transform: (result) => { if (result && result.config) { if (typeof result.config === "string") { - const modulePath = resolve.sync(result.config, { - basedir: path.dirname(result.filepath), - }); + const dir = path.dirname(result.filepath); + const modulePath = resolve(result.config, { paths: [dir] }); result.config = eval("require")(modulePath); } From d06c42ec57a5dc60b704ed18be181797ec1e2d40 Mon Sep 17 00:00:00 2001 From: fisker Date: Fri, 17 Apr 2020 18:06:19 +0800 Subject: [PATCH 03/13] Lazy load --- src/common/resolve.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index 7f4caeb390f2..b0cf09c0a59c 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -1,19 +1,17 @@ "use strict"; -const resolve = require("resolve"); - -const { resolve: nativeResolve } = eval("require"); +let { resolve } = eval("require"); // In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if (nativeResolve.length === 1) { - module.exports = (id, options) => { +if (resolve.length === 1) { + resolve = (id, options) => { let baseDir; if (options && options.paths && options.paths.length === 1) { baseDir = options.paths[0]; } - return resolve.sync(id, { baseDir }); + return require("resolve").sync(id, { baseDir }); }; -} else { - module.exports = nativeResolve; } + +module.exports = resolve; From 8ab88f4797af89d7c6676ccab2836199d5d126bd Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 17 Apr 2020 19:56:24 +0800 Subject: [PATCH 04/13] Update resolve.js Coding on cellphone :) --- src/common/resolve.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index b0cf09c0a59c..8ad84dc912b0 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -5,12 +5,12 @@ let { resolve } = eval("require"); // In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. if (resolve.length === 1) { resolve = (id, options) => { - let baseDir; + let basedir; if (options && options.paths && options.paths.length === 1) { - baseDir = options.paths[0]; + basedir = options.paths[0]; } - return require("resolve").sync(id, { baseDir }); + return require("resolve").sync(id, { basedir }); }; } From cc574bb7b7b4b4df9dd2ce2e114bb345351787b2 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 17 Apr 2020 21:49:55 +0800 Subject: [PATCH 05/13] Disable length check to run tests --- src/common/resolve.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index 8ad84dc912b0..44532fcfabdf 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -3,7 +3,7 @@ let { resolve } = eval("require"); // In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if (resolve.length === 1) { +// if (resolve.length === 1) { resolve = (id, options) => { let basedir; if (options && options.paths && options.paths.length === 1) { @@ -12,6 +12,6 @@ if (resolve.length === 1) { return require("resolve").sync(id, { basedir }); }; -} +// } module.exports = resolve; From eec17f570af0dc13b1d340a6687a6b2204abc14d Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 18 Apr 2020 02:28:09 +0800 Subject: [PATCH 06/13] Try to test --- .github/workflows/dev-test.yml | 6 ++++++ .github/workflows/prod-test.yml | 7 +++++++ src/common/resolve.js | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/dev-test.yml b/.github/workflows/dev-test.yml index a8f9464c298d..b5f762bdfb3d 100644 --- a/.github/workflows/dev-test.yml +++ b/.github/workflows/dev-test.yml @@ -60,6 +60,12 @@ jobs: if: matrix.os != 'macos-latest' run: yarn test --maxWorkers=2 + # #8073 test + - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) + run: yarn test "tests_integration/__tests__/{config,plugin}-*.js" + env: + PRETTIER_FALLBACK_RESOLVE: true + - name: Upload Coverage uses: codecov/codecov-action@v1 if: matrix.ENABLE_CODE_COVERAGE diff --git a/.github/workflows/prod-test.yml b/.github/workflows/prod-test.yml index d84ca7a1bd88..32fe9e7f7d97 100644 --- a/.github/workflows/prod-test.yml +++ b/.github/workflows/prod-test.yml @@ -131,3 +131,10 @@ jobs: - name: Run Tests (standalone) (Linux and Windows) if: matrix.os != 'macos-latest' run: yarn test:dist-standalone --maxWorkers=2 + + # #8073 test + - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) + run: yarn test "tests_integration/__tests__/{config,plugin}-*.js" + env: + NODE_ENV: production + PRETTIER_FALLBACK_RESOLVE: true diff --git a/src/common/resolve.js b/src/common/resolve.js index 44532fcfabdf..b8e3999772cb 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -3,7 +3,7 @@ let { resolve } = eval("require"); // In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -// if (resolve.length === 1) { +if (resolve.length === 1 || process.env.PRETTIER_FALLBACK_RESOLVE) { resolve = (id, options) => { let basedir; if (options && options.paths && options.paths.length === 1) { @@ -12,6 +12,6 @@ let { resolve } = eval("require"); return require("resolve").sync(id, { basedir }); }; -// } +} module.exports = resolve; From b35f7cf0a5e5076ea78bd5f0b2e47eaa9cf27be1 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 18 Apr 2020 02:33:08 +0800 Subject: [PATCH 07/13] istanbul ignore --- src/common/resolve.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index b8e3999772cb..9990424763ce 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -3,7 +3,11 @@ let { resolve } = eval("require"); // In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if (resolve.length === 1 || process.env.PRETTIER_FALLBACK_RESOLVE) { +if ( + /* istanbul ignore next */ + resolve.length === 1 || + process.env.PRETTIER_FALLBACK_RESOLVE +) { resolve = (id, options) => { let basedir; if (options && options.paths && options.paths.length === 1) { From e13a4694288c8b737a15b9133ebd0ba1f5929048 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 18 Apr 2020 02:34:09 +0800 Subject: [PATCH 08/13] Comment --- src/common/resolve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index 9990424763ce..6650644d2b1b 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -2,7 +2,7 @@ let { resolve } = eval("require"); -// In the VS Code extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. +// In the VS Code and Atom extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. if ( /* istanbul ignore next */ resolve.length === 1 || From bf560322c9730bd6bd880deb757dd1c0ab95df67 Mon Sep 17 00:00:00 2001 From: fisker Date: Sat, 18 Apr 2020 02:51:39 +0800 Subject: [PATCH 09/13] Fix argument --- .github/workflows/dev-test.yml | 2 +- .github/workflows/prod-test.yml | 2 +- src/common/resolve.js | 6 +----- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.github/workflows/dev-test.yml b/.github/workflows/dev-test.yml index b5f762bdfb3d..f85365c02377 100644 --- a/.github/workflows/dev-test.yml +++ b/.github/workflows/dev-test.yml @@ -62,7 +62,7 @@ jobs: # #8073 test - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) - run: yarn test "tests_integration/__tests__/{config,plugin}-*.js" + run: yarn test "tests_integration/__tests__/(config|plugin)" env: PRETTIER_FALLBACK_RESOLVE: true diff --git a/.github/workflows/prod-test.yml b/.github/workflows/prod-test.yml index 32fe9e7f7d97..b4a19447c082 100644 --- a/.github/workflows/prod-test.yml +++ b/.github/workflows/prod-test.yml @@ -134,7 +134,7 @@ jobs: # #8073 test - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) - run: yarn test "tests_integration/__tests__/{config,plugin}-*.js" + run: yarn test "tests_integration/__tests__/(config|plugin)" env: NODE_ENV: production PRETTIER_FALLBACK_RESOLVE: true diff --git a/src/common/resolve.js b/src/common/resolve.js index 6650644d2b1b..2c5093cd76bc 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -3,11 +3,7 @@ let { resolve } = eval("require"); // In the VS Code and Atom extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. -if ( - /* istanbul ignore next */ - resolve.length === 1 || - process.env.PRETTIER_FALLBACK_RESOLVE -) { +if (resolve.length === 1 || process.env.PRETTIER_FALLBACK_RESOLVE) { resolve = (id, options) => { let basedir; if (options && options.paths && options.paths.length === 1) { From 0f0e8ce3189b3bebf98d43dd34803968d36df9a4 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Sat, 18 Apr 2020 12:45:28 +0800 Subject: [PATCH 10/13] Try upload coverage separately --- .github/workflows/dev-test.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/dev-test.yml b/.github/workflows/dev-test.yml index f85365c02377..a04bfe9bc1f7 100644 --- a/.github/workflows/dev-test.yml +++ b/.github/workflows/dev-test.yml @@ -60,13 +60,21 @@ jobs: if: matrix.os != 'macos-latest' run: yarn test --maxWorkers=2 + - name: Upload Coverage + uses: codecov/codecov-action@v1 + if: matrix.ENABLE_CODE_COVERAGE + with: + token: ${{ secrets.CODECOV_TOKEN }} + file: ./coverage/lcov.info + fail_ci_if_error: true + # #8073 test - name: Run Tests (PRETTIER_FALLBACK_RESOLVE) run: yarn test "tests_integration/__tests__/(config|plugin)" env: PRETTIER_FALLBACK_RESOLVE: true - - name: Upload Coverage + - name: Upload Coverage (PRETTIER_FALLBACK_RESOLVE) uses: codecov/codecov-action@v1 if: matrix.ENABLE_CODE_COVERAGE with: From 7d741b2bb60bb72d193ede7f75eecc1b09b95362 Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Mon, 20 Apr 2020 16:05:33 +0300 Subject: [PATCH 11/13] Update src/common/resolve.js --- src/common/resolve.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index 2c5093cd76bc..73142bcabf47 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -1,6 +1,6 @@ "use strict"; -let { resolve } = eval("require"); +let resolve = eval("require").resolve; // In the VS Code and Atom extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. if (resolve.length === 1 || process.env.PRETTIER_FALLBACK_RESOLVE) { From 61b3d4db54e3d4a9ebd0fba5a1330f0eae15d53b Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Mon, 20 Apr 2020 16:28:13 +0300 Subject: [PATCH 12/13] add eslint-disable --- src/common/resolve.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/resolve.js b/src/common/resolve.js index 73142bcabf47..b8c8a42ec607 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -1,5 +1,6 @@ "use strict"; +// eslint-disable-next-line prefer-destructuring let resolve = eval("require").resolve; // In the VS Code and Atom extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. From a5950e820de5fd733d40a0f63b3919cd4eeedcd1 Mon Sep 17 00:00:00 2001 From: Georgii Dolzhykov Date: Mon, 20 Apr 2020 20:36:46 +0300 Subject: [PATCH 13/13] add comment --- src/common/resolve.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/resolve.js b/src/common/resolve.js index b8c8a42ec607..f413d1cae388 100644 --- a/src/common/resolve.js +++ b/src/common/resolve.js @@ -1,9 +1,10 @@ "use strict"; +// `/scripts/build/babel-plugins/transform-custom-require.js` doesn't support destructuring. // eslint-disable-next-line prefer-destructuring let resolve = eval("require").resolve; -// In the VS Code and Atom extension `require` is overridden and `require.resolve` doesn't support the 2nd argument. +// 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) { resolve = (id, options) => { let basedir;