plotly / dash-component-boilerplate

Get started creating your own Dash components here.
269 stars 182 forks source link

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

Closed wilzbach closed 5 years ago

wilzbach commented 5 years ago

& 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 commented 5 years ago

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.

wilzbach commented 5 years ago

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 commented 5 years ago

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