From 3af193d4662a277bcab758ab29e471a5ba36e713 Mon Sep 17 00:00:00 2001 From: Eli <88557639+lishaduck@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:27:22 -0500 Subject: [PATCH] refactor: npx replacement Closes #224. --- lib/autofix.js | 62 +--------------------------- lib/elm-binary.js | 3 +- lib/npx.js | 76 +++++++++++++++++++++++++++++++++++ test/jest-helpers/cli.js | 22 +--------- tsconfig.no-implicit-any.json | 3 +- 5 files changed, 83 insertions(+), 83 deletions(-) create mode 100644 lib/npx.js diff --git a/lib/autofix.js b/lib/autofix.js index 533520a14..f2d455478 100644 --- a/lib/autofix.js +++ b/lib/autofix.js @@ -3,7 +3,6 @@ const path = require('node:path'); const chalk = require('chalk'); const prompts = require('prompts'); const spawn = require('cross-spawn'); -const getPathKey = require('path-key'); const FS = require('./fs-wrapper'); const AppState = require('./state'); const ErrorMessage = require('./error-message'); @@ -12,6 +11,7 @@ const StyledMessage = require('./styled-message'); const {startReview} = require('./run-review'); const ProjectDependencies = require('./project-dependencies'); const exit = require('../vendor/exit'); +const {backwardsCompatiblePath, pathKey} = require('./npx'); /** * @import {App, AutofixRequest} from "./types/app" @@ -190,7 +190,7 @@ function formatFileContent(options, file, filePath) { input: file.source, env: hasElmFormatPathFlag ? process.env - : backwardsCompatibleElmFormatEnv() + : {...process.env, [pathKey]: backwardsCompatiblePath()} } ); @@ -237,61 +237,3 @@ ${result.stderr.toString()}`, source: fs.readFileSync(filePath, 'utf8') }; } - -// When `--elm-format-path` was _not_ provided, we used to execute -// `elm-format` like this: -// -// 1. Try `npx elm-format` -// 2. Try `elm-format` -// -// Just starting `npx` takes 200 ms though. Luckily `npx` isn’t even -// necessary, because the common ways of running `elm-review` are: -// -// 1. Install everything globally and run just `elm-review`. -// 2. Install everything locally and run `npx elm-review`. -// 3. Use the `--elm-format-path`. -// -// That’s also the only supported ways we have for the `elm` binary – there we -// have never tried to execute `npx elm`. -// -// `npx` adds all potential `node_modules/.bin` up to current directory to the -// beginning of PATH, for example: -// -// ❯ npx node -p 'process.env.PATH.split(require("path").delimiter)' -// [ -// '/Users/you/stuff/node_modules/.bin', -// '/Users/you/node_modules/.bin', -// '/Users/node_modules/.bin', -// '/node_modules/.bin', -// '/usr/bin', -// 'etc' -// ] -// -// So if a user runs `npx elm-review`, when we later try to spawn just -// `elm-format`, it’ll be found since when spawning we inherit the same PATH. -// -// The `npx elm-format` approach has been removed to avoid those unnessary 200 ms, -// but to stay backwards compatible we prepend the same paths to the beginning -// of PATH just like `npx` would (see above). This is needed when: -// -// - Executing `elm-review` _without_ `npx`. -// - And expecting a _local_ `elm-format` to be used. That’s an odd use case, -// but was supported due to the `npx` approach. -// -// This can be removed in a major version. -function backwardsCompatibleElmFormatEnv() { - const pathKey = getPathKey(); - return { - ...process.env, - [pathKey]: [ - ...process - .cwd() - .split(path.sep) - .map((_, index, parts) => - [...parts.slice(0, index + 1), 'node_modules', '.bin'].join(path.sep) - ) - .reverse(), - process.env[pathKey] - ].join(path.delimiter) - }; -} diff --git a/lib/elm-binary.js b/lib/elm-binary.js index 22929256b..26b0ef07e 100644 --- a/lib/elm-binary.js +++ b/lib/elm-binary.js @@ -3,6 +3,7 @@ const chalk = require('chalk'); const which = require('which'); const spawn = require('cross-spawn'); const ErrorMessage = require('./error-message'); +const {backwardsCompatiblePath} = require('./npx'); /** * @import {Options} from "./types/options" @@ -19,7 +20,7 @@ const ErrorMessage = require('./error-message'); async function getElmBinary(options) { if (options.compiler === undefined) { try { - return await which('elm'); + return await which('elm', {path: backwardsCompatiblePath()}); } catch { throw new ErrorMessage.CustomError( // prettier-ignore diff --git a/lib/npx.js b/lib/npx.js new file mode 100644 index 000000000..9956e933e --- /dev/null +++ b/lib/npx.js @@ -0,0 +1,76 @@ +/** + * A fast `npx` alternative. + */ + +const path = require('node:path'); +const getPathKey = require('path-key'); + +/** + * The key for `process.env` containing the path, generally `PATH`. + * + * @type {string} + */ +const pathKey = getPathKey(); + +/** + * Prepends `node_modules/.bin` up to the root directory to the PATH. + * + * @remarks + * When `--elm-format-path` was _not_ provided, we used to execute + * `elm-format` like this: + * + * 1. Try `npx elm-format` + * 2. Try `elm-format` + * + * Just starting `npx` takes 200 ms though. Luckily, `npx` isn’t even + * necessary, because the common ways of running `elm-review` are: + * + * 1. Install everything globally and run just `elm-review`. + * 2. Install everything locally and run `npx elm-review`. + * 3. Use the `--elm-format-path`. + * + * However, `npx` adds all potential `node_modules/.bin` up to current directory to the + * beginning of PATH, for example: + * + * ```sh + * $ npx node -p 'process.env.PATH.split(require("path").delimiter)' + * [ + * '/Users/you/stuff/node_modules/.bin', + * '/Users/you/node_modules/.bin', + * '/Users/node_modules/.bin', + * '/node_modules/.bin', + * '/usr/bin', + * 'etc' + * ] + * ``` + * + * So if a user runs `npx elm-review`, when we later try to spawn just + * `elm-format`, it’ll be found since when spawning we inherit the same PATH. + * + * The `npx elm-format` approach has been removed to avoid those unnecessary 200 ms, + * but to stay backwards compatible we prepend the same paths to the beginning + * of PATH just like `npx` would (see above). This is needed when: + * + * - Executing `elm-review` _without_ `npx`/`npm exec`, npm scripts, or `--elm-format-path`. + * - And expecting a _local_ `elm-format` to be used. + * That’s an odd use case, but was supported due to the `npx` approach. + * + * This can be removed in a major version. + */ +function backwardsCompatiblePath() { + return [ + ...process + .cwd() + .split(path.sep) + .map((_, index, parts) => + [...parts.slice(0, index + 1), 'node_modules', '.bin'].join(path.sep) + ) + .reverse(), + process.env[pathKey] + ].join(path.delimiter); +} + +module.exports = { + backwardsCompatiblePath, + pathKey +}; diff --git a/test/jest-helpers/cli.js b/test/jest-helpers/cli.js index 5708ff3f1..dff2d3fd2 100644 --- a/test/jest-helpers/cli.js +++ b/test/jest-helpers/cli.js @@ -52,14 +52,7 @@ async function internalExec(args, options = {}) { try { const result = await exec( - [ - colorFlag, - cli, - reportMode(options), - colors(options), - ...(args.includes('--compiler') ? [] : [elmPath()]), - args - ].join(' '), + [colorFlag, cli, reportMode(options), colors(options), args].join(' '), { ...options, cwd: cwdFromOptions(options) @@ -71,19 +64,6 @@ async function internalExec(args, options = {}) { } } -function elmPath() { - const elmPath = path.resolve( - __dirname, - '..', - '..', - 'node_modules', - '.bin', - 'elm' - ); - - return `--compiler=${elmPath}`; -} - /** * @param {Options} options */ diff --git a/tsconfig.no-implicit-any.json b/tsconfig.no-implicit-any.json index df0ac1d8d..54c98b98d 100644 --- a/tsconfig.no-implicit-any.json +++ b/tsconfig.no-implicit-any.json @@ -17,6 +17,7 @@ "lib/sync-get.js", "lib/sync-get-worker.js", "lib/error-message.js", + "lib/path-helpers.js", "lib/flags.js", "lib/fs-wrapper.js", "lib/hash.js", @@ -24,11 +25,11 @@ "lib/load-compiled-app.js", "lib/min-version.js", "lib/new-rule.js", + "lib/npx.js", "lib/options.js", "lib/os-helpers.js", "lib/parse-elm.js", "lib/parse-elm-worker.js", - "lib/path-helpers.js", "lib/project-json-files.js", "lib/promisify-port.js", "lib/report.js",