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

Improve types of result.stdout|stderr|all #681

Merged
merged 1 commit into from Jan 13, 2024
Merged

Improve types of result.stdout|stderr|all #681

merged 1 commit into from Jan 13, 2024

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Jan 12, 2024

Fixes #673.

Most of the time, we know that result.stdout|stderr|all is defined. However, we currently type it as possibly undefined. For example, the following fails becomes stdout is typed as string | undefined even though we know it cannot be undefined.

const { stdout } = await execa('echo')
console.log(stdout.trim())

The reverse is true: we sometimes know that result.stdout|stderr|all is undefined. Typing it as undefined instead of string | undefined would help TypeScript users realize they cannot use that property. For example, when using the buffer: false option or the stdout: 'inherit' option, users cannot retrieve result.stdout. The type should then be undefined, not string | undefined.

This PR implements this. The stream output is now correctly either undefined, string or Uint8Array.

This PR works with all the following cases:

  • New result.stdio property
  • stdio: string option
  • stdio: array option
  • stdio option with more than 3 items
  • buffer: false option
  • result.all property

Example from the type tests:

const { stdout, stderr, all, stdio } = await execa('unicorns', {stdio: ['pipe', 'ignore', 'pipe', 'pipe', new Uint8Array()]});
expectType<undefined>(stdout);
expectType<string>(stderr);
expectType<string | undefined>(all);
expectType<undefined>(stdio[0]);
expectType<undefined>(stdio[1]);
expectType<string>(stdio[2]);
expectType<string>(stdio[3]);
expectType<undefined>(stdio[4]);
// stdio[5] fails

| number
| Readable
| Writable
| [NoOutputStdioOption];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdio options which prevent Node.js from creating childProcess.std*, i.e. prevent us from retrieving the output.
Please note that when this does not apply when passing multiple values to this option. For example, result.stdout is undefined with { stdout: 'inherit' } but not with { stdout: ['pipe', 'inherit'] }. This PR accounts for this, and has a type test for it.

@@ -419,21 +493,21 @@ export type ExecaReturnValue<IsSync extends boolean, StdoutStderrType extends St

This is `undefined` if the `stdout` option is set to [`'inherit'`, `'ipc'`, `'ignore'`, `Stream` or `integer`](https://nodejs.org/api/child_process.html#child_process_options_stdio).
*/
stdout: StdoutStderrType;
stdout: StdioOutput<'1', OptionsType>;
Copy link
Collaborator Author

@ehmicky ehmicky Jan 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to how tuples are typed in TypeScript, we need to pass '1' not 1. That's because when mapping a tuple, the indices are iterated as strings, not numbers.

@ehmicky ehmicky force-pushed the std-types branch 2 times, most recently from 1663557 to d8ac65a Compare January 13, 2024 00:24
@sindresorhus sindresorhus merged commit b858c07 into main Jan 13, 2024
14 checks passed
@sindresorhus sindresorhus deleted the std-types branch January 13, 2024 09:56
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

Successfully merging this pull request may close these issues.

Invalid types for result.stdout|stderr|all
2 participants