Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open issues on Node.js #32

Open
8 tasks
sindresorhus opened this issue Aug 23, 2024 · 5 comments
Open
8 tasks

Open issues on Node.js #32

sindresorhus opened this issue Aug 23, 2024 · 5 comments

Comments

@sindresorhus
Copy link
Owner

sindresorhus commented Aug 23, 2024

Some ideas:

  • Improved errors
    • Include command
  • Buffer a Node.js stream to a string:

    nano-spawn/index.js

    Lines 86 to 97 in 8bdc484

    const bufferOutput = async (stream, result, streamName) => {
    if (!stream) {
    return;
    }
    stream.setEncoding('utf8');
    result[streamName] = '';
    stream.on('data', chunk => {
    result[streamName] += chunk;
    });
    await finished(stream, {cleanup: true});
    };
  • inheritEnv option
  • Duration on the ChildProcess and errors
  • Windows stuff: Windows support #12
  • child_process/promises.execFile with:
    • Does not throw on non-0 exit code or signal termination
    • Does not handle subprocess.std* stream errors, which leads to unhandled promise rejections
    • Opportunity to also add .output (interleaved).
  • The stdout/stderr output is only buffered on success, not failure
  • promise.child is a ChildProcess that might not have been spawned (e.g. if the command was misspelled). With nano-spawn/pico-spawn, it is a promise that is rejected in such cases.

Thoughts? Anything missing?

@ehmicky
Copy link
Collaborator

ehmicky commented Aug 23, 2024

Buffer a Node.js stream to a string

They might argue that this is available with execFile(). The thing with it though is that it does not give you access to the subprocess instance.

@sindresorhus
Copy link
Owner Author

Buffer a Node.js stream to a string

I meant a general Node.js stream utility like Readable#toArray().

The thing with it though is that it does not give you access to the subprocess instance.

It's returned in the callback version and available as a properly in the promise version.

@ehmicky
Copy link
Collaborator

ehmicky commented Aug 23, 2024

I meant a general Node.js stream utility like Readable#toArray().

There is node:stream/consumers text(). We can't use it in nano-spawn though because it uses stream.read() under-the-hood, which prevents promise.stdout iteration (since it uses stream.read() too under-the-hood, and that cannot have multiple consumers at once, unlike stream.on('data')).

It's returned in the callback version and available as a properly in the promise version.

execFile() has a special promisified version when using util.promisify(execFile). They probably should expose it under some node:child_process/promises module though.

@sindresorhus
Copy link
Owner Author

There is node:stream/consumers text(). We can't use it in nano-spawn though because it uses stream.read() under-the-hood, which prevents promise.stdout iteration (since it uses stream.read() too under-the-hood, and that cannot have multiple consumers at once, unlike stream.on('data')).

Maybe we could propose some kind of multicast() utility, which would make a stream multi-consumable even when stream.read() is used? Basically, a JS version of https://developer.apple.com/documentation/combine/publishers/multicast

I tried my hand and making something for this: https://github.com/sindresorhus/multicast-stream

@sindresorhus
Copy link
Owner Author

I think a 'child_process/promises'#.execFile proposal would be really useful. Let me know if you're interested in collaborating on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants