Add input option to async child_process methods

[sindresorhus]

Is your feature request related to a problem? Please describe.

When spawning a child process, it's common to want to send some data to the process' stdin. Usually, a string or buffer. This can be done by writing to the stdin stream:

const {execFile} = require('child_process');

const child = execFile('node', (error, stdout, stderr) => {
	if (error) {
		throw error;
	}
	console.log(stdout);
});

child.stdin.write('foo');
child.stdin.end();

However, that's a bit verbose and not that obvious. It's also easy to forget to call .end(), see #2339.

Describe the solution you'd like

I propose the async child_process methods get an input option for convenience. Just like the synchronous methods already have.

const {execFile} = require('child_process');

execFile('node', {input: 'foo'}, (error, stdout, stderr) => {
	if (error) {
		throw error;
	}
	console.log(stdout);
});

This will also improve the stdin situation when a child_process method is promisified, as it then returns a Promise<Object> with stdout and stdin instead of the ChildProcess object.

Describe alternatives you've considered

I could create a child_process wrapper that does this, however, I have already done that: https://github.com/sindresorhus/execa#input But I would like to upstream some of the most useful ideas from that package.

---

// @floatdrop @Qix-

[Qix-]

Sorry @sindresorhus but I'm 👎 on this.

I'm not sure what the official mindset of the Node core team is, but the way I've always treated Node core is a small wrapper around V8 (for javascript), libuv (for async I/O), and a subset of UNIX syscalls (shimmed on non-UNIX systems).

Nothing more. I'd even argue some of the (admittedly helpful) built-ins such as http[s2] are out of scope for Node core, but that's another discussion.

That being said, the standard input/output streams are just that - streams. They are character descriptors that cannot be seek'd or tell'd, unless you've re-mapped those file descriptors to something else.

That's why the .stdin stream makes a lot of sense. Unless you're re-mapping those file descriptors to something else, it should not be treated as a block of text - because that isn't how the child process is going to treat it (at least, not under the hood).

Further, .end() makes sense as well. It's a core part of the streaming API in node and is essential to properly signal EOF in consumers.

With this proposed feature, two things are happening:

• A second way to do the same low-level thing is introduced. This makes semantics unclear when combining both {input: foo} and .stdin.write(foo).
• You're shifting semantics from a stream to implying that stdin is some sort of block reader, which I foresee adding more confusion to users when they try to read from stdin in their child programs and they don't get the entirety of the data all at once.
• It's an attempt to 'fix' engineers who otherwise do not want to read documentation or understand the technologies they're using - which is not the job nor the fault of Node core.

So if the API design makes sense for how things actually work at a low level, and if the new feature causes confusion for those that use the low level facilities provided by child_process, then I don't see this as any net benefit.

Plus, execa is already a stellar module - albeit opinionated, which in most cases is just fine. I understand that this feature wouldn't remove anything, but I don't think execa-like functionality needs to be mainlined in just to appease a few confused users.

[mscdex]

-1 I don't see the value over simply doing child.stdin.end(input).

Having the input option for the synchronous methods makes sense for obvious reasons.

[sindresorhus]

@mscdex Any comments on:

This will also improve the stdin situation when a child_process method is promisified, as it then returns a Promise with stdout and stdin instead of the ChildProcess object.

[mscdex]

@sindresorhus Not really, I don't use Promises.

[szmarczak]

@Qix- Imagine if your thinking was applied to all Node modules. You'd have to make your own wrappers for literally everything. Also, many modules would have been gone completely.

A second way to do the same low-level thing is introduced. This makes semantics unclear when combining both {input: foo} and .stdin.write(foo).

Then throw an error. Problem solved :)

You're shifting semantics from a stream to implying that stdin is some sort of block reader, which I foresee adding more confusion to users when they try to read from stdin in their child programs and they don't get the entirety of the data all at once.

I see no confusion. For example, if you're trying to send 500MB over 100MB/s and you expect to get it all at once - it's obvious you'll get it in parts. That's how data transfer works.

It's an attempt to 'fix' engineers who otherwise do not want to read documentation or understand the technologies they're using - which is not the job nor the fault of Node core.

