You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We should make signal names case-sensitive, always uppercase.
Currently, signal names passed to subprocess.kill() or the killSignal option are case-insensitive. This is inherited from Node.js behavior which was added in Node v8 in the following PR (nodejs/node#10423). This PR does not actually mention this change of behavior (although it's clear both the committer and the reviewer were aware of it), nor is this is mentioned in an issue.
The main cons is that this would be a breaking change.
That being said, this behavior is undocumented in both Execa and Node.js.
Also, the current behavior is inconsistent with process.kill() and os.constants.signals, which are case-sensitive and always uppercase. For example, passing 'sigterm' or 'Sigterm' to subprocess.kill() works, but passing it to process.kill() fails.
Furthermore, uppercase is the convention for signal names. For example, passing a lowercase signal name to the kill Unix utility does not work.
Finally, being stricter would allow us to provide with stricter typing. Typos in signal names are likely, especially with signals like SIGALRM, SIGSTKFLT, SIGCHLD vs SIGCLD (yes, those are two different signals...), SIGSTOP vs SIGTSTP (also two different signals) or SIGVTALRM. It'd be helpful to let TypeScript warn of those typos, but that's not possible if the value is case-insensitive.
We should make signal names case-sensitive, always uppercase.
Currently, signal names passed to
subprocess.kill()
or thekillSignal
option are case-insensitive. This is inherited from Node.js behavior which was added in Node v8 in the following PR (nodejs/node#10423). This PR does not actually mention this change of behavior (although it's clear both the committer and the reviewer were aware of it), nor is this is mentioned in an issue.The main cons is that this would be a breaking change.
That being said, this behavior is undocumented in both Execa and Node.js.
Also, the current behavior is inconsistent with
process.kill()
andos.constants.signals
, which are case-sensitive and always uppercase. For example, passing'sigterm'
or'Sigterm'
tosubprocess.kill()
works, but passing it toprocess.kill()
fails.Furthermore, uppercase is the convention for signal names. For example, passing a lowercase signal name to the
kill
Unix utility does not work.Finally, being stricter would allow us to provide with stricter typing. Typos in signal names are likely, especially with signals like
SIGALRM
,SIGSTKFLT
,SIGCHLD
vsSIGCLD
(yes, those are two different signals...),SIGSTOP
vsSIGTSTP
(also two different signals) orSIGVTALRM
. It'd be helpful to let TypeScript warn of those typos, but that's not possible if the value is case-insensitive.What do you think @sindresorhus?
The text was updated successfully, but these errors were encountered: