search

denali-js / cli

The official CLI for starting, building, and running Denali apps and addons
MIT License
5 stars 5 forks source link

CLI Spinner should be made synchronous #22

Closed acburdine closed 7 years ago

acburdine commented 7 years ago

The spinner currently resides in a child process, however this approach introduces several issues esp. because broccoli emits events on build, which makes it difficult to queue promises (see #19)

The spinner (and all of its usages) should be rewritten so that it is not run in a child process and is run synchronously. The only real downside is that broccoli can block the spinner on build.

davewasmer commented 7 years ago

It's a bummer - I'd prefer to have a responsive spinner regardless of what's blocking the CPU in the build. Unfortunately, the implementation here is rife with complexity, as @acburdine pointed out.

So yea - in the interest of focusing on what's important here, lets move to an in-process spinner, but if anyone feels adventurous, or finds a good library to handle the async, inter-process queuing and communication - feel free to mention, I'd love to restore this.

acburdine commented 7 years ago

Closing this because #25 is merged which fixes most, if not all, of the problems with asynchronous spinners. If this comes back to bite later, we can always re-open this.