CLI: Reduce the size of the dependencies tree #1345
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Follows b1a2552, which committed an unresolved merge conflicts.
Package 'findup-sync':
* Tree:
(96 packages)
|- detect-file@1.0.0
|- is-glob@3.1.0
\ ...
|- micromatch@3.1.10
\ ...
|-resolve-dir@1.0.1
\ ...
* Summary:
We used it in one place only, for finding a 'package.json' file.
The findup-async library seems to only offer features we don't need.
For example:
- 'detect-file' provides a case-insensitive version of `fs.stat`.
- 'is-glob' and 'micromatch' helps provide a pattern-based search.
- 'resolve-dir' allows the start directory to contain tilde (~)
for `$HOME` and `@` for package-bound traversal.
For our use case, we only needed a loop that calls the built-in
`require('fs').stat()` and `require('path').dirname()`.
On second thought, I realised that we can avoid this entire problem
by simply using `require('<module>/package.json')`. Which allows us
to directly include a file from the module directory.
Package 'exists-stat'
* (1 package, no sub-dependencies.)
* Summary:
We used it in one place only. Added the function to cli/utils
directly.
Package 'walk-sync'
* Tree:
(7 packages)
|- ensure-posix-path@1.0.2
|- matcher-collection@1.0.5
|- minimatch@3.0.4
|- brace-expansion@1.1.11
|- balanced-match@1.0.0
|- concat-map@0.0.1
* Summary:
We used in two places.
I created a simplified version (directly based on Minimatch)
in cli/utils and updated callers.
Ref #1342.
Similar to the previous commit, I considered a simple approach of our own based on the built-in Node.js API. The fs.watch() API is actually quite good in Node 6+ (certainly much better than it used to be). But, there are two notable issues that I think we should care about: 1. Its "recursive" feature is lacking on Linux (only stable on macOS and Windows). 2. Its ability to distinguish between create, update and remove events isn't very good. This last point would be fairly easily to do on the consumer side with a quick fs.stat() call. We could even omit it entirely given we only use it for one word (a verb) in the CLI output. But the first point (recursion) is slightly more involved than I'd like to maintain locally. All the reviewed packages essentially handle this the same way. They create a non-recursive fs.watch() for each sub directory found on the system, track them in an object. Then, start new ones as needed when new directories are created, and stop old one when directories are removed. That's about 100 lines of simple code. Where they differ is: * How many extra features they provide. * How many simple functions for non-critical code are delegated to other packages. * Whether they use native "recursive" when available (on macOS/Windows). privide in addition to that I'm proposing we go with node-watch. This package is well-maintained, tested with the latest Node versions, optimised for Node 6+, and provides no additional features, and is dependency-free. > sane@4.0.2 (current) > Dependencies:⚠️ > 119 packages. > (concerning in terms of dicipline and security) > License: ✅ > MIT. > Supported: ✅ > 2018 saw 25 commits, 8 contributors, 2 major releases. > Modern: *️⃣ > Requires Node 6+, but not yet tested on Node 10. > gaze@1.1.3 > Dependencies: *️⃣ > 14 packages. > (okay, but could be better.) > License: ✅ > MIT. > Supported: ✅ > 2018 saw 22 commits, 6 contributors, 1 release. > Modern: ✅ > Requires Node 4+, tested with Node 10. > watch@1.0.2: > Dependencies: ✅ > 3 packages. > License: ✅ > Apache-2.0. > Supported:⚠️ > 2018 saw no commits or releases. > Modern:⚠️ > Still supports Node 0.1. No visible CI or commit activity > indicating testing with recent Node releases. > node-watch@0.5.9: > Dependencies: ✅ > 1 package (dependency-free). > License: ✅ > MIT. > Supported: ✅ > 2018 saw 22 commits, 3 contributors, 3 minor releases. > Modern: ✅ > Requires Node 6+, tested with Node 10. > Uses native fs.watch/recursive support where available. > filewatcher@3.0.1: > Dependencies: ✅ > 2 packages (1 dependency). > License: ✅ > MIT. > Supported:⚠️ > 2018 saw no commits or releases. > Modern:⚠️ > Last tested with Node 0.10. > Uses per-file watching and polling for all platforms. Fixes #1342.
|
@stefanpenner thoughts on the replacement of |
|
Landing per @platinumazure, but will monitor impact closely, especially with regards to cross-platform support. Let me know about any issues at #1342. Quite open to other approaches as well! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages.
Fixes #1342.