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

Simplify passing options #13

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Simplify passing options #13

merged 1 commit into from
Aug 22, 2024

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Aug 22, 2024

This PR is a proposal to allow users to pass node:child_process native options directly at the top-level instead of namespacing them in options.nativeOptions.

Using options.nativeOptions is nice as it allows us to distinguish between nano-spawn curated options, and forwards "everything else" to node:child_process spawn().

That being said, I believe allowing native options at the top level could provide with more pros than cons:

  1. The most commonly used options are most likely going to be some of the native ones: stdout: 'inherit', timeout, signal, shell, env, cwd. Entering {nativeOptions: {timeout: 1000}} is more cumbersome than directly {timeout: 1000}.
  2. Exposing some of the native options at the top level (like we currently do with signal and timeout), but not others, requires typing and documenting them. It also adds a few bytes (although not much). Flattening it the top level would remove that, keeping the API and types small. Hopefully, nano-spawn will not have that many options (if any? let's see), so it'd be nice to be able to keep the API very small.

Please let me know what you think.

@ehmicky ehmicky force-pushed the simpler-options branch 2 times, most recently from beffce6 to ecb7ccb Compare August 22, 2024 18:24
@sindresorhus
Copy link
Owner

Yeah, I agree. It's probably easier this way.

@sindresorhus sindresorhus merged commit 14e44ca into main Aug 22, 2024
6 checks passed
@sindresorhus sindresorhus deleted the simpler-options branch August 22, 2024 18:41
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.

2 participants