Repository: sindresorhus/execa
Pull request #287 (2019-06-14 09:03:25)

Refactor `getStream()`
This refactors the `getStream()` function, simplifying it a little. It does not change behavior.


Patch:
diff --git a/index.d.ts b/index.d.ts
index e5d6402..cb25030 100644
--- a/index.d.ts
+++ b/index.d.ts
@@ -159,9 +159,9 @@ declare namespace execa {
 		readonly timeout?: number;
 
 		/**
-		Largest amount of data in bytes allowed on `stdout` or `stderr`. Default: 10MB.
+		Largest amount of data in bytes allowed on `stdout` or `stderr`. Default: 100 MB.
 
-		@default 10000000
+		@default 100_000_000
 		*/
 		readonly maxBuffer?: number;
 
diff --git a/index.js b/index.js
index 4482095..a56276c 100644
--- a/index.js
+++ b/index.js
@@ -7,13 +7,13 @@ const crossSpawn = require('cross-spawn');
 const stripFinalNewline = require('strip-final-newline');
 const npmRunPath = require('npm-run-path');
 const isStream = require('is-stream');
-const _getStream = require('get-stream');
+const getStream = require('get-stream');
 const mergeStream = require('merge-stream');
 const pFinally = require('p-finally');
 const onExit = require('signal-exit');
 const stdio = require('./lib/stdio');
 
-const TEN_MEGABYTES = 1000 * 1000 * 10;
+const DEFAULT_MAX_BUFFER = 1000 * 1000 * 100;
 const DEFAULT_FORCE_KILL_TIMEOUT = 1000 * 5;
 
 const SPACES_REGEXP = / +/g;
@@ -25,7 +25,7 @@ function handleArgs(file, args, options = {}) {
 	options = parsed.options;
 
 	options = {
-		maxBuffer: TEN_MEGABYTES,
+		maxBuffer: DEFAULT_MAX_BUFFER,
 		buffer: true,
 		stripFinalNewline: true,
 		preferLocal: true,
@@ -120,30 +120,25 @@ async function getBufferedData(stream, streamPromise) {
 	}
 }
 
-function getStream(process, stream, {encoding, buffer, maxBuffer}) {
-	if (!process[stream]) {
+function getStreamPromise(stream, {encoding, buffer, maxBuffer}) {
+	if (!stream) {
 		return;
 	}
 
-	let ret;
-
 	if (!buffer) {
-		// TODO: Use `ret = util.promisify(stream.finished)(process[stream]);` when targeting Node.js 10
-		ret = new Promise((resolve, reject) => {
-			process[stream]
+		// TODO: Use `ret = util.promisify(stream.finished)(stream);` when targeting Node.js 10
+		return new Promise((resolve, reject) => {
+			stream
 				.once('end', resolve)
 				.once('error', reject);
 		});
-	} else if (encoding) {
-		ret = _getStream(process[stream], {
-			encoding,
-			maxBuffer
-		});
-	} else {
-		ret = _getStream.buffer(process[stream], {maxBuffer});
 	}
 
-	return ret;
+	if (encoding) {
+		return getStream(stream, {encoding, maxBuffer});
+	}
+
+	return getStream.buffer(stream, {maxBuffer});
 }
 
 function makeError(result, options) {
@@ -228,6 +223,30 @@ function joinCommand(file, args = []) {
 	return [file, ...args].join(' ');
 }
 
+function mergePromiseProperty(spawned, getPromise, property) {
+	Object.defineProperty(spawned, property, {
+		value(...args) {
+			return getPromise()[property](...args);
+		},
+		writable: true,
+		enumerable: false,
+		configurable: true
+	});
+}
+
+// The return value is a mixin of `childProcess` and `Promise`
+function mergePromise(spawned, getPromise) {
+	mergePromiseProperty(spawned, getPromise, 'then');
+	mergePromiseProperty(spawned, getPromise, 'catch');
+
+	// TODO: Remove the `if`-guard when targeting Node.js 10
+	if (Promise.prototype.finally) {
+		mergePromiseProperty(spawned, getPromise, 'finally');
+	}
+
+	return spawned;
+}
+
 function spawnedKill(kill, signal = 'SIGTERM', options = {}) {
 	const killResult = kill(signal);
 	setKillTimeout(kill, signal, options, killResult);
@@ -275,13 +294,15 @@ const execa = (file, args, options) => {
 	try {
 		spawned = childProcess.spawn(parsed.file, parsed.args, parsed.options);
 	} catch (error) {
-		return Promise.reject(makeError({error, stdout: '', stderr: '', all: ''}, {
-			command,
-			parsed,
-			timedOut: false,
-			isCanceled: false,
-			killed: false
-		}));
+		return mergePromise(new childProcess.ChildProcess(), () =>
+			Promise.reject(makeError({error, stdout: '', stderr: '', all: ''}, {
+				command,
+				parsed,
+				timedOut: false,
+				isCanceled: false,
+				killed: false
+			}))
+		);
 	}
 
 	const kill = spawned.kill.bind(spawned);
@@ -340,9 +361,9 @@ const execa = (file, args, options) => {
 	}), cleanup);
 
 	const handlePromise = () => {
-		const stdoutPromise = getStream(spawned, 'stdout', {encoding, buffer, maxBuffer});
-		const stderrPromise = getStream(spawned, 'stderr', {encoding, buffer, maxBuffer});
-		const allPromise = getStream(spawned, 'all', {encoding, buffer, maxBuffer: maxBuffer * 2});
+		const stdoutPromise = getStreamPromise(spawned.stdout, {encoding, buffer, maxBuffer});
+		const stderrPromise = getStreamPromise(spawned.stderr, {encoding, buffer, maxBuffer});
+		const allPromise = getStreamPromise(spawned.all, {encoding, buffer, maxBuffer: maxBuffer * 2});
 
 		const finalize = async () => {
 			let results;
@@ -403,21 +424,13 @@ const execa = (file, args, options) => {
 
 	spawned.all = makeAllStream(spawned);
 
-	// eslint-disable-next-line promise/prefer-await-to-then
-	spawned.then = (onFulfilled, onRejected) => handlePromise().then(onFulfilled, onRejected);
-	spawned.catch = onRejected => handlePromise().catch(onRejected);
 	spawned.cancel = () => {
 		if (spawned.kill()) {
 			isCanceled = true;
 		}
 	};
 
-	// TODO: Remove the `if`-guard when targeting Node.js 10
-	if (Promise.prototype.finally) {
-		spawned.finally = onFinally => handlePromise().finally(onFinally);
-	}
-
-	return spawned;
+	return mergePromise(spawned, handlePromise);
 };
 
 module.exports = execa;
diff --git a/readme.md b/readme.md
index 4ffbb35..d4198c3 100644
--- a/readme.md
+++ b/readme.md
@@ -12,7 +12,7 @@
 - [Strips the final newline](#stripfinalnewline) from the output so you don't have to do `stdout.trim()`.
 - Supports [shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) binaries cross-platform.
 - [Improved Windows support.](https://github.com/IndigoUnited/node-cross-spawn#why)
-- Higher max buffer. 10 MB instead of 200 KB.
+- Higher max buffer. 100 MB instead of 200 KB.
 - [Executes locally installed binaries by name.](#preferlocal)
 - [Cleans up spawned processes when the parent process dies.](#cleanup)
 - [Get interleaved output](#all) from `stdout` and `stderr` similar to what is printed on the terminal. [*(Async only)*](#execasyncfile-arguments-options)
@@ -411,7 +411,7 @@ If timeout is greater than `0`, the parent will send the signal identified by th
 #### maxBuffer
 
 Type: `number`<br>
-Default: `10000000` (10MB)
+Default: `100_000_000` (100 MB)
 
 Largest amount of data in bytes allowed on `stdout` or `stderr`.
 


Test patch:
diff --git a/test.js b/test.js
index 94a095b..18bee8c 100644
--- a/test.js
+++ b/test.js
@@ -301,6 +301,15 @@ test('execa() returns a promise with kill() and pid', t => {
 	t.is(typeof pid, 'number');
 });
 
+test('child_process.spawn() propagated errors have correct shape', t => {
+	const cp = execa('noop', {uid: -1});
+	t.notThrows(() => {
+		cp.catch(() => {});
+		cp.unref();
+		cp.on('error', () => {});
+	});
+});
+
 test('child_process.spawn() errors are propagated', async t => {
 	const {failed} = await t.throwsAsync(execa('noop', {uid: -1}));
 	t.true(failed);
@@ -664,6 +673,17 @@ test('removes exit handler on exit', async t => {
 	t.false(included);
 });
 
+test('promise methods are not enumerable', t => {
+	const descriptors = Object.getOwnPropertyDescriptors(execa('noop'));
+	// eslint-disable-next-line promise/prefer-await-to-then
+	t.false(descriptors.then.enumerable);
+	t.false(descriptors.catch.enumerable);
+	// TOOD: Remove the `if`-guard when targeting Node.js 10
+	if (Promise.prototype.finally) {
+		t.false(descriptors.finally.enumerable);
+	}
+});
+
 // TOOD: Remove the `if`-guard when targeting Node.js 10
 if (Promise.prototype.finally) {
 	test('finally function is executed on success', async t => {