-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
refactor: replace unmaintained shelljs dependency by execa #10358
Conversation
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: -1.05 kB (-0.01%) Total Size: 11.5 MB ℹ️ View Unchanged
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
Size Change: -1.06 kB (-0.01%) Total Size: 11.1 MB ℹ️ View Unchanged
|
Size Change: -1.07 kB (-0.01%) Total Size: 11.6 MB ℹ️ View Unchanged
|
} | ||
|
||
// Clear out any existing contents in the target directory | ||
shellExecLog(`git rm -rf ${targetDirectory}`); | ||
exec(`git rm -rf ${escapeArg(targetDirectory)}`, { |
Check warning
Code scanning / CodeQL
Unsafe shell command constructed from library input Medium
library input
shell command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
This replaces the unmaintained shelljs dependency by execa (v5 because upper versions are ESM only)
Fix #10348
I tested locally everything keeps working, but it's error-prone and shelljs/execa behave differently in terms of arg escaping.
Note: this PR doesn't replace sync by async, but tries to keep the same behavior as before by switching lib.
More refactoring can be done in other PRs, but considering all this is not well-covered by tests, it's risky and we should rather write tests first.
Motivation
Replace unmaintained shelljs dependency
Fix #10348 ?
Test Plan
Unit tests + preview
Test links
https://deploy-preview-10358--docusaurus-2.netlify.app/