Skip to content

npm run scripts: wait for the spawned process to finish #15

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

wilzbach
Copy link

& causes the command to be run in the background. whereas && waits for the command to exit successfully.

As far as I can tell, it could have been intended to use &. If that's the cause using wait might be beneficial as (1) it shows the clear intention of running programs in parallel to the reader and (2) the CLI actually waits for the processes to finish and not suddenly dumps texts after a few seconds.
Though I'm not sure whether wait will work on Windows.
A better alternative might be concurrently.

See also:

@T4rk1n
Copy link
Contributor

T4rk1n commented Oct 11, 2018

It doesn't change anything for the commands you changed, the & will make the commands run simultanously whereas && wait for the other command to finish. Those commands can run at the same time and it will be faster. If you look at build:py, you'll see that it does use && because it needs the output of the previous command before generating the python classes.

Using `wait` is beneficial as (1) it shows the clear intention of running programs in parallel to the reader and (2) the CLI actually waits for the processes to finish and not suddenly dumps texts after a few seconds.

See also: http://tldp.org/LDP/abs/html/x9644.html
@wilzbach wilzbach changed the title Fix potential typo in package.json: npm run scripts should use double ampersand npm run scripts: wait for the spawned process to finish Oct 11, 2018
@wilzbach wilzbach changed the title npm run scripts: wait for the spawned process to finish npm run scripts: wait for the spawned process to finish Oct 11, 2018
@wilzbach
Copy link
Author

Those commands can run at the same time and it will be faster.

Yep, I'm aware of it, but npm never waits for spawned processes which makes this pretty bad for scripting. The specific example I ran into is because I used a stupidly simple watcher which can easily lead to an infinite loop:

while true; do
    npm run build:all;
    inotifywait --event modify,create,delete,delete_self,close_write,move,move_self -q **/*.js ;
done

The problem is that after the npm run build:all command has exited, the watch loop continues, but as the build process is still running, files are still written which trigger the watcher and it jumps to the first line again.

I rebased the PR with proposal (2) (using wait), but as it wouldn't work on Windows, feel free to close this if Windows support is important to you. I just thought I dog-food this repo with issues I run into.

@T4rk1n
Copy link
Contributor

T4rk1n commented Oct 16, 2018

Merged #14, if you want to add a watch in a new PR it would have to support windows.

@T4rk1n T4rk1n closed this Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants