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

Attach .subprocess to the execa promise #413

Open
sindresorhus opened this issue Feb 16, 2020 · 8 comments
Open

Attach .subprocess to the execa promise #413

sindresorhus opened this issue Feb 16, 2020 · 8 comments

Comments

@sindresorhus
Copy link
Owner

Node.js does this now for childProcess. when you promisify it.

https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback

The name is to be decided.

One day, I hope we can get rid of execa() being both a childProcess and promise and just have the .child property on the promise. But that's a huge breaking change. For now, the best we can do is to add it, and recommend new users to use it instead of the old way. We could even un-document the old way.

@ehmicky Thoughts?

@ehmicky
Copy link
Collaborator

ehmicky commented Feb 17, 2020

Yes this sounds good 👍

@sindresorhus
Copy link
Owner Author

Any thoughts on the name?

  • .child
  • .process
  • .subprocess
  • .childProcess

@ehmicky
Copy link
Collaborator

ehmicky commented Feb 17, 2020

.child since this is the name used by Node.js util.promisify(childProcess.exec)?

@sindresorhus
Copy link
Owner Author

👍

@ehmicky
Copy link
Collaborator

ehmicky commented Feb 22, 2020

@wesleytodd Based on your comment here, would you like to take a look at doing a PR for this feature? Please let us know if you need any help!

@wesleytodd
Copy link

I plan to, just have other priorities at work. If someone else gets to it before me that’s fine, my guess is I will be able to do it late next week at the earliest.

@ehmicky
Copy link
Collaborator

ehmicky commented Mar 8, 2024

Per this comment, we are going to name the property subprocess instead.

@ehmicky ehmicky changed the title Attach .child to the execa promise Attach .subprocess to the execa promise Mar 12, 2024
@ehmicky
Copy link
Collaborator

ehmicky commented May 2, 2024

We probably should keep the following specific methods on the returned promise, as opposed to the promise.subprocess property: .pipe() and [Symbol.asyncIterator]/.iterable(). That's because when those methods are called, the promise is awaited indirectly/differently. So those methods are more related to the promise than to the subprocess instance.

This leads a much more natural syntax.

const result = await execa`npm run build`
  .pipe`sort`
  .pipe`head -n 2`;

for await (const line of execa`npm run build`) {
  // ...
}

for await (const line of execa`npm run build`.iterable().drop(3)) {
  // ...
}

Instead of:

const result = await execa`npm run build`
  .subprocess.pipe`sort`
  .subprocess.pipe`head -n 2`;

for await (const line of execa`npm run build`.subprocess) {
  // ...
}

for await (const line of execa`npm run build`.subprocess.iterable().drop(3)) {
  // ...
}

All the other properties/methods though can be on the .subprocess property.

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

No branches or pull requests

3 participants