Expose the ChildProcess constructor

[isaacs]

So I don't have to do stuff like this: https://github.com/isaacs/spawn-wrap/blob/master/index.js#L33-L38

[evanlucas]

Is there any reason it is not currently exposed?

[piscisaureus]

Prior discussion: nodejs/node-v0.x-archive#2419

[isaacs]

In the last 3 years, I've changed my mind on this, having had to jump through hoops to get at ChildProcess. It's not like it's actually hidden or private anyway, so we may as well save me the exec syscall to get it.

[evanlucas]

Thanks. Since we now we have internal modules, would it be worth refactoring ChildProcess into an internal module that is used by child_process and can be exposed via the --expose-internals flag?

[isaacs]

@evanlucas It's not an "internal module", though. It's a class in JavaScript-land. I need access to it in non---expose-internals code, and I can get it, but the fact that it's not exposed nicely makes that unpleasant.

[piscisaureus]

I think one of the problems is that the signature of the ChildProcess
constructor is internal / unspecified.

On Wed, May 20, 2015 at 3:07 PM, isaacs notifications@github.com wrote:

@evanlucas https://github.com/evanlucas It's not an "internal module",
though. It's a class in JavaScript-land. I need access to it in non-
--expose-internals code, and I can get it, but the fact that it's not
exposed nicely makes that unpleasant.

—
Reply to this email directly or view it on GitHub
#1751 (comment).

[vkurchatkin]

@isaacs can't you just wrap exports.spawn?

[evanlucas]

@isaacs good point.

[isaacs]

@vkurchatkin No. I'd have to also wrap exports.exec, exports.fork, etc.

@piscisaureus Ok, so, let's specify it :) It hasn't changed since (at least) 0.8, I think it's relatively stable at this point.

[chrisdickinson]

I'd support throwing the class into an internal module and then exposing it through the public child_process – it'd be a nice way to trim down on file length, plus we could take some of the sundry internal functions and expose them on the internal module so we can test 'em.

[isaacs]

@chrisdickinson I still need to be able to get at the class for spawn-wrap, though. Wherever the code lives, assert(child_process.spawn(..) instanceof child_process.ChildProcess) should be valid.

[chrisdickinson]

@isaacs Oh yeah, you would be able to – via require('child_process').ChildProcess – but this is an approach we could take to make sure we've got adequate coverage on the class before exposing it.

Edit: to be clear, I'm suggesting that we rip ChildProcess out of lib/child_process.js, throw it and the private functions it depends on into an internal module, then replace it in lib/child_process.js with var ChildProcess = exports.ChildProcess = require('internal/child_process'); so we can expand test coverage on it, which reduces risk for exposing it to the world.

[Fishrock123]

Edit: to be clear, I'm suggesting that we rip ChildProcess out of lib/child_process.js, throw it and the private functions it depends on into an internal module, then replace it in lib/child_process.js with var ChildProcess = exports.ChildProcess = require('internal/child_process'); so we can expand test coverage on it, which reduces risk for exposing it to the world.

Since we aren't exposing the private stuff anyway, I see no benefit to this?

EDIT:

and expose them on the internal module so we can test 'em.

Missed that, sorry. Yeah, +1