mysticatea / npm-run-all

A CLI tool to run multiple npm-scripts in parallel or sequential.
MIT License
5.72k stars 240 forks source link

npm-run-all slurps stdin #166

Open cjolowicz opened 5 years ago

cjolowicz commented 5 years ago

The following should print hello after running test but doesn't.

echo hello | ( node_modules/.bin/run-p test; cat )

run-p should not read from stdin unless it's using it. This behaviour breaks use cases where we read lines from stdin and want to invoke run-p on each line.

TehShrike commented 5 years ago

I don't think that's how stdin works... you're sending hello to run-p even if run-p doesn't care about stdin.

You're asking for run-p to read in everything from stdin, and then pipe it to stdout, which seems a bit unusual

cjolowicz commented 5 years ago

Something in run-p reads from stdin and throws away the result. That's not good practise for a command line utility because it breaks shell scripts. Try any other command, and see how it behaves:

echo hello | ( grep foo /dev/null ; cat )

echo hello | ( node_modules/.bin/eslint src ; cat )

These all leave stdin untouched.

cjolowicz commented 5 years ago

Here's some context:

https://stackoverflow.com/questions/55751619/why-does-this-fish-while-loop-terminate-after-a-single-iteration

cjolowicz commented 5 years ago

The issue is caused by the way npm-run-all handles process.stdin when creating a child process. The correct method would be to let the child process inherit stdin from the parent. Instead, the runTask function opens a pipe to the child process when stdin is not a tty:

https://github.com/mysticatea/npm-run-all/blob/5017a8879af4afead7a87d378ac0cca5393e7e43/lib/run-task.js#L79-L80

It will then immediately start forwarding data from its own stdin to the write end of the pipe:

https://github.com/mysticatea/npm-run-all/blob/5017a8879af4afead7a87d378ac0cca5393e7e43/lib/run-task.js#L177-L178

See https://github.com/mysticatea/npm-run-all/issues/24 and https://github.com/nodejs/node/issues/5620 for the rationale. Apparently this is a workaround for an issue with Git Bash on Windows.

Unfortunately, this behaviour has several issues:

Here are two short reproducible examples.

This example should invoke run-s twice, but terminates after the first iteration because the pipe is drained by the first run-s:

echo '{"scripts": {"test": "true"}}' > package.json

npm install --no-progress --silent npm-run-all

( echo first ; echo second ) | while read line ; do
    echo "==> $line <=="
    node_modules/.bin/run-s test
done

This example should print hello but does not print anything because the input was already sent to the first process:

echo '{"scripts": {"test:first": "true", "test:second": "cat"}}' > package.json

npm install --no-progress --silent npm-run-all

echo hello | node_modules/.bin/run-s test:*

My environment:

$ uname -srm
Darwin 18.5.0 x86_64

$ bash --version
GNU bash, version 5.0.3(1)-release (x86_64-apple-darwin18.2.0)

$ npm --version
6.7.0

$ node --version
v11.14.0

$ node_modules/.bin/npm-run-all --version
v4.1.5