CLI Hooks ionic:serve:before is blocking ionic serve

[dnmd]

Description:

Defining an ionic:watch:before hook with a blocking script (e.g. an express server startup) will block ionic serve. Since the close resemblance to npm hook scripts it is expected to execute in a similar way; where prestart is non blocking for start.

Steps to Reproduce:

Clone repo: https://github.com/dnmd/cli-hooks

First

$ npm install

---

Non-blocking behaviour

$ npm start

This will executed both the prestart and start script.

---

Blocking behaviour

$ ionic serve

This will only execute the ionic:watch:before script.

Behaviour

When running ionic serve, the ionic:watch:before script is executed and opens a new terminal window, but doesn't continue the execution of ionic serve. When running npm start, the prestart script is executed and opens a new terminal window and does continue the execution of start.

My ionic info: cli packages:

@ionic/cli-utils  : 1.8.1
ionic (Ionic CLI) : 3.8.1

global packages:

Cordova CLI : 7.0.1

local packages:

@ionic/app-scripts : 2.1.3
Cordova Platforms  : android 6.2.3
Ionic Framework    : ionic-angular 3.6.0

System:

Node : v6.6.0
npm  : 3.10.3
OS   : Windows 10

[dnmd]

Looking a bit closer at the execution of the ionic:watch:before, it seems that the await, L97 causes the pending state of the ionic:watch:before script.

[imhoffd]

prestart is not non-blocking for start. It is in fact imperative for build scripts to be able to do things sequentially. See this gist: https://gist.github.com/dwieeb/78d6937161fe6bccae814d0bf111e480

I'm guessing this has something to do with npm and the Ionic CLI handling the invocation of start (the Windows command) differently. npm may realize that start is not meant to block, like nohup for mac & linux.

[dnmd]

Agreed. The reason start is used was indeed to be able to run a parallel/concurrent script.

Using something like concurrently or npm-run-all requires both commands to be present for parallel execution (e.g. concurrently "node ./server.js" "ionic serve"), from within the context of ionic:watch:before one cannot write something like concurrently "node ./server.js" "command-that-triggered:before-hook"

Being able to run parallel/concurrent scripts from lifecycle hooks/events might be too much of a stretch here, I guess... :) If no further insights pop to mind, this issue can be closed.

[imhoffd]

@dnmd I know it's been a while, but I discovered why it hangs.

We're waiting for the 'close' event of a child process, not the 'exit' event: https://github.com/ionic-team/ionic-cli/blob/4dbe922233aa322b49fd820b1bb574a471b94500/packages/%40ionic/cli-framework/src/utils/shell.ts#L121

For almost any use case, this works great. The one case it doesn't is if the child process spawns a process that uses the same stdio. Because 'close' waits for those streams to be closed, and the grandchild process is still using them, the "all done" event doesn't bubble up, and the process is left hanging.

I forgot why I used 'close' instead of 'exit' long ago, but it was for a reason, and now I'm hesitant to switch everything to 'exit'.

I'll give this some more thought.