Handle nested arrays #362
Handle nested arrays #362
Conversation
es128
force-pushed
the
nested-arrays
branch
2 times, most recently
from
29aabc7
to
b210c88
Compare
| @@ -8,6 +8,8 @@ var globparent = require('glob-parent'); | |||
| var isglob = require('is-glob'); | |||
| var arrify = require('arrify'); | |||
| var isAbsolute = require('path-is-absolute'); | |||
| var flatten = require('lodash.flatten'); | |||
| var isString = require('is-string'); | |||
There was a problem hiding this comment.
do we really need a separate module for .filter(item => typeof item === 'string')?
|
What if we declare flatten, arrify and is-string as three small separate functions inside the main js file? That would reduce the number of dependencies used by chokidar. I think this is useful esp for our huge user base |
|
I think we're better off using externally maintained tiny dependencies than redefining them within the lib. These have way less impact as deps than 1) fsevents, and 2) all the stuff that comes with glob matching. |
|
looks good other than that |
|
Actually turns out I broke the test... fixing/rebasing now |
would be great to see some numbers related to this statement on a big scale. 3 deps = at least 6-12 more https requests. let's merge this right now though, we can do a research later |
|
don't merge quite yet, let me turn the CI checks green |
es128
force-pushed
the
nested-arrays
branch
from
4d2e66d
to
0e05aa8
Compare
|
Regarding the deps, I tend to agree with @sindresorhus' perspective If the size of the dep tree were a major concern, we'd make all sorts of choices differently, like anymatch using micromatch vs minimatch. But I don't think it should be thought of that way. We use modules for any common repeatable utility, even if simple, and just stick to just what we use, avoiding any mega kitchen sink libs. That said, yeah, maybe is-string in particular is too bloated for what we need here. I guess I'll replace it with a simpler inline implementation. |
|
is-string seems overkill |
es128
force-pushed
the
nested-arrays
branch
from
17c30f6
to
da180da
Compare
|
okok it's gone now |
| paths = arrify(paths); | ||
| paths = flatten(arrify(paths)); | ||
|
|
||
| function isString(maybeString) { |
There was a problem hiding this comment.
this would declare it on every add -- more memory usage etc.
es128
force-pushed
the
nested-arrays
branch
from
da180da
to
464e85d
Compare
|
@es128 @paulmillr will 1.2 be released today? |
|
yup, actually i'll take care of that now |
Also adds an explicit error message when bad (non-string or array of strings) input is provided
Resolves #360