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

Already on GitHub? Sign in to your account

CLI Hooks ionic:serve:before is blocking ionic serve #2661

Open
dnmd opened this issue Aug 17, 2017 · 4 comments
Open

CLI Hooks ionic:serve:before is blocking ionic serve #2661

dnmd opened this issue Aug 17, 2017 · 4 comments
Labels

Comments

@dnmd
Copy link

dnmd commented Aug 17, 2017

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
Copy link
Author

dnmd commented Aug 17, 2017

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
Copy link
Contributor

imhoffd commented Aug 17, 2017

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
Copy link
Author

dnmd commented Aug 18, 2017

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 imhoffd added this to the CLI Future milestone Aug 29, 2017
@imhoffd imhoffd added this to Backlog 🤖 in Tooling 🔧 via automation Oct 9, 2018
@imhoffd imhoffd removed this from the CLI Future milestone Nov 15, 2018
@imhoffd imhoffd changed the title CLI Hooks ionic:watch:before is blocking ionic serve CLI Hooks ionic:serve:before is blocking ionic serve Nov 15, 2018
@imhoffd
Copy link
Contributor

imhoffd commented Nov 15, 2018

@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:

p.on('close', (code, signal) => {

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Tooling 🔧
  
Backlog 🤖
Development

No branches or pull requests

2 participants