Repository: sindresorhus/execa
Pull request #303 (2019-06-19 12:07:01)

Refactor `stdio` option for `execa.node()`
This is a refactoring of the `stdio` option handling for `execa.node()`. 

It also fixes one bug: `undefined` descriptors beyond the first three were converted to `pipe`. Those should be kept as `undefined` instead. See [Node.js documentation](https://nodejs.org/api/child_process.html#child_process_options_stdio):

```
null, undefined - Use default value. For stdio fds 0, 1, and 2 (in other words, stdin, stdout, and stderr) a pipe is created. For fd 3 and up, the default is 'ignore'.
```

This does not change behavior otherwise. 

Some code comments and tests are added too.

@GMartigny 	


Patch:
diff --git a/index.d.ts b/index.d.ts
index 9a21846..6a7e21e 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -436,6 +436,11 @@ declare const execa: {
 	/**
 	Execute a Node.js script as a child process.
 
+	Same as `execa('node', [scriptPath, ...arguments], options)` except (like [`child_process#fork()`](https://nodejs.org/api/child_process.html#child_process_child_process_fork_modulepath_args_options)):
+		- the current Node version and options are used. This can be overridden using the `nodePath` and `nodeArguments` options.
+		- the `shell` option cannot be used
+		- an extra channel [`ipc`](https://nodejs.org/api/child_process.html#child_process_options_stdio) is passed to [`stdio`](#stdio)
+
 	@param scriptPath - Node.js script to execute.
 	@param arguments - Arguments to pass to `scriptPath` on execution.
 	@returns A [`child_process` instance](https://nodejs.org/api/child_process.html#child_process_class_childprocess), which is enhanced to also be a `Promise` for a result `Object` with `stdout` and `stderr` properties.
@@ -446,12 +451,12 @@ declare const execa: {
 		options?: execa.NodeOptions
 	): execa.ExecaChildProcess;
 	node(
-		file: string,
+		scriptPath: string,
 		arguments?: readonly string[],
 		options?: execa.Options<null>
 	): execa.ExecaChildProcess<Buffer>;
-	node(file: string, options?: execa.Options): execa.ExecaChildProcess;
-	node(file: string, options?: execa.Options<null>): execa.ExecaChildProcess<Buffer>;
+	node(scriptPath: string, options?: execa.Options): execa.ExecaChildProcess;
+	node(scriptPath: string, options?: execa.Options<null>): execa.ExecaChildProcess<Buffer>;
 };
 
 export = execa;
diff --git a/index.js b/index.js
index 8af6259..3d78cf7 100644
--- a/index.js
+++ b/index.js
@@ -18,7 +18,7 @@ const DEFAULT_FORCE_KILL_TIMEOUT = 1000 * 5;
 
 const SPACES_REGEXP = / +/g;
 
-function handleArgs(file, args, options = {}) {
+const handleArgs = (file, args, options = {}) => {
 	const parsed = crossSpawn._parse(file, args, options);
 	file = parsed.command;
 	args = parsed.args;
@@ -59,9 +59,9 @@ function handleArgs(file, args, options = {}) {
 	}
 
 	return {file, args, options, parsed};
-}
+};
 
-function handleInput(spawned, input) {
+const handleInput = (spawned, input) => {
 	// Checking for stdin is workaround for https://github.com/nodejs/node/issues/26852
 	// TODO: Remove `|| spawned.stdin === undefined` once we drop support for Node.js <=12.2.0
 	if (input === undefined || spawned.stdin === undefined) {
@@ -73,9 +73,9 @@ function handleInput(spawned, input) {
 	} else {
 		spawned.stdin.end(input);
 	}
-}
+};
 
-function handleOutput(options, value, error) {
+const handleOutput = (options, value, error) => {
 	if (typeof value !== 'string' && !Buffer.isBuffer(value)) {
 		// When `execa.sync()` errors, we normalize it to '' to mimic `execa()`
 		return error === undefined ? undefined : '';
@@ -86,9 +86,9 @@ function handleOutput(options, value, error) {
 	}
 
 	return value;
-}
+};
 
-function makeAllStream(spawned) {
+const makeAllStream = spawned => {
 	if (!spawned.stdout && !spawned.stderr) {
 		return;
 	}
@@ -104,9 +104,9 @@ function makeAllStream(spawned) {
 	}
 
 	return mixed;
-}
+};
 
-async function getBufferedData(stream, streamPromise) {
+const getBufferedData = async (stream, streamPromise) => {
 	if (!stream) {
 		return;
 	}
@@ -118,9 +118,9 @@ async function getBufferedData(stream, streamPromise) {
 	} catch (error) {
 		return error.bufferedData;
 	}
-}
+};
 
-function getStreamPromise(stream, {encoding, buffer, maxBuffer}) {
+const getStreamPromise = (stream, {encoding, buffer, maxBuffer}) => {
 	if (!stream) {
 		return;
 	}
@@ -139,9 +139,9 @@ function getStreamPromise(stream, {encoding, buffer, maxBuffer}) {
 	}
 
 	return getStream.buffer(stream, {maxBuffer});
-}
+};
 
-async function getPromiseResult({stdout, stderr, all}, {encoding, buffer, maxBuffer}, processDone) {
+const getPromiseResult = async ({stdout, stderr, all}, {encoding, buffer, maxBuffer}, processDone) => {
 	const stdoutPromise = getStreamPromise(stdout, {encoding, buffer, maxBuffer});
 	const stderrPromise = getStreamPromise(stderr, {encoding, buffer, maxBuffer});
 	const allPromise = getStreamPromise(all, {encoding, buffer, maxBuffer: maxBuffer * 2});
@@ -156,9 +156,9 @@ async function getPromiseResult({stdout, stderr, all}, {encoding, buffer, maxBuf
 			getBufferedData(all, allPromise)
 		]);
 	}
-}
+};
 
-function makeError(result, options) {
+const makeError = (result, options) => {
 	const {stdout, stderr, signal} = result;
 	let {error} = result;
 	const {code, command, timedOut, isCanceled, killed, parsed: {options: {timeout}}} = options;
@@ -198,9 +198,9 @@ function makeError(result, options) {
 	error.signal = signal || undefined;
 
 	return error;
-}
+};
 
-function getCode({error = {}}, code) {
+const getCode = ({error = {}}, code) => {
 	if (error.code) {
 		return [error.code, os.constants.errno[error.code]];
 	}
@@ -210,9 +210,9 @@ function getCode({error = {}}, code) {
 	}
 
 	return [];
-}
+};
 
-function getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}) {
+const getErrorPrefix = ({timedOut, timeout, signal, exitCodeName, exitCode, isCanceled}) => {
 	if (timedOut) {
 		return `timed out after ${timeout} milliseconds`;
 	}
@@ -230,17 +230,17 @@ function getErrorPrefix({timedOut, timeout, signal, exitCodeName, exitCode, isCa
 	}
 
 	return 'failed';
-}
+};
 
-function joinCommand(file, args = []) {
+const joinCommand = (file, args = []) => {
 	if (!Array.isArray(args)) {
 		return file;
 	}
 
 	return [file, ...args].join(' ');
-}
+};
 
-function mergePromiseProperty(spawned, getPromise, property) {
+const mergePromiseProperty = (spawned, getPromise, property) => {
 	Object.defineProperty(spawned, property, {
 		value(...args) {
 			return getPromise()[property](...args);
@@ -249,10 +249,10 @@ function mergePromiseProperty(spawned, getPromise, property) {
 		enumerable: false,
 		configurable: true
 	});
-}
+};
 
 // The return value is a mixin of `childProcess` and `Promise`
-function mergePromise(spawned, getPromise) {
+const mergePromise = (spawned, getPromise) => {
 	mergePromiseProperty(spawned, getPromise, 'then');
 	mergePromiseProperty(spawned, getPromise, 'catch');
 
@@ -262,15 +262,15 @@ function mergePromise(spawned, getPromise) {
 	}
 
 	return spawned;
-}
+};
 
-function spawnedKill(kill, signal = 'SIGTERM', options = {}) {
+const spawnedKill = (kill, signal = 'SIGTERM', options = {}) => {
 	const killResult = kill(signal);
 	setKillTimeout(kill, signal, options, killResult);
 	return killResult;
-}
+};
 
-function setKillTimeout(kill, signal, options, killResult) {
+const setKillTimeout = (kill, signal, options, killResult) => {
 	if (!shouldForceKill(signal, options, killResult)) {
 		return;
 	}
@@ -279,18 +279,18 @@ function setKillTimeout(kill, signal, options, killResult) {
 	setTimeout(() => {
 		kill('SIGKILL');
 	}, timeout).unref();
-}
+};
 
-function shouldForceKill(signal, {forceKillAfterTimeout}, killResult) {
+const shouldForceKill = (signal, {forceKillAfterTimeout}, killResult) => {
 	return isSigterm(signal) && forceKillAfterTimeout !== false && killResult;
-}
+};
 
-function isSigterm(signal) {
+const isSigterm = signal => {
 	return signal === os.constants.signals.SIGTERM ||
 		(typeof signal === 'string' && signal.toUpperCase() === 'SIGTERM');
-}
+};
 
-function getForceKillAfterTimeout({forceKillAfterTimeout = true}) {
+const getForceKillAfterTimeout = ({forceKillAfterTimeout = true}) => {
 	if (forceKillAfterTimeout === true) {
 		return DEFAULT_FORCE_KILL_TIMEOUT;
 	}
@@ -300,9 +300,9 @@ function getForceKillAfterTimeout({forceKillAfterTimeout = true}) {
 	}
 
 	return forceKillAfterTimeout;
-}
+};
 
-function handleSpawned(spawned, context) {
+const handleSpawned = (spawned, context) => {
 	return new Promise((resolve, reject) => {
 		spawned.on('exit', (code, signal) => {
 			if (context.timedOut) {
@@ -323,7 +323,7 @@ function handleSpawned(spawned, context) {
 			});
 		}
 	});
-}
+};
 
 const setupTimeout = (spawned, {timeout, killSignal}, context) => {
 	if (timeout > 0) {
@@ -492,7 +492,7 @@ module.exports.sync = (file, args, options) => {
 };
 
 // Allow spaces to be escaped by a backslash if not meant as a delimiter
-function handleEscaping(tokens, token, index) {
+const handleEscaping = (tokens, token, index) => {
 	if (index === 0) {
 		return [token];
 	}
@@ -504,14 +504,14 @@ function handleEscaping(tokens, token, index) {
 	}
 
 	return [...tokens, token];
-}
+};
 
-function parseCommand(command) {
+const parseCommand = command => {
 	return command
 		.trim()
 		.split(SPACES_REGEXP)
 		.reduce(handleEscaping, []);
-}
+};
 
 module.exports.command = (command, options) => {
 	const [file, ...args] = parseCommand(command);
@@ -523,19 +523,20 @@ module.exports.commandSync = (command, options) => {
 	return execa.sync(file, args, options);
 };
 
-module.exports.node = (scriptPath, args, options) => {
+module.exports.node = (scriptPath, args, options = {}) => {
 	if (args && !Array.isArray(args) && typeof args === 'object') {
 		options = args;
 		args = [];
 	}
 
 	const stdioOption = stdio.node(options);
-	options = options || {};
+
+	const {nodePath = process.execPath, nodeArguments = process.execArgv} = options;
 
 	return execa(
-		options.nodePath || process.execPath,
+		nodePath,
 		[
-			...(options.nodeArguments || process.execArgv),
+			...nodeArguments,
 			scriptPath,
 			...(Array.isArray(args) ? args : [])
 		],
diff --git a/lib/stdio.js b/lib/stdio.js
index d9f5458..310d5b4 100644
--- a/lib/stdio.js
+++ b/lib/stdio.js
@@ -1,61 +1,52 @@
 'use strict';
-const alias = ['stdin', 'stdout', 'stderr'];
+const aliases = ['stdin', 'stdout', 'stderr'];
 
-const hasAlias = opts => alias.some(x => Boolean(opts[x]));
+const hasAlias = opts => aliases.some(alias => opts[alias] !== undefined);
 
 const stdio = opts => {
 	if (!opts) {
 		return;
 	}
 
-	if (opts.stdio && hasAlias(opts)) {
-		throw new Error(`It's not possible to provide \`stdio\` in combination with one of ${alias.map(x => `\`${x}\``).join(', ')}`);
+	const {stdio} = opts;
+
+	if (stdio === undefined) {
+		return aliases.map(alias => opts[alias]);
 	}
 
-	if (typeof opts.stdio === 'string') {
-		return opts.stdio;
+	if (hasAlias(opts)) {
+		throw new Error(`It's not possible to provide \`stdio\` in combination with one of ${aliases.map(alias => `\`${alias}\``).join(', ')}`);
 	}
 
-	const stdio = opts.stdio || [];
+	if (typeof stdio === 'string') {
+		return stdio;
+	}
 
 	if (!Array.isArray(stdio)) {
 		throw new TypeError(`Expected \`stdio\` to be of type \`string\` or \`Array\`, got \`${typeof stdio}\``);
 	}
 
-	const result = [];
-	const len = Math.max(stdio.length, alias.length);
-
-	for (let i = 0; i < len; i++) {
-		let value;
-
-		if (stdio[i] !== undefined) {
-			value = stdio[i];
-		} else if (opts[alias[i]] !== undefined) {
-			value = opts[alias[i]];
-		}
-
-		result[i] = value;
-	}
-
-	return result;
+	const length = Math.max(stdio.length, aliases.length);
+	return Array.from({length}, (value, index) => stdio[index]);
 };
 
 module.exports = stdio;
 
+// `ipc` is pushed unless it is already present
 module.exports.node = opts => {
-	const defaultOption = 'pipe';
+	const stdioOption = stdio(opts);
 
-	let stdioOption = stdio(opts || {stdio: defaultOption});
+	if (stdioOption === 'ipc') {
+		return 'ipc';
+	}
 
-	if (typeof stdioOption === 'string') {
-		stdioOption = [...new Array(3)].fill(stdioOption);
-	} else if (Array.isArray(stdioOption)) {
-		stdioOption = stdioOption.map((channel = defaultOption) => channel);
+	if (stdioOption === undefined || typeof stdioOption === 'string') {
+		return [stdioOption, stdioOption, stdioOption, 'ipc'];
 	}
 
-	if (!stdioOption.includes('ipc')) {
-		stdioOption.push('ipc');
+	if (stdioOption.includes('ipc')) {
+		return stdioOption;
 	}
 
-	return stdioOption;
+	return [...stdioOption, 'ipc'];
 };
diff --git a/readme.md b/readme.md
index e0e8571..100cd36 100644
--- a/readme.md
+++ b/readme.md
@@ -188,9 +188,9 @@ Returns or throws a [`childProcessResult`](#childProcessResult).
 
 Execute a Node.js script as a child process.
 
-Same as `execa('node', [file, ...arguments], options)` except (like [`child_process#fork()`](https://nodejs.org/api/child_process.html#child_process_child_process_fork_modulepath_args_options)):
-  - the [`nodePath`](#nodepath-for-node-only) and [`nodeArguments`](#nodearguments-for-node-only) options can be used
-  - the [`shell`](#shell) option cannot be used 
+Same as `execa('node', [scriptPath, ...arguments], options)` except (like [`child_process#fork()`](https://nodejs.org/api/child_process.html#child_process_child_process_fork_modulepath_args_options)):
+  - the current Node version and options are used. This can be overridden using the [`nodePath`](#nodepath-for-node-only) and [`nodeArguments`](#nodearguments-for-node-only) options.
+  - the [`shell`](#shell) option cannot be used
   - an extra channel [`ipc`](https://nodejs.org/api/child_process.html#child_process_options_stdio) is passed to [`stdio`](#stdio)
 
 ### childProcessResult


Test patch:
diff --git a/test/stdio.js b/test/stdio.js
index a9ccb03..153ca61 100644
--- a/test/stdio.js
+++ b/test/stdio.js
@@ -2,32 +2,21 @@ import util from 'util';
 import test from 'ava';
 import stdio from '../lib/stdio';
 
-util.inspect.styles.name = 'magenta';
+const macro = (t, input, expected, func) => {
+	if (expected instanceof Error) {
+		t.throws(() => {
+			stdio(input);
+		}, expected.message);
+		return;
+	}
 
-function createMacro(func) {
-	const macro = (t, input, expected) => {
-		if (expected instanceof Error) {
-			t.throws(() => {
-				stdio(input);
-			}, expected.message);
-			return;
-		}
+	t.deepEqual(func(input), expected);
+};
 
-		const result = func(input);
+const macroTitle = name => (title, input) => `${name} ${(util.inspect(input))}`;
 
-		if (typeof expected === 'object' && expected !== null) {
-			t.deepEqual(result, expected);
-		} else {
-			t.is(result, expected);
-		}
-	};
-
-	macro.title = (providedTitle, input) => `${func.name} ${(providedTitle || util.inspect(input, {colors: true}))}`;
-
-	return macro;
-}
-
-const stdioMacro = createMacro(stdio);
+const stdioMacro = (...args) => macro(...args, stdio);
+stdioMacro.title = macroTitle('execa()');
 
 test(stdioMacro, undefined, undefined);
 test(stdioMacro, null, undefined);
@@ -56,17 +45,22 @@ test(stdioMacro, {stdio: {foo: 'bar'}}, new TypeError('Expected `stdio` to be of
 test(stdioMacro, {stdin: 'inherit', stdio: 'pipe'}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
 test(stdioMacro, {stdin: 'inherit', stdio: ['pipe']}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
 test(stdioMacro, {stdin: 'inherit', stdio: [undefined, 'pipe']}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
+test(stdioMacro, {stdin: 0, stdio: 'pipe'}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
 
-const forkMacro = createMacro(stdio.node);
+const forkMacro = (...args) => macro(...args, stdio.node);
+forkMacro.title = macroTitle('execa.fork()');
 
-test(forkMacro, undefined, ['pipe', 'pipe', 'pipe', 'ipc']);
+test(forkMacro, undefined, [undefined, undefined, undefined, 'ipc']);
 test(forkMacro, {stdio: 'ignore'}, ['ignore', 'ignore', 'ignore', 'ipc']);
+test(forkMacro, {stdio: 'ipc'}, 'ipc');
 test(forkMacro, {stdio: [0, 1, 2]}, [0, 1, 2, 'ipc']);
 test(forkMacro, {stdio: [0, 1, 2, 3]}, [0, 1, 2, 3, 'ipc']);
 test(forkMacro, {stdio: [0, 1, 2, 'ipc']}, [0, 1, 2, 'ipc']);
 
-test(forkMacro, {stdout: 'ignore'}, ['pipe', 'ignore', 'pipe', 'ipc']);
-test(forkMacro, {stdout: 'ignore', stderr: 'ignore'}, ['pipe', 'ignore', 'ignore', 'ipc']);
+test(forkMacro, {stdio: [0, 1, undefined]}, [0, 1, undefined, 'ipc']);
+test(forkMacro, {stdio: [0, 1, 2, undefined]}, [0, 1, 2, undefined, 'ipc']);
+test(forkMacro, {stdout: 'ignore'}, [undefined, 'ignore', undefined, 'ipc']);
+test(forkMacro, {stdout: 'ignore', stderr: 'ignore'}, [undefined, 'ignore', 'ignore', 'ipc']);
 
 test(forkMacro, {stdio: {foo: 'bar'}}, new TypeError('Expected `stdio` to be of type `string` or `Array`, got `object`'));
 test(forkMacro, {stdin: 'inherit', stdio: 'pipe'}, new Error('It\'s not possible to provide `stdio` in combination with one of `stdin`, `stdout`, `stderr`'));
diff --git a/test/test.js b/test/test.js
index 613ddec..bf274a8 100644
--- a/test/test.js
+++ b/test/test.js
@@ -487,10 +487,10 @@ test('result.signal is undefined if process failed, but was not killed', async t
 	t.is(signal, undefined);
 });
 
-async function testExitCode(t, num) {
+const testExitCode = async (t, num) => {
 	const {exitCode} = await t.throwsAsync(execa('exit', [`${num}`]), {message: getExitRegExp(num)});
 	t.is(exitCode, num);
-}
+};
 
 test('error.exitCode is 2', testExitCode, 2);
 test('error.exitCode is 3', testExitCode, 3);
@@ -525,22 +525,22 @@ test('timedOut will be false if no timeout was set and zero exit code in sync mo
 	t.false(timedOut);
 });
 
-async function errorMessage(t, expected, ...args) {
+const errorMessage = async (t, expected, ...args) => {
 	await t.throwsAsync(execa('exit', args), {message: expected});
-}
+};
 
 errorMessage.title = (message, expected) => `error.message matches: ${expected}`;
 
 test(errorMessage, /Command failed with exit code 2.*: exit 2 foo bar/, 2, 'foo', 'bar');
 test(errorMessage, /Command failed with exit code 3.*: exit 3 baz quz/, 3, 'baz', 'quz');
 
-async function command(t, expected, ...args) {
+const command = async (t, expected, ...args) => {
 	const {command: failCommand} = await t.throwsAsync(execa('fail', args));
 	t.is(failCommand, `fail${expected}`);
 
 	const {command} = await execa('noop', args);
 	t.is(command, `noop${expected}`);
-}
+};
 
 command.title = (message, expected) => `command is: ${JSON.stringify(expected)}`;
 
@@ -549,9 +549,9 @@ test(command, ' baz quz', 'baz', 'quz');
 test(command, '');
 
 // When child process exits before parent process
-async function spawnAndExit(t, cleanup, detached) {
+const spawnAndExit = async (t, cleanup, detached) => {
 	await t.notThrowsAsync(execa('sub-process-exit', [cleanup, detached]));
-}
+};
 
 test('spawnAndExit', spawnAndExit, false, false);
 test('spawnAndExit cleanup', spawnAndExit, true, false);
@@ -559,7 +559,7 @@ test('spawnAndExit detached', spawnAndExit, false, true);
 test('spawnAndExit cleanup detached', spawnAndExit, true, true);
 
 // When parent process exits before child process
-async function spawnAndKill(t, signal, cleanup, detached, isKilled) {
+const spawnAndKill = async (t, signal, cleanup, detached, isKilled) => {
 	const subprocess = execa('sub-process', [cleanup, detached], {stdio: ['ignore', 'ignore', 'ignore', 'ipc']});
 
 	const pid = await pEvent(subprocess, 'message');
@@ -576,7 +576,7 @@ async function spawnAndKill(t, signal, cleanup, detached, isKilled) {
 	if (!isKilled) {
 		process.kill(pid, 'SIGKILL');
 	}
-}
+};
 
 // Without `options.cleanup`:
 //   - on Windows subprocesses are killed if `options.detached: false`, but not