Remove --inspect & --inspect-brk from execArgv
#435
Conversation
| const { | ||
| nodePath = process.execPath, | ||
| nodeOptions = defaultExecArgv | ||
| } = options; |
There was a problem hiding this comment.
Any thoughts on doing it no matter what? I don't really see any good reason users would want to override it.
There was a problem hiding this comment.
Sometimes you want to set inspect on a subprocess when your main process is just a small wrapper. We could also tell people to move towards https://nodejs.org/api/inspector.html instead for this functionality.
I'm okay either way, just remember that changing this would be a breaking change.
There was a problem hiding this comment.
Sometimes you want to set inspect on a subprocess when your main process is just a small wrapper.
Yes, it looks to me that could be the case, and this change would then be a breaking change if any users is doing this. Based on this, I personally think only doing this as a default value (what the PR currently does) might be better?
sindresorhus
changed the title
--inspect & --inspect-brk from execArgv
There was a problem hiding this comment.
This looks good to me 👍
Thanks @wardpeet!
What do you think @sindresorhus ?
|
Thanks! I wasn't familiar with ava so sorry for taking so long :) |
|
Nice work, @wardpeet |
When using
execa.nodeand the parent process has --inspect or --inspect-brk defined, it will run the child process with those options as well. It will throw an error that the inspector already is running on port 9229.I've added a filter for those options on the default execArgv value. I tried to test it but I'm not sure this is the correct way to do so.