Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default value of stdin with $ #550

Merged
merged 1 commit into from Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions index.d.ts
Expand Up @@ -27,7 +27,7 @@ export type CommonOptions<EncodingType> = {

If you `$ npm install foo`, you can then `execa('foo')`.

@default `true` with `$`/`$.sync`, `false` otherwise
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think $.sync is implied by $, so I am not sure we need to be explicit there.

@default `true` with `$`, `false` otherwise
*/
readonly preferLocal?: boolean;

Expand Down Expand Up @@ -63,7 +63,7 @@ export type CommonOptions<EncodingType> = {
/**
Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio).

@default 'pipe'
@default `inherit` with `$`, `pipe` otherwise
*/
readonly stdin?: StdioOption;

Expand Down
16 changes: 13 additions & 3 deletions index.js
Expand Up @@ -232,14 +232,24 @@ export function execaSync(file, args, options) {
};
}

const normalizeScriptStdin = ({input, inputFile, stdio}) => input === undefined && inputFile === undefined && stdio === undefined
? {stdin: 'inherit'}
: {};

const normalizeScriptOptions = (options = {}) => ({
preferLocal: true,
...normalizeScriptStdin(options),
...options,
});

function create$(options) {
function $(templatesOrOptions, ...expressions) {
if (!Array.isArray(templatesOrOptions)) {
return create$({...options, ...templatesOrOptions});
}

const [file, ...args] = parseTemplates(templatesOrOptions, expressions);
return execa(file, args, options);
return execa(file, args, normalizeScriptOptions(options));
}

$.sync = (templates, ...expressions) => {
Expand All @@ -248,13 +258,13 @@ function create$(options) {
}

const [file, ...args] = parseTemplates(templates, expressions);
return execaSync(file, args, options);
return execaSync(file, args, normalizeScriptOptions(options));
};

return $;
}

export const $ = create$({preferLocal: true});
export const $ = create$();

export function execaCommand(command, options) {
const [file, ...args] = parseCommand(command);
Expand Down
4 changes: 2 additions & 2 deletions readme.md
Expand Up @@ -467,7 +467,7 @@ Kill the spawned process when the parent process exits unless either:
#### preferLocal

Type: `boolean`\
Default: `true` with [`$`](#command)/[`$.sync`](#synccommand), `false` otherwise
Default: `true` with [`$`](#command), `false` otherwise

Prefer locally installed binaries when looking for a binary to execute.\
If you `$ npm install foo`, you can then `execa('foo')`.
Expand Down Expand Up @@ -521,7 +521,7 @@ If the input is not a file, use the [`input` option](#input) instead.
#### stdin

Type: `string | number | Stream | undefined`\
Default: `pipe`
Default: `inherit` with [`$`](#command), `pipe` otherwise

Same options as [`stdio`](https://nodejs.org/dist/latest-v6.x/docs/api/child_process.html#child_process_options_stdio).

Expand Down
15 changes: 15 additions & 0 deletions test/command.js
@@ -1,5 +1,6 @@
import {inspect} from 'node:util';
import test from 'ava';
import {isStream} from 'is-stream';
import {execa, execaSync, execaCommand, execaCommandSync, $} from '../index.js';
import {setFixtureDir} from './helpers/fixtures-dir.js';

Expand Down Expand Up @@ -273,3 +274,17 @@ test('[ $`noop.js` ]', invalidExpression, [$`noop.js`], 'Unexpected "object" in

test('$({stdio: \'inherit\'}).sync`noop.js`', invalidExpression, $({stdio: 'inherit'}).sync`noop.js`, 'Unexpected "undefined" stdout in template expression');
test('[ $({stdio: \'inherit\'}).sync`noop.js` ]', invalidExpression, [$({stdio: 'inherit'}).sync`noop.js`], 'Unexpected "undefined" stdout in template expression');

test('$ stdin defaults to "inherit"', async t => {
const {stdout} = await $({input: 'foo'})`stdin-script.js`;
t.is(stdout, 'foo');
});

test('$.sync stdin defaults to "inherit"', t => {
const {stdout} = $({input: 'foo'}).sync`stdin-script.js`;
t.is(stdout, 'foo');
});

test('$ stdin has no default value when stdio is set', t => {
t.true(isStream($({stdio: 'pipe'})`noop.js`.stdin));
});
5 changes: 5 additions & 0 deletions test/fixtures/stdin-script.js
@@ -0,0 +1,5 @@
#!/usr/bin/env node
import {$} from '../../index.js';
import {FIXTURES_DIR} from '../helpers/fixtures-dir.js';

await $({stdout: 'inherit'})`node ${`${FIXTURES_DIR}/stdin.js`}`;
2 changes: 1 addition & 1 deletion test/helpers/fixtures-dir.js
Expand Up @@ -4,7 +4,7 @@ import {fileURLToPath} from 'node:url';
import pathKey from 'path-key';

export const PATH_KEY = pathKey();
const FIXTURES_DIR = fileURLToPath(new URL('../fixtures', import.meta.url));
export const FIXTURES_DIR = fileURLToPath(new URL('../fixtures', import.meta.url));

// Add the fixtures directory to PATH so fixtures can be executed without adding
// `node`. This is only meant to make writing tests simpler.
Expand Down
26 changes: 25 additions & 1 deletion test/stream.js
Expand Up @@ -6,7 +6,7 @@ import test from 'ava';
import getStream from 'get-stream';
import {pEvent} from 'p-event';
import tempfile from 'tempfile';
import {execa, execaSync} from '../index.js';
import {execa, execaSync, $} from '../index.js';
import {setFixtureDir} from './helpers/fixtures-dir.js';

setFixtureDir();
Expand Down Expand Up @@ -71,13 +71,25 @@ test('input can be a Stream', async t => {
t.is(stdout, 'howdy');
});

test('input option can be used with $', async t => {
const {stdout} = await $({input: 'foobar'})`stdin.js`;
t.is(stdout, 'foobar');
});

test('inputFile can be set', async t => {
const inputFile = tempfile();
fs.writeFileSync(inputFile, 'howdy');
const {stdout} = await execa('stdin.js', {inputFile});
t.is(stdout, 'howdy');
});

test('inputFile can be set with $', async t => {
const inputFile = tempfile();
fs.writeFileSync(inputFile, 'howdy');
const {stdout} = await $({inputFile})`stdin.js`;
t.is(stdout, 'howdy');
});

test('inputFile and input cannot be both set', t => {
t.throws(() => execa('stdin.js', {inputFile: '', input: ''}), {
message: /cannot be both set/,
Expand All @@ -96,6 +108,11 @@ test('input option can be a String - sync', t => {
t.is(stdout, 'foobar');
});

test('input option can be used with $.sync', t => {
const {stdout} = $({input: 'foobar'}).sync`stdin.js`;
t.is(stdout, 'foobar');
});

test('input option can be a Buffer - sync', t => {
const {stdout} = execaSync('stdin.js', {input: Buffer.from('testing12', 'utf8')});
t.is(stdout, 'testing12');
Expand Down Expand Up @@ -125,6 +142,13 @@ test('inputFile can be set - sync', t => {
t.is(stdout, 'howdy');
});

test('inputFile option can be used with $.sync', t => {
const inputFile = tempfile();
fs.writeFileSync(inputFile, 'howdy');
const {stdout} = $({inputFile}).sync`stdin.js`;
t.is(stdout, 'howdy');
});

test('inputFile and input cannot be both set - sync', t => {
t.throws(() => execaSync('stdin.js', {inputFile: '', input: ''}), {
message: /cannot be both set/,
Expand Down
4 changes: 4 additions & 0 deletions test/test.js
Expand Up @@ -100,6 +100,10 @@ test('preferLocal: undefined with $', async t => {
await t.notThrowsAsync($({env: getPathWithoutLocalDir()})`ava --version`);
});

test('preferLocal: undefined with $.sync', t => {
t.notThrows(() => $({env: getPathWithoutLocalDir()}).sync`ava --version`);
});

test('localDir option', async t => {
const command = process.platform === 'win32' ? 'echo %PATH%' : 'echo $PATH';
const {stdout} = await execa(command, {shell: true, preferLocal: true, localDir: '/test'});
Expand Down