From 00c64ce7d923c02c25c7aa1539e454abfb3a529d Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Mon, 24 Jul 2023 13:09:40 +0200 Subject: [PATCH 1/3] Remove cross-spawn as a dependency --- package-lock.json | 25 ++++++++++++++++++++----- package.json | 2 +- src/Spawn.ts | 40 +++++++++++++++++++++++++++------------- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2ec1179a..0d1232a9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,7 +8,6 @@ "hasInstallScript": true, "dependencies": { "chokidar": "^3.5.3", - "cross-spawn": "^7.0.3", "tiny-decoders": "^7.0.1", "ws": "^8.12.0" }, @@ -19,6 +18,7 @@ "@types/ws": "8.5.4", "@typescript-eslint/eslint-plugin": "5.48.2", "@typescript-eslint/parser": "5.48.2", + "cross-spawn": "7.0.3", "elm-tooling": "1.12.0", "esbuild": "0.15.18", "esbuild-register": "3.4.2", @@ -2025,7 +2025,9 @@ }, "node_modules/cross-spawn": { "version": "7.0.3", - "license": "MIT", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", + "integrity": "sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==", + "dev": true, "dependencies": { "path-key": "^3.1.0", "shebang-command": "^2.0.0", @@ -3605,6 +3607,7 @@ }, "node_modules/isexe": { "version": "2.0.0", + "dev": true, "license": "ISC" }, "node_modules/istanbul-lib-coverage": { @@ -4778,6 +4781,7 @@ }, "node_modules/path-key": { "version": "3.1.1", + "dev": true, "license": "MIT", "engines": { "node": ">=8" @@ -5139,6 +5143,7 @@ }, "node_modules/shebang-command": { "version": "2.0.0", + "dev": true, "license": "MIT", "dependencies": { "shebang-regex": "^3.0.0" @@ -5149,6 +5154,7 @@ }, "node_modules/shebang-regex": { "version": "3.0.0", + "dev": true, "license": "MIT", "engines": { "node": ">=8" @@ -5628,6 +5634,7 @@ }, "node_modules/which": { "version": "2.0.2", + "dev": true, "license": "ISC", "dependencies": { "isexe": "^2.0.0" @@ -7266,6 +7273,9 @@ }, "cross-spawn": { "version": "7.0.3", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", + "integrity": "sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==", + "dev": true, "requires": { "path-key": "^3.1.0", "shebang-command": "^2.0.0", @@ -8305,7 +8315,8 @@ "dev": true }, "isexe": { - "version": "2.0.0" + "version": "2.0.0", + "dev": true }, "istanbul-lib-coverage": { "version": "3.2.0", @@ -9196,7 +9207,8 @@ "dev": true }, "path-key": { - "version": "3.1.1" + "version": "3.1.1", + "dev": true }, "path-parse": { "version": "1.0.7", @@ -9433,12 +9445,14 @@ }, "shebang-command": { "version": "2.0.0", + "dev": true, "requires": { "shebang-regex": "^3.0.0" } }, "shebang-regex": { - "version": "3.0.0" + "version": "3.0.0", + "dev": true }, "signal-exit": { "version": "3.0.7", @@ -9766,6 +9780,7 @@ }, "which": { "version": "2.0.2", + "dev": true, "requires": { "isexe": "^2.0.0" } diff --git a/package.json b/package.json index aa3e33f7..2c3df25f 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "@types/ws": "8.5.4", "@typescript-eslint/eslint-plugin": "5.48.2", "@typescript-eslint/parser": "5.48.2", + "cross-spawn": "7.0.3", "elm-tooling": "1.12.0", "esbuild": "0.15.18", "esbuild-register": "3.4.2", @@ -31,7 +32,6 @@ }, "dependencies": { "chokidar": "^3.5.3", - "cross-spawn": "^7.0.3", "tiny-decoders": "^7.0.1", "ws": "^8.12.0" } diff --git a/src/Spawn.ts b/src/Spawn.ts index 26c02fc7..eeb1e45b 100644 --- a/src/Spawn.ts +++ b/src/Spawn.ts @@ -41,7 +41,7 @@ export type Command = { stdin?: Buffer | string; }; -export function spawn(command: Command): { +export function spawn(passedCommand: Command): { promise: Promise; kill: () => void; } { @@ -53,16 +53,17 @@ export function spawn(command: Command): { }; const promise = ( - actualSpawn: typeof childProcess.spawn + command: Command, + previousAttemptError?: Error ): Promise => - new Promise((resolve) => { + new Promise((resolve, reject) => { // istanbul ignore if if (killed) { resolve({ tag: "Killed", command }); return; } - const child = actualSpawn(command.command, command.args, { + const child = childProcess.spawn(command.command, command.args, { ...command.options, cwd: command.options.cwd.absolutePath, }); @@ -71,12 +72,27 @@ export function spawn(command: Command): { const stderr: Array = []; child.on("error", (error: Error & { code?: string }) => { - resolve( - // istanbul ignore next - error.code === "ENOENT" - ? { tag: "CommandNotFoundError", command } - : { tag: "OtherSpawnError", error, command } - ); + // istanbul ignore else + if (error.code === "ENOENT") { + // istanbul ignore if + if (IS_WINDOWS && previousAttemptError === undefined) { + // On Windows, executing just `elm` works for `elm.exe`, + // but not for `elm.cmd` – then we need to explicitly say + // `.cmd`. When installing Elm via npm or elm-tooling a + // `.cmd` file is used (pointing to the `.exe` somewhere else). + promise({ ...command, command: `${command.command}.cmd` }, error) + .then(resolve) + .catch(reject); + } else { + resolve({ tag: "CommandNotFoundError", command }); + } + } else { + resolve({ + tag: "OtherSpawnError", + error: previousAttemptError ?? error, + command, + }); + } }); let stdinWriteError: @@ -167,9 +183,7 @@ export function spawn(command: Command): { // istanbul ignore next return { - promise: IS_WINDOWS - ? import("cross-spawn").then((crossSpawn) => promise(crossSpawn.spawn)) - : promise(childProcess.spawn), + promise: promise(passedCommand), kill: () => { kill(); }, From 1482bed34af466df4e503db6d06b5ed2b401b255 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Tue, 25 Jul 2023 09:04:32 +0200 Subject: [PATCH 2/3] Fix wrong printed command on Windows --- src/Spawn.ts | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/Spawn.ts b/src/Spawn.ts index eeb1e45b..98e5d904 100644 --- a/src/Spawn.ts +++ b/src/Spawn.ts @@ -41,7 +41,7 @@ export type Command = { stdin?: Buffer | string; }; -export function spawn(passedCommand: Command): { +export function spawn(command: Command): { promise: Promise; kill: () => void; } { @@ -52,10 +52,7 @@ export function spawn(passedCommand: Command): { killed = true; }; - const promise = ( - command: Command, - previousAttemptError?: Error - ): Promise => + const promise = (windowsPreviousAttemptError?: Error): Promise => new Promise((resolve, reject) => { // istanbul ignore if if (killed) { @@ -63,10 +60,22 @@ export function spawn(passedCommand: Command): { return; } - const child = childProcess.spawn(command.command, command.args, { - ...command.options, - cwd: command.options.cwd.absolutePath, - }); + const child = childProcess.spawn( + // On Windows, executing just `elm` works for `elm.exe`, but not for + // `elm.cmd` – then we need to explicitly say `.cmd`. When installing + // Elm via npm or elm-tooling a `.cmd` file is used (pointing to the + // `.exe` somewhere else). So we try first with the original command, + // and then with `.cmd` appended. + // istanbul ignore next + windowsPreviousAttemptError === undefined + ? command.command + : `${command.command}.cmd`, + command.args, + { + ...command.options, + cwd: command.options.cwd.absolutePath, + } + ); const stdout: Array = []; const stderr: Array = []; @@ -75,21 +84,15 @@ export function spawn(passedCommand: Command): { // istanbul ignore else if (error.code === "ENOENT") { // istanbul ignore if - if (IS_WINDOWS && previousAttemptError === undefined) { - // On Windows, executing just `elm` works for `elm.exe`, - // but not for `elm.cmd` – then we need to explicitly say - // `.cmd`. When installing Elm via npm or elm-tooling a - // `.cmd` file is used (pointing to the `.exe` somewhere else). - promise({ ...command, command: `${command.command}.cmd` }, error) - .then(resolve) - .catch(reject); + if (IS_WINDOWS && windowsPreviousAttemptError === undefined) { + promise(error).then(resolve).catch(reject); } else { resolve({ tag: "CommandNotFoundError", command }); } } else { resolve({ tag: "OtherSpawnError", - error: previousAttemptError ?? error, + error: windowsPreviousAttemptError ?? error, command, }); } @@ -183,7 +186,7 @@ export function spawn(passedCommand: Command): { // istanbul ignore next return { - promise: promise(passedCommand), + promise: promise(), kill: () => { kill(); }, From f6479cd4abfa95c7ae1abf3c1400541f4c2217de Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Tue, 25 Jul 2023 09:14:07 +0200 Subject: [PATCH 3/3] Fix coverage --- src/Spawn.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Spawn.ts b/src/Spawn.ts index 98e5d904..a6112be2 100644 --- a/src/Spawn.ts +++ b/src/Spawn.ts @@ -80,10 +80,11 @@ export function spawn(command: Command): { const stdout: Array = []; const stderr: Array = []; + // Having a test for `CommandNotFoundError` is good, but the rest is not + // possible to test. It’s the least messy to ignore coverage for the whole thing. + // istanbul ignore next child.on("error", (error: Error & { code?: string }) => { - // istanbul ignore else if (error.code === "ENOENT") { - // istanbul ignore if if (IS_WINDOWS && windowsPreviousAttemptError === undefined) { promise(error).then(resolve).catch(reject); } else {