remove is-windows dependency #207
Conversation
Codecov Report
@@ Coverage Diff @@
## master #207 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 4 +1
Lines 68 69 +1
Branches 17 18 +1
=====================================
+ Hits 68 69 +1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #207 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 4 +1
Lines 68 69 +1
Branches 17 18 +1
=====================================
+ Hits 68 69 +1
Continue to review full report at Codecov.
|
|
Recently there has been a lot of controversy within the community with these one-liner packages which depend on each other and fill our |
|
No problem, it was very easy. Yesterday I submitted a PR to remove shebang-regex from another package which is an even bigger offender than is-windows (the regex is only 9 bytes while the packages is in total 2 828 bytes, 314 times bigger). Hopefully with the next standard library proposal more and more of these packages will disappear and be replaced by the standard library . |
|
Hopefully yes. The big problem is when maintainers disagree with inlining one-liners and actively fight against it (see moxystudio/node-cross-spawn#102). In that case, the only course of action left is to fork the offending package, inline the dependencies, release it and build trust, and then convince the dependants to switch over the fork. Which is simply too much effort, so we're left in the current state. |
|
Looks like nice-try and semver were removed. But the package has not been updated on npm yet. Soon cross-env will depend on two less sub-dependencies which is a good thing ! |
What: Removed "is-windows" from package.json and replace it by a one-liner function
Why: The actual function of is-windows is very and short and simple, it contains only one line (https://github.com/jonschlinkert/is-windows/blob/master/index.js#L25).
When downloading is-windows from npm, npm/yarn also downloads the LISCENCE and package.json file which are much bigger than the actual wanted code, as such it's much better to directly include the function in the code than to pull in a package.
Also more packages means more risk, here's an article talking about it :
I’m harvesting credit card numbers and passwords from your site. Here’s how.
An incident has already happened this year with the package event-stream :
Malicious code found in npm package event-stream downloaded 8 million times in the past 2.5 months
How: removed the dependency from package.json and added the one liner to is-windows.js, modified the tests to make it work with the new file
Checklist:
I had trouble making the tests work, I feel like having is-windows be a function instead of constant not ideal but it's mandatory if the tests want to mock the OS (unless there is a way to mock a constant)