export child process constructor

[polotek]

This might not be a good idea, but I'm looking to subclass ChildProcess. I think it should be exported from the module unless there's a real good reason not to do it.

[bnoordhuis]

I think it should be exported from the module unless there's a real good reason not to do it.

It's the other way around: it should not be exported unless there's a really good reason to do it - and I don't see it, sorry.

[polotek]

Can you explain why you take this position? There's nothing special about the ChildProcess constructor other than the fact it is backed by a binding. It's no different from exposing Socket from net or Server from http. People have used these to great effect. And I think exposing it is in keeping with node being a low level platform where you get all the tools you need.

[bnoordhuis]

I don't want people to depend on what is essentially an implementation detail. Exposing the constructor means supporting it indefinitely. Legacy is the only reason net.Socket and net.Server are exposed, I'd axe them if I could.

[polotek]

In general this is a good position to take, but I don't think this fits in js land. Servers, sockets, processes. These are objects. They're specific objects with a contract associated with them. The way you represent that is with a constructor and prototype. The way you access the API is via constructor. If you want to subclass, monkey-patch or mixin, you wanna use the constructor prototype. This isn't an implementation detail. It's part of the interface you program against. Even if the implementation changes, there will still be a prototype for these objects and there should probably be a constructor as well.

Or put another way. If it's an implementation detail, then any implementation should probably look similar. And if it changes significantly, hiding the constructor won't save you from breaking lots of programs.

On Dec 27, 2011, at 10:06 AM, Ben [email protected] wrote:

I don't want people to depend on what is essentially an implementation detail. Exposing the constructor means supporting it indefinitely. Legacy is the only reason net.Socket and net.Server are exposed, I'd axe them if I could.

---

Reply to this email directly or view it on GitHub:
#2419 (comment)

[isaacs]

I'm with @bnoordhuis on this.

The "good reason not to do it" is that every function and class that we expose is an API that has to be supported. Often it seems trivial to expose something if it could be useful. However, switching something from an "internal implementation detail" to "public documented part of the official API" is in fact a decision that increases the maintenance cost of node.

I think at this point in node's maturity, we need to start reining in the api surface, rather than expanding it. That's not to say that nothing can ever change, but increases in surface area need a very high degree of justification.

[mikeal]

I agree and disagree with all of you.

Yes, core should not solidify the API for a constructor until we are confident it won't change again.

No, core should not hesitate from exposing core constructors for consumption.

Solution:

exports._ChildProcess = ChildProcess

It's an internal property that is exposed. Modifying it is likely to break shit, use at your own peril because the API is subject to change.

[yorkie]

Hey guys there, sometimes we maybe do useful check for child_process object like this:

function xxx(cp) {
  if (!(cp instanceof ChildProcess))
    throw new Error('ChildProcess required');
  //...
}

Even though we could get this in another way, but it's much better for reader, I also know the API exposure problem is very important, maybe we need do it as @mikeal said?