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

Promise chains are flattened #409

Closed
wesleytodd opened this issue Jan 28, 2020 · 3 comments
Closed

Promise chains are flattened #409

wesleytodd opened this issue Jan 28, 2020 · 3 comments

Comments

@wesleytodd
Copy link

wesleytodd commented Jan 28, 2020

async function execThing (args) {
  const cmd = await getCmd()
  return execa(cmd, args)
}

const proc = await execThing(['--version'])
proc.stdout.pipe(process.stdout) // fails because proc.stdout is a string

Since a promise resolved with another promise will wait for the underlying promise to resolve, we are unable to pass the return of an execa call from a async function or resolve a promise with it. In this case it waits to unravel the promise chain.

In this example it crashes because proc.stdout is a string because the promise chain has be resolved. I am not sure how we would resolve this with just the current api surface. It seems like the most simple solution would be to add an option extendPromise: false to disable this behavior.

Is there a way to get this behavior with the current API I am not seeing? Thanks for the help in advance 😄

@ehmicky
Copy link
Collaborator

ehmicky commented Jan 30, 2020

We previously discussed with @sindresorhus that the fact that the returned value is both a child process and a promise might be an issue, for several reasons including your case.

Our conclusion was that changing it was unfortunately too much of a breaking change at this point. @sindresorhus Please let us know if I got this wrong.

A workaround is to wrap the return value in a plain object:

async function execThing (args) {
  const cmd = await getCmd()
  const proc = execa(cmd, args)
  return { proc }
}

const { proc } = await execThing(['--version'])
proc.stdout.pipe(process.stdout)

@ehmicky ehmicky closed this as completed Jan 30, 2020
@wesleytodd
Copy link
Author

I guess I will have to change the API exposed in my use case. If there is an ongoing discussion on this I would love to put in a vote that this be reconsidered if there is a reasonable proposal for alternate behavior. Do you have a link to there it was discussed?

@ehmicky
Copy link
Collaborator

ehmicky commented Feb 6, 2020

@wesleytodd Absolutely. The discussion was in #351, feel free to jump in.

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