build(lint): disallow node process api, favor Toolkit ChildProcess api #6113
Conversation
| const goPath: string = JSON.parse( | ||
| (await ChildProcess.exec('go', ['env', '-json'], { collect: true })).stdout | ||
| ).GOPATH |
There was a problem hiding this comment.
Use a local var is a bit more terse and readable, at the cost of having to claim a name.
But generally, invoking a process almost always should involve checking an exit code (though that's out of scope for this PR), so inlining exec() or run() is unusual and likely discouraged. .
| const goPath: string = JSON.parse( | |
| (await ChildProcess.exec('go', ['env', '-json'], { collect: true })).stdout | |
| ).GOPATH | |
| const r = await ChildProcess.exec('go', ['env', '-json'], { collect: true }) | |
| const goPath: string = JSON.parse(r.stdout).GOPATH |
There was a problem hiding this comment.
Ideally checking the exit code would be part of some higher-level API provided by ChildProcess right?
There was a problem hiding this comment.
Ideally checking the exit code would be part of some higher-level API provided by ChildProcess right?
Failure handling in some way needs to be done by the caller, typically
|
This is good to have in place. We don't need to bother migrating any of the child_process cases for Q features, since most of those will use Flare. |
Co-authored-by: Justin M. Keyes <[email protected]>
…aws#6113 ## Problem contributors may use the low-level node `child_process` library, which lacks the instrumentation and robustness of the toolkit `ChildProcess` utility. Example: aws#5925 (comment) ## Solution - add the node `child_process` library as a "banned import" with a message pointing to our `ChildProcess` utility. - Migrate simple cases to use our `ChildProcess`, mark more complex ones as exceptions. ## Future Work - To migrate the existing cases marked as exceptions, we will need to add functionality to our `ChildProcess` api, such as enforcing a `maxBufferSize` on the output. - Additionally, some of the current uses of `child_process` are tied to implementation details in the `child_process` library that make it hard to change without a larger-scale refactor. These cases were marked as exceptions.
2 participants
Problem
contributors may use the low-level node
child_processlibrary, which lacks the instrumentation and robustness of the toolkitChildProcessutility.Example: #5925 (comment)
Solution
child_processlibrary as a "banned import" with a message pointing to ourChildProcessutility.ChildProcess, mark more complex ones as exceptions.Future Work
ChildProcessapi, such as enforcing amaxBufferSizeon the output.child_processare tied to implementation details in thechild_processlibrary that make it hard to change without a larger-scale refactor. These cases were marked as exceptions.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.