Idea: consider functions that return instances of ChildProcess

[pkozlowski-opensource]

As of today the async function can mark its completion by calling a callback, returning a stream or a promise etc. How about adding instances of ChildProcess (http://nodejs.org/api/child_process.html#child_process_class_childprocess) to the supported list.

Having built-in ChildProcess support would allow us to have functions that spawns processes as Gulp4 tasks. The async-done could work with ChildProcess instance as follows:

• listen to the 'exit' event and mark the task as success / error based on the exit code
• use domains as for other event emitters to handle errors

Not sure if the exact technical solution is easy / feasible but it would be kind of cool to be able to spawn a process and have a Gulp4 task dependent on the completion of this process.

WDYT?

[phated]

I've definitely been considering this. Spawning is something a lot of blacklisted plugins do, so why shouldn't people just be allowed to do it themselves.

The weird thing about child_process is that if 2 use the same stdout, an exit event won't be fired until both have completed. I think this use case is supported by the end-of-stream module (it even handles a duplex request stream!).

[pkozlowski-opensource]

@phated OK, glad to hear that you've been considering this. I might give it a go over the weekend but if anyone else is faster than the better :-)

[phated]

@pkozlowski-opensource i'd be interested for you to try returning a child_process.spawn from the latest gulp, orchestrator is coming close to the async-done feature set and even uses end-of-stream internally.

[phated]

@pkozlowski-opensource would you be able to write a test for this? I think it should just work out of the box.

[pkozlowski-opensource]

@phated I guess my textual description of the issue wasn't clear enough, maybe I can better demonstrate what I mean with the code example. So, this works out of the box, as this is standard callback-based async stuff:

asyncDone(function (done) {
  exec('node --version', {}, done);
}, function (err, result) {
  console.log(err, result);
});

but what I was after is this:

asyncDone(function () {
  return exec('node --version');
}, function (err, result) {
  console.log(err, result);
});

This is really not a big deal as the code is just minimally different but I've started down this path based on this entry in the doc: https://github.com/phated/async-done/blob/master/README.md#completion-and-error-resolution where it is stated that the return value can be either a stream or and EventEmitter.

I believe that the current implementation based on stream-exhaust won't do anything with a general EventEmitter (what should it do with it, after all?).

So, we've got 2 ways of resolving this issue:

• simply update the doc to state "Stream or EventEmitter returned" instead of "Stream or EventEmitter returned"; I'm perfectly fine with this;
• or detect instances of ChildProcess, subscribe to the error / exit events and call the asyc-done callback based on those events.

As I've said above, not a big deal, just need to make sure that docs are accurate.

[phated]

stream-exhaust will just return an EventEmitter if it doesn't have the property readable, so we should be fine there. I was just thinking we could have a test using return exec('echo 123') or something. I am not really into testing but I know we need it for gulp4, which is why I asked.

[phated]

PR submitted to end-of-stream to support the exit event that child_process streams emit. mafintosh/end-of-stream#2

[phated]

PR was merged and I just bumped end-of-stream module. Tests were added for cp.exec and cp.spawn