Skip to content
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

feature: use zx to improve scripting dev experience #271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zacaytion
Copy link

Description

Adds the zx package to improve the experience of shell scripting with node. zx provides useful wrappers around child_process, escapes arguments and gives sensible defaults.

ZX uses ES modules and prefers to use .mjs extension. If this is undesirable, the PR can be updated to use .js and wrap the scripts in void async function () {}(). The scripts could also be written in typescript as ZX can compile and execute .ts files

Performance impact

None, except when scripts are written in typescript. .ts files are passed to ts-node which is slower than calling node directly.

Security impact

N/A || unknown

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

@benjie
Copy link
Member

benjie commented Sep 9, 2021

Thanks for raising a pull request. Alas, I'm concerned that moving to shell commands, e.g. using rm -rf instead of rimraf, will decrease our Windows compatibility. I also see negligible benefit in adding this extra dependency and all its dependencies when it seems most of its functionality is already provided by node natively just with slightly different syntax. I'm going to leave the PR open because I think other people may find it useful (particularly people who only need to support unix... though if this were me I'd personally just use bash scripts directly) but I think it's unlikely I'll merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants