search

shelljs / shelljs

:shell: Portable Unix shell commands for Node.js
https://www.npmjs.com/package/shelljs
BSD 3-Clause "New" or "Revised" License
14.25k stars 732 forks source link

Reduce quote escaping by using template literals #788

Open nfischer opened 6 years ago

nfischer commented 6 years ago

I noticed that we're escaping quote characters in some of our test descriptions (example). We should be able to avoid this by preferring template literals for such scenarios. The motivation is to improve readability.

We should also consider:

  1. Prefer template literals over string concatenation (eslint rule)
  2. Prefer template literals everywhere (I don't think there's any real downside) (eslint rule)
nfischer commented 6 years ago

To clarify, this should be limited to only our tests (because those use ES6). Once we drop support for node 4/5, we can expand this to cover the whole source tree.

Preferring template literals for all of the tests might be a bit harsh, but preferring them over string concatenation sound reasonable.

blubrick commented 4 years ago

I would be happy to have a crack at this gruntwork. It seems pretty simple, if a little tedious(hence the 'chore' label, I guess).

So I cloned the repo and ran npm install

Is that right - 51 package vulnerabilities? I figure I gotta be doing something wrong. (Ubuntu 18.04, node 12.7.0, npm 6.11.3)

Fortunately, it seems that only one of them is considered to be a SEMVER WARNING. The remaining 50 (lots of dupes) can be resolved via npm audit fix upgrading 6 packages, but then my build env will not not be the same as the repo.

Do you think I should raise this as a separate issue?

nfischer commented 4 years ago

@blubrick yes, please file a separate issue. I'm happy to take a look at the report (and would also appreciate PRs to update flagged packages if doing so does not break the project).

nfischer commented 4 years ago

Do you think I should raise this as a separate issue?

Filed as #969