I disagree. For example, the http2 module ends the request stream immediately (by default).

The main point is to make things easier. Of course you can do everything on your own, but the question is: why? If there's something we can improve then let's do it.

[Qix-]

You'd have to make your own wrappers for literally everything.

Yes, and? That's how software works. You compose smaller pieces into bigger pieces.

Then throw an error. Problem solved :)

Using errors to mask poor API design is hardly solving problems..

That's how data transfer works.

And using EOF is how streams work. I don't see your point here.

The main point is to make things easier.

This doesn't make anything easier. This muddies the concept of stream vs. payload for the ease of a few use-cases that should be using wrappers anyway. Having all of your stdin payload ready and in memory to be transferred is an application decision that happens outside of node.

The fact is, the way stdin is structured right now is how it works at the low level. I don't see any point in changing it.

[szmarczak]

Yes, and? That's how software works. You compose smaller pieces into bigger pieces.

Well, if you love it then go assembly :D

Using errors to mask poor API design is hardly solving problems..

And we are here for that particular reason - to discuss the possibilities.

And using EOF is how streams work.

When someone passes an input option write data and send EOF. Simple.

I don't see your point here.

Please refer to your sentence:

adding more confusion to users when they try to read from stdin in their child programs and they don't get the entirety of the data all at once.

then read my answer again.

This muddies the concept of stream vs. payload for the ease of a few use-cases that should be using wrappers anyway.

And we are here for that particular reason - to discuss the possibilities.

I just want to note, if there weren't any small changes we still would be using Node 8.

[Trott]

I'm not sure what the official mindset of the Node core team is, but the way I've always treated Node core is a small wrapper around V8 (for javascript), libuv (for async I/O), and a subset of UNIX syscalls (shimmed on non-UNIX systems).

Speaking for myself only and not the project, but speaking as someone deeply involved for a few years:

Node.js has always had tension between the ideals of "small core" and "batteries included". This has frustrated some folks who want one of those two ideals to be the only ideal. And there have certainly been some missteps where Node.js has included stuff that many of us later wish it hadn't and now we're stuck with. But overall, I believe the success of Node.js is owed largely to striking a reasonably good balance between the ideals. (For example, and my opinion only: "Small core" would have omitted the http module but the http module has been essential to Node.js adoption. If we could remove it now and have it live as a separately-maintained npm module, that would be awesome. But doing that now would break more codebases than I can imagine. So we're stuck with it.)

What this means is that ultimately we make decisions about what to include and what to omit on a case-by-case basis, making the best judgment we can at the time with the information we have.

Personally, I'm still pondering this one.

[Qix-]

@Trott I don't see how this adds any functionality not originally in Node - nor does it really simplify an existing process.

To me, it feels like it's based entirely out of confusion around the streaming API. It's not node's job to solve that.

I don't see how this does anything more whatsoever.

[yordis]

@Qix- even when I understand that there is some technical reason why it works as it is today.

My only argument will be that we need to start moving the level of abstraction a little higher without losing the underline control.

Because it is closer to how low-level works shouldn't be the reason why it works that way today, to be honest.

But I also would prefer to keep Node as lean as possible, but this will require massive support from leaders and NodeJS foundation on directions and the creation of a good ecosystem then.

Yes, many people are aware of execa and other packages but at the same time NPM packages are the old wild west so trusting such of ecosystem becomes harder and harder and that is why I would prefer NodeJS to take the responsibility to raise the level of abstraction, but only because of that.

I am 50%, 50% on this.

It's not node's job to solve that.

I think it is NodeJS or V8 to solve this, the whole point of creating such of languages and abstraction is to move to the next level of abstraction don't you think?

[Qix-]

@yordis you contradict yourself. Either you have abstractions such as this, or you have a "lean core". You can't have both.

Secondly, wanting to kill off the concept of packages because you don't trust them isn't going to happen. Making node entirely batteries included would make it massive.

I think it is NodeJS or V8 to solve this, the whole point of creating such of languages and abstraction is to move to the next level of abstraction don't you think?

Not even a little bit, no. As I said in my first comment above, Node's point (as I see it) is to be a Javascript engine wrapping async unix system calls. Nothing more.

[yordis]

@yordis you contradict yourself.

Tell me more ---> I am 50%, 50% on this.

Making node entirely batteries included would make it massive

I don't want that.

Being mainly on Erlang/OTP and Elixir, I want these tools to be as lean as possible. OTP if I am not mistaking is starting to move things into packages so the core focus in does one thing really well.

But the community of Erlang/Elixir support these type of movements, this is why we need to start shaping a little bit JavaScript community.

Not even a little bit, no. As I said in my first comment above, Node's point (as I see it) is to be a Javascript engine wrapping async unix system calls. Nothing more.

Fair enough, this is 50% of what I love, the other 50% is forced by the community, but no because pragmatically speaking I want it on NodeJS.

[mcollina]

I’m in favor of this one as long as it is implemented as child.stdin.end(input). In this way the input option and stdin could not be misused.

[sholladay]

To me, it feels like it's based entirely out of confusion around the streaming API. It's not node's job to solve that.

@Qix- It is Node's job to provide a useful, intuitive, and reasonably powerful abstraction in its APIs, among which streams and child processes are important ones. Even if we take your viewpoint for granted, that this feature request is borne out of misunderstanding of the API, then this is an opportunity for the core team to learn. Why was it misunderstood by experienced Node.js users? That deserves some thought.

The numbers here are also interesting. execa has 5.5 million downloads per week on npm. To me, this signals that the child_process module in core is more than just "small", it's inadequate or broken. Clearly there are common needs not being met. Maybe "send some simple input to a process without streams" is one of them? It seems reasonable to me and the sync APIs already have it, so this only makes things more consistent.

The possibility for misuse also seems overblown to me. The input is obviously going to get sent first, since it's happening within the function call that launches the child process. As a user, I can't interact with it until that function returns control to me. In order to be confused about this, I would have to believe that input is going to be delayed somehow, and there is nothing that indicates that to me.

[Qix-]

@sholladay:

Why was it misunderstood by experienced Node.js users? That deserves some thought.

Because they haven't taken the time to learn how process I/O works at the system level. This isn't Node's fault.

To me, this signals that the child_process module in core is more than just "small", it's inadequate or broken.

This is confirmation bias. We do not have stats on how many people would have "downloaded" child_process, nor have we enumerated all of the possible use-cases for child process spawning. execa is filling a very specific use-case and taking a lot of opinions about how subprocesses are spawned.

Conversely, node's API is very much similar to the exec* suite of POSIX syscalls.

It seems reasonable to me and the sync APIs already have it, so this only makes things more consistent.

Because the sync APIs cannot function without it.

The possibility for misuse also seems overblown to me. The input is obviously going to get sent first, since it's happening within the function call that launches the child process. As a user, I can't interact with it until that function returns control to me. In order to be confused about this, I would have to believe that input is going to be delayed somehow, and there is nothing that indicates that to me.

This doesn't make sense to me, could you explain?

If the input is large enough to reach the kernel's buffer limits before the child process can consume it, it's not going to block the parent process if the parent process is using epoll or the like (in this case, libuv). That's not really an issue here...

[yordis]

Because they haven't taken the time to learn how process I/O works at the system level. This isn't Node's fault.

@Qix- Why those many people should care about how I/O works at a system level in 2019?

If you are not working in low-level software that requires would require such of knowledge this is pretty much pointless and contra-productive.

I see that you love to know how things work and you do not have issues with understanding low-level APIs. So I, I love as you do understanding how things work but pretending that everyone should be like us is selfish.

I want people like you that enjoy these topics working on the core of NodeJS or V8, but for the rest of us, the best we could do is to focus in the next layer of abstraction.
The layer that probably people don't understand that much but at least we create businesses and pay people like you to do these jobs since you are so passioned about it. Or build amazing tech on top of the fantastic work done by people like you.

Win-win.

If you are going to think that way, where people should know how everything works, well, we all should code in Assembly and be good at it. Right?!
No person in the right mind would propose such of thing in 2019.

This topic is where I find myself hating the idea that NodeJS is merely a wrapper.
I am tired that we have to keep repeating the same APIs cross languages and without even raising the level of abstraction.
We do not create anything new; we repeat the same task over and over and over.

I will stop here since this is just a philosophical/subjective topic and as I said, I am 50%/50% on adding these type of APIs to NodeJS itself instead of being on an external package as today. We are not a disciplined community to do so.

[Qix-]

I'm not going to respond to a reductio ad absurdum response such as that. No, obviously we shouldn't be writing pure assembly - that's a common argument people make in favor of needless abstraction. Talking down to me doesn't help further this conversation.

I will add that it's the responsibility of all of us to understand our field and to keep complexity to a minimum. I don't think this addition brings us any closer to that.

[tm8utv]

I think that it's too much abstraction.
At first look, I can't understand how it works.

[benjamingr]

Hey 👋

I'm not too happy with how discussion went down here - I feel like the idea proposed by @sindresorhus was shot down very quickly without first discussing alternatives and options.

I feel like valid and relevant technical points were raised but also some less constructive arguments. I'd love to see some constructive discussion on the pros and cons.

Note that we can do other things for the promises version of child_process like return a Promise with streams on it directly (rather than an object with those streams) or other things. We can have any API we want for the experimental promises version when it comes (pr welcome):

const { execFile } = require('child_process').promises;

// later in an async function, we might have an API like:
const child = await execFile('node').stdin.end('foo');
// or something like:
const child = await execFile('node');
child.stdin.write('foo'); // added to ChildProcess object returned from promises
// or something like:
const child = execFile('node').stdin.write('foo');
await child.ready // child is returned synchronously, exposes a property for the promise 

Or anything we'd like really.

[Qix-]

But are we discussing an addition to the Promises API or an addition to the existing API? Those are two very different things.

[mcollina]

@qix I think the underlining problem is how execFile, etc API works with async/await.

As an example, it's not possible to do:

const cp = require('child_process');
const { promisify } = require('util');
const execFile = promisify(cp.execFile)

async function run () {
  const { stdin, stdout, stderr } = await execFile('cat')
  stdin.end('hello') // this will never be executed
  return stdout
});

