Update execa libdef

[callumlocke]

This updates the new execa libdef to make it compatible with Promise.all().

• The first commit was a mistake, and the second commit reverts it.
• The third commit (eef807d) adds a failing test case.
• The fourth commit (27a5373) fixes it.

The downside is you no longer automatically get the ExecaError type when you catch an error, so you have to explicitly type it like .catch((err: ExecaError) => ... if you need the typing, or it will be typed as any. But I think this is probably more correct anyway, as there's no hard guarantee that anything passed to an execa's .catch() will always be the expected error type.

Pinging @jirutka for comment.

[jirutka]

This is wrong; execa does not return a real Promise; it just adds methods then and catch to the ChildProcess object, but not finally, so it's not fully compatible with Promise.

I disagree with this change, because it makes the definition incorrect.

I recommend to fix execa to define all Promise's methods on the returned object, so we can treat it as a union of ChildProcess and Promise.

[callumlocke]

@jirutka I see, that makes sense. But this is still is a problem then:

async () => {
  const children: ThenableChildProcess[] = [1, 2, 3].map(x => execa(`echo ${x}`))
  children.forEach(child => child.stdin.end('unicorns'))
  const results = await Promise.all(children);
  (results: Result[]) // INCORRECT FLOW ERROR
};

Do you have any ideas?

Either we incorrectly mark ThenableChildProcess as being fully compatible with Promise, or we incorrectly mark it as incompatible with Promise.all(). Neither is good.

I guess the best solution would be to ask @sindresorhus if he objects to making it a real promise somehow, or at least just adding a finally method. Or maybe it's a bug in Flow, and its await Promise.all() logic needs to be fixed to work with simple duck-thenables as well as proper Promise objects, as per the ECMAScript spec?

[callumlocke]

He says he's happy to add a .finally() method to execa. If we do that, are you happy with this PR in theory?

[gantoine]

Waiting on @jirutka, ping me when this is ready to move forward.

[bradfordlemley]

I've submitted sindresorhus/execa#174 to make if fully compatible with Promise.

The updated typedef should extend Promise instead of redefining Promise methods.

Something like:

  declare interface ExecaPromise extends Promise<Result>, child_process$ChildProcess {
    catch<E>(
      onrejected?: ?((reason: ExecaError) => E | Promise<E>)
    ): Promise<Result | E>;
  }

[AndrewSouthpaw]

Ping us when the PR is merged. :-)

[gantoine]

Closing in favour of a new PR.