Repository navigation
ref(clack-utils): Use child_process spawn instead of exec when installing package - #859
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #859 +/- ##
=======================================
Coverage 51.54% 51.55%
=======================================
Files 53 53
Lines 3618 3629 +11
Branches 854 855 +1
=======================================
+ Hits 1865 1871 +6
- Misses 1623 1628 +5
Partials 130 130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
child_process spawn instead of exec when to install packagechild_process spawn instead of exec when installing package
| JSON.stringify( | ||
| { | ||
| stdout, | ||
| stderr, | ||
| }, | ||
| null, | ||
| 2, | ||
| ), |
There was a problem hiding this comment.
Dunno if it's worth doing this change. The output is an object with two gargantuan unreadable strings anyhow 🤷♂️
There was a problem hiding this comment.
fair enough :) reverted for now
| const installProcess = childProcess.spawn( | ||
| pkgManager.name, | ||
| installArgs, | ||
| { shell: true }, |
There was a problem hiding this comment.
If we add stdio: ['pipe', 'ignore', 'pipe'] it fixes #851 but we cannot then listen on stdout further down the line.
Seems okay to me to do, wdyt?
36f5c58 to
9bdfe7a
Compare
This PR refactors our
installPackagehelper to usespawninstead ofexec. While using spawn is a bit more complicated, it gives us finer control over the the child process lifecycle as well as runtime. This is primarily made because I want to attempt fixing #851.Additionally, we now send an error to Sentry when this happens to keep track how often this would occur. Previously, such errors weren't captured by Sentry since we try/caught the promise rejection.
I'll open another follow-up PR to split up the
clack-utilsfile. This thing has become huge and no longer is easy to maintain.