Overall execFile is completely non-promisifiable without an input option. I think adding such an option would just improve execFile.

I think input could be a string, a buffer, or a stream.

[Qix-]

What does cat output in this case?

const cp = require('child_process');
const { promisify } = require('util');
const execFile = promisify(cp.execFile);

async function run () {
  const child = execFile('cat', {input: 'hello'});
  child.stdin.end('world!');
  const { stdout, stderr } = await child;
  return stdout;
});

Hint: this script should throw an error.

This is the confusion I'm talking about.

[ljharb]

In general, i don’t think a promise-based version of a callback-based api in node should have a different API - that makes it harder to refactor between the forms. If a change is worth making, it’d be worth making in both interfaces, imo.

[davidmarkclements]

Nevermind, totally wrong, should have thought more, promise would have to be awaited to get I/O streams

I'm indifferent to whether the input option is added or not, but I don't think the promise argument is relevant. This is a common issue with `await` and promises that are tied to additional interaction, the solution in both cases is to use a different pattern:

const cp = require('child_process');
const { promisify } = require('util');
const execFile = promisify(cp.execFile)

async function run () {
  const sp = execFile('cat')
  const { stdin, stdout, stderr } = sp
  stdin.end('hello') 
  await sp
  return stdout
}

const cp = require('child_process');
const { promisify } = require('util');
const execFile = promisify(cp.execFile)

async function run () {
  const sp = execFile('cat', {input: 'hello'})
  const { stdin, stdout, stderr } = sp
  stdin.end('world!')
  await sp
  return stdout
}

Seems to me that the implementation would just call stdin.write(options.input), so for the second case, output would be "helloworld!".

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[szmarczak]

unstale

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[github-actions]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[sindresorhus]

This is still a desired feature. Can someone please reopen?

[github-actions]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

[sindresorhus]

Please keep this open.

[bnoordhuis]

I'm putting this out to pasture. It's been open for over 4 years and there's no consensus (mild understatement) on whether it's a desirable feature, let alone what the details should look like.