Repository: sindresorhus/execa
Pull request #551 (2023-03-13 22:30:11)

Improve validation of `$.sync` options binding
To bind options with the synchronous `$` API, one must do:

```js
$(options).sync`command`
```

However, users are likely to do the following instead:

```js
$.sync(options)`command`
```

We could fix this either:
  - By supporting that second syntax.
  - By making it fail with a better error message.

This PR implements the second solution. However, I am happy to implement the first solution if you'd prefer it instead, just let me know!

ehmicky: I ran into the mistake myself, when writing automated tests, and it took me a little bit to realize what I was doing wrong :)

Patch:
diff --git a/index.js b/index.js
index 87c8276..963c96d 100644
--- a/index.js
+++ b/index.js
@@ -234,15 +234,19 @@ export function execaSync(file, args, options) {
 
 function create$(options) {
 	function $(templatesOrOptions, ...expressions) {
-		if (Array.isArray(templatesOrOptions)) {
-			const [file, ...args] = parseTemplates(templatesOrOptions, expressions);
-			return execa(file, args, options);
+		if (!Array.isArray(templatesOrOptions)) {
+			return create$({...options, ...templatesOrOptions});
 		}
 
-		return create$({...options, ...templatesOrOptions});
+		const [file, ...args] = parseTemplates(templatesOrOptions, expressions);
+		return execa(file, args, options);
 	}
 
 	$.sync = (templates, ...expressions) => {
+		if (!Array.isArray(templates)) {
+			throw new TypeError('Please use $(options).sync`command` instead of $.sync(options)`command`.');
+		}
+
 		const [file, ...args] = parseTemplates(templates, expressions);
 		return execaSync(file, args, options);
 	};


Test patch:
diff --git a/test/command.js b/test/command.js
index 43b7495..affcc58 100644
--- a/test/command.js
+++ b/test/command.js
@@ -53,12 +53,12 @@ test('allow commands with spaces and array arguments', async t => {
 });
 
 test('execaCommand()', async t => {
-	const {stdout} = await execaCommand('node test/fixtures/echo.js foo bar');
+	const {stdout} = await execaCommand('echo.js foo bar');
 	t.is(stdout, 'foo\nbar');
 });
 
 test('execaCommand() ignores consecutive spaces', async t => {
-	const {stdout} = await execaCommand('node test/fixtures/echo.js foo    bar');
+	const {stdout} = await execaCommand('echo.js foo    bar');
 	t.is(stdout, 'foo\nbar');
 });
 
@@ -68,27 +68,27 @@ test('execaCommand() allows escaping spaces in commands', async t => {
 });
 
 test('execaCommand() allows escaping spaces in arguments', async t => {
-	const {stdout} = await execaCommand('node test/fixtures/echo.js foo\\ bar');
+	const {stdout} = await execaCommand('echo.js foo\\ bar');
 	t.is(stdout, 'foo bar');
 });
 
 test('execaCommand() escapes other whitespaces', async t => {
-	const {stdout} = await execaCommand('node test/fixtures/echo.js foo\tbar');
+	const {stdout} = await execaCommand('echo.js foo\tbar');
 	t.is(stdout, 'foo\tbar');
 });
 
 test('execaCommand() trims', async t => {
-	const {stdout} = await execaCommand('  node test/fixtures/echo.js foo bar  ');
+	const {stdout} = await execaCommand('  echo.js foo bar  ');
 	t.is(stdout, 'foo\nbar');
 });
 
 test('execaCommandSync()', t => {
-	const {stdout} = execaCommandSync('node test/fixtures/echo.js foo bar');
+	const {stdout} = execaCommandSync('echo.js foo bar');
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js foo bar`;
+	const {stdout} = await $`echo.js foo bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
@@ -98,76 +98,76 @@ test('$ accepts options', async t => {
 });
 
 test('$ allows string interpolation', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js foo ${'bar'}`;
+	const {stdout} = await $`echo.js foo ${'bar'}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows number interpolation', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js 1 ${2}`;
+	const {stdout} = await $`echo.js 1 ${2}`;
 	t.is(stdout, '1\n2');
 });
 
 test('$ allows array interpolation', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js ${['foo', 'bar']}`;
+	const {stdout} = await $`echo.js ${['foo', 'bar']}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows execa return value interpolation', async t => {
-	const foo = await $`node test/fixtures/echo.js foo`;
-	const {stdout} = await $`node test/fixtures/echo.js ${foo} bar`;
+	const foo = await $`echo.js foo`;
+	const {stdout} = await $`echo.js ${foo} bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows execa return value array interpolation', async t => {
-	const foo = await $`node test/fixtures/echo.js foo`;
-	const {stdout} = await $`node test/fixtures/echo.js ${[foo, 'bar']}`;
+	const foo = await $`echo.js foo`;
+	const {stdout} = await $`echo.js ${[foo, 'bar']}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows execa return value buffer interpolation', async t => {
-	const foo = await $({encoding: null})`node test/fixtures/echo.js foo`;
-	const {stdout} = await $`node test/fixtures/echo.js ${foo} bar`;
+	const foo = await $({encoding: null})`echo.js foo`;
+	const {stdout} = await $`echo.js ${foo} bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows execa return value buffer array interpolation', async t => {
-	const foo = await $({encoding: null})`node test/fixtures/echo.js foo`;
-	const {stdout} = await $`node test/fixtures/echo.js ${[foo, 'bar']}`;
+	const foo = await $({encoding: null})`echo.js foo`;
+	const {stdout} = await $`echo.js ${[foo, 'bar']}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ ignores consecutive spaces', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js foo    bar`;
+	const {stdout} = await $`echo.js foo    bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$ allows escaping spaces with interpolation', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js ${'foo bar'}`;
+	const {stdout} = await $`echo.js ${'foo bar'}`;
 	t.is(stdout, 'foo bar');
 });
 
 test('$ disallows escaping spaces with backslashes', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js foo\\ bar`;
+	const {stdout} = await $`echo.js foo\\ bar`;
 	t.is(stdout, 'foo\\\nbar');
 });
 
 test('$ allows space escaped values in array interpolation', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js ${['foo', 'bar baz']}`;
+	const {stdout} = await $`echo.js ${['foo', 'bar baz']}`;
 	t.is(stdout, 'foo\nbar baz');
 });
 
 test('$ passes newline escape sequence as one argument', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js \n`;
+	const {stdout} = await $`echo.js \n`;
 	t.is(stdout, '\n');
 });
 
 test('$ passes newline escape sequence in interpolation as one argument', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js ${'\n'}`;
+	const {stdout} = await $`echo.js ${'\n'}`;
 	t.is(stdout, '\n');
 });
 
 test('$ handles invalid escape sequence', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js \u`;
+	const {stdout} = await $`echo.js \u`;
 	t.is(stdout, '\\u');
 });
 
@@ -177,17 +177,17 @@ test('$ allows escaping spaces in commands with interpolation', async t => {
 });
 
 test('$ escapes other whitespaces', async t => {
-	const {stdout} = await $`node test/fixtures/echo.js foo\tbar`;
+	const {stdout} = await $`echo.js foo\tbar`;
 	t.is(stdout, 'foo\tbar');
 });
 
 test('$ trims', async t => {
-	const {stdout} = await $`  node test/fixtures/echo.js foo bar  `;
+	const {stdout} = await $`  echo.js foo bar  `;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$.sync', t => {
-	const {stdout} = $.sync`node test/fixtures/echo.js foo bar`;
+	const {stdout} = $.sync`echo.js foo bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
@@ -196,39 +196,43 @@ test('$.sync accepts options', t => {
 	t.is(stdout, 'foo');
 });
 
+test('$.sync must be used after options binding, not before', t => {
+	t.throws(() => $.sync({})`noop.js`, {message: /Please use/});
+});
+
 test('$.sync allows execa return value interpolation', t => {
-	const foo = $.sync`node test/fixtures/echo.js foo`;
-	const {stdout} = $.sync`node test/fixtures/echo.js ${foo} bar`;
+	const foo = $.sync`echo.js foo`;
+	const {stdout} = $.sync`echo.js ${foo} bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$.sync allows execa return value array interpolation', t => {
-	const foo = $.sync`node test/fixtures/echo.js foo`;
-	const {stdout} = $.sync`node test/fixtures/echo.js ${[foo, 'bar']}`;
+	const foo = $.sync`echo.js foo`;
+	const {stdout} = $.sync`echo.js ${[foo, 'bar']}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$.sync allows execa return value buffer interpolation', t => {
-	const foo = $({encoding: null}).sync`node test/fixtures/echo.js foo`;
-	const {stdout} = $.sync`node test/fixtures/echo.js ${foo} bar`;
+	const foo = $({encoding: null}).sync`echo.js foo`;
+	const {stdout} = $.sync`echo.js ${foo} bar`;
 	t.is(stdout, 'foo\nbar');
 });
 
 test('$.sync allows execa return value buffer array interpolation', t => {
-	const foo = $({encoding: null}).sync`node test/fixtures/echo.js foo`;
-	const {stdout} = $.sync`node test/fixtures/echo.js ${[foo, 'bar']}`;
+	const foo = $({encoding: null}).sync`echo.js foo`;
+	const {stdout} = $.sync`echo.js ${[foo, 'bar']}`;
 	t.is(stdout, 'foo\nbar');
 });
 
 const invalidExpression = test.macro({
 	async exec(t, input, expected) {
 		await t.throwsAsync(
-			async () => $`node test/fixtures/echo.js ${input}`,
+			async () => $`echo.js ${input}`,
 			{instanceOf: TypeError, message: expected},
 		);
 
 		t.throws(
-			() => $.sync`node test/fixtures/echo.js ${input}`,
+			() => $.sync`echo.js ${input}`,
 			{instanceOf: TypeError, message: expected},
 		);
 	},
diff --git a/test/fixtures/detach.js b/test/fixtures/detach.js
index b21d056..0f09128 100755
--- a/test/fixtures/detach.js
+++ b/test/fixtures/detach.js
@@ -2,6 +2,6 @@
 import process from 'node:process';
 import {execa} from '../../index.js';
 
-const subprocess = execa('node', ['./test/fixtures/forever.js'], {detached: true});
+const subprocess = execa('forever.js', {detached: true});
 console.log(subprocess.pid);
 process.exit(0);
diff --git a/test/fixtures/sub-process-exit.js b/test/fixtures/sub-process-exit.js
index 79890ab..2c1e624 100755
--- a/test/fixtures/sub-process-exit.js
+++ b/test/fixtures/sub-process-exit.js
@@ -6,7 +6,7 @@ const cleanup = process.argv[2] === 'true';
 const detached = process.argv[3] === 'true';
 
 try {
-	await execa('node', ['./test/fixtures/noop.js'], {cleanup, detached});
+	await execa('noop.js', {cleanup, detached});
 } catch (error) {
 	console.error(error);
 	process.exit(1);
diff --git a/test/fixtures/sub-process.js b/test/fixtures/sub-process.js
index a1b806d..5b5b247 100755
--- a/test/fixtures/sub-process.js
+++ b/test/fixtures/sub-process.js
@@ -4,5 +4,5 @@ import {execa} from '../../index.js';
 
 const cleanup = process.argv[2] === 'true';
 const detached = process.argv[3] === 'true';
-const subprocess = execa('node', ['./test/fixtures/forever.js'], {cleanup, detached});
+const subprocess = execa('forever.js', {cleanup, detached});
 process.send(subprocess.pid);
diff --git a/test/kill.js b/test/kill.js
index dc6b892..db65f9e 100644
--- a/test/kill.js
+++ b/test/kill.js
@@ -10,7 +10,7 @@ setFixtureDir();
 const TIMEOUT_REGEXP = /timed out after/;
 
 test('kill("SIGKILL") should terminate cleanly', async t => {
-	const subprocess = execa('node', ['./test/fixtures/no-killable.js'], {stdio: ['ipc']});
+	const subprocess = execa('no-killable.js', {stdio: ['ipc']});
 	await pEvent(subprocess, 'message');
 
 	subprocess.kill('SIGKILL');
@@ -23,7 +23,7 @@ test('kill("SIGKILL") should terminate cleanly', async t => {
 // Therefore, this feature and those tests do not make sense on Windows.
 if (process.platform !== 'win32') {
 	test('`forceKillAfterTimeout: false` should not kill after a timeout', async t => {
-		const subprocess = execa('node', ['./test/fixtures/no-killable.js'], {stdio: ['ipc']});
+		const subprocess = execa('no-killable.js', {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
 		subprocess.kill('SIGTERM', {forceKillAfterTimeout: false});
@@ -33,7 +33,7 @@ if (process.platform !== 'win32') {
 	});
 
 	test('`forceKillAfterTimeout: number` should kill after a timeout', async t => {
-		const subprocess = execa('node', ['./test/fixtures/no-killable.js'], {stdio: ['ipc']});
+		const subprocess = execa('no-killable.js', {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
 		subprocess.kill('SIGTERM', {forceKillAfterTimeout: 50});
@@ -43,7 +43,7 @@ if (process.platform !== 'win32') {
 	});
 
 	test('`forceKillAfterTimeout: true` should kill after a timeout', async t => {
-		const subprocess = execa('node', ['./test/fixtures/no-killable.js'], {stdio: ['ipc']});
+		const subprocess = execa('no-killable.js', {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
 		subprocess.kill('SIGTERM', {forceKillAfterTimeout: true});
@@ -53,7 +53,7 @@ if (process.platform !== 'win32') {
 	});
 
 	test('kill() with no arguments should kill after a timeout', async t => {
-		const subprocess = execa('node', ['./test/fixtures/no-killable.js'], {stdio: ['ipc']});
+		const subprocess = execa('no-killable.js', {stdio: ['ipc']});
 		await pEvent(subprocess, 'message');
 
 		subprocess.kill();
diff --git a/test/stream.js b/test/stream.js
index 3d2c187..6ce70be 100644
--- a/test/stream.js
+++ b/test/stream.js
@@ -19,13 +19,13 @@ test('buffer', async t => {
 
 test('pass `stdout` to a file descriptor', async t => {
 	const file = tempfile('.txt');
-	await execa('test/fixtures/noop.js', ['foo bar'], {stdout: fs.openSync(file, 'w')});
+	await execa('noop.js', ['foo bar'], {stdout: fs.openSync(file, 'w')});
 	t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n');
 });
 
 test('pass `stderr` to a file descriptor', async t => {
 	const file = tempfile('.txt');
-	await execa('test/fixtures/noop-err.js', ['foo bar'], {stderr: fs.openSync(file, 'w')});
+	await execa('noop-err.js', ['foo bar'], {stderr: fs.openSync(file, 'w')});
 	t.is(fs.readFileSync(file, 'utf8'), 'foo bar\n');
 });