Add/ignoring handler

[retrofox]

This PR avoids create chokidar watcher instances when the file matchs with the ignoring rule defined through --ignore-watch parameter so the idea try to is optimize resources.

[retrofox]

In my app when I run with current 3.1.0 version I get the follow:

1610040 bytes written to > "public/build.js" (7.52 seconds)
1610040 bytes written to > "public/build.js" (8.21 seconds)
1610040 bytes written to > "public/build.js" (8.65 seconds)
1610040 bytes written to > "public/build.js" (15.62 seconds)
1610040 bytes written to > "public/build.js" (5.70 seconds)

With this PR:

1610284 bytes written to > "public/build.js" (7.34 seconds)
1610284 bytes written to > "public/build.js" (0.64 seconds)
1610284 bytes written to > "public/build.js" (0.67 seconds)
1610284 bytes written to > "public/build.js" (0.65 seconds)
1610284 bytes written to > "public/build.js" (0.59 seconds)

I'm passing `--ignore-watch="/node_modules/" to watchify

[zertosh]

@retrofox My comment in the last paragraph of https://github.com/substack/watchify/pull/193#issuecomment-91344096 still stands. Can you implement it in the same way as browserify does?

Those sample stats look way off for a build that size. There is no way you're bundling 10k files in 1.6mb. Did you try what I suggested in https://github.com/substack/watchify/pull/193#issuecomment-91090285? The bit about removing the . (period)? Trying this command watchify index.js --ignore-watch -o public/build.js? Please try that and run test. What happens?

[retrofox]

@zertosh Hi dude. Thanks for your answer. Well ... I can't define only index.js. I don't why but I lose to keep watching some files. I guess it's because we have a lot of files in our structure and use only one file isn't enough to grab to all of them. Anyway this PR is independent of this results. Let me understand clearly what do you mean with browserify mode. Thank you for your patience.

[retrofox]

Hi @zertosh Honestly I don't get how to implement this control using glob in a similar way that browserify does. I've been working in bin/args.js file: https://gist.github.com/retrofox/fe232d6ec6be4c970b3b

I don't know if you meant something like that so I did these changes to try to tune to your idea. This implementation doesn't avoid my main purpose which is do not create instances of chokidar.watcher unnecessarily. Also it makes the process slower.

By another hand please let me insist on muy idea:

• Use glob adds complexity unnecessarily that anymatch (just only one line)
• anymatch supports globpattern too
• watchify is using chikidar right now where it uses anymatch
• using anymatch results are really good at least in my tests

[retrofox]

What do you think to use minimatch ? https://github.com/isaacs/minimatch node-glob use this module which is part of the npm core

[mattdesl]

Is there a repo where we can reproduce this 15 second watchify times? It could represent a bug with chokidar/watchify.

[retrofox]

@mattdesl I don't worry about these tests because ultimately it's a particular case. Maybe I'm doing something wrong. I've started to ignore other folders and it seems work ok. But the time process really gets better when it avoids create watcher instances.

[zertosh]

@retrofox I don't mean to give you a hard time, but I'd really like to rule out the possibility of a watchify/chokidar bug - I don't just want to mask it, I'd like to find the root cause and fix that.

I think there's a misunderstanding on how watchify determines what files to watch. Watchify only watches files that your bundle explicitly requires. It doesn't matter that your project lives in a directory structure with a million files - if your dependency graph only has 200 modules, then only 200 watchers will be set up. And this setup only happens at the start. Whenever a file changes only that watcher is closed (and maybe reopened).

I've reiterated that creating chokidar instances is really cheap - it's a negligible cost. If you're having long build times - and that's your motivation for this change - then lets find the root cause of your problem instead of covering it up.

I did some profiling myself with the diff below, and the cost of the chokidar instances is negligible.

diff --git a/index.js b/index.js
index 88249528..0c14706a 100644
--- a/index.js
+++ b/index.js
@@ -2,6 +2,7 @@ var through = require('through2');
 var path = require('path');
 var chokidar = require('chokidar');
 var xtend = require('xtend');
+var Minimatch = require('minimatch').Minimatch;

 module.exports = watchify;
 module.exports.args = {
@@ -15,12 +16,16 @@ function watchify (b, opts) {
     var delay = typeof opts.delay === 'number' ? opts.delay : 600;
     var changingDeps = {};
     var pending = false;
+    var ignored;

     var wopts = {persistent: true};
     if (opts.ignoreWatch) {
-        wopts.ignored = opts.ignoreWatch !== true
-            ? opts.ignoreWatch
-            : '**/node_modules/**';
+        ignored = opts.ignoreWatch !== true
+                ? opts.ignoreWatch
+                : '**/node_modules/**';
+        ignored = [].concat(ignored).map(function(i) {
+            return new Minimatch(i);
+        });
     }
     if (opts.poll || typeof opts.poll === 'number') {
         wopts.usePolling = true;
@@ -93,6 +98,7 @@ function watchify (b, opts) {
     });

     function watchFile (file) {
+        if (ignored && ignore(file)) return;
         if (!fwatchers[file]) fwatchers[file] = [];
         if (!fwatcherFiles[file]) fwatcherFiles[file] = [];
         if (fwatcherFiles[file].indexOf(file) >= 0) return;
@@ -108,6 +114,7 @@ function watchify (b, opts) {
     }

     function watchDepFile(mfile, file) {
+        if (ignored && ignore(file)) return;
         if (!fwatchers[mfile]) fwatchers[mfile] = [];
         if (!fwatcherFiles[mfile]) fwatcherFiles[mfile] = [];
         if (fwatcherFiles[mfile].indexOf(file) >= 0) return;
@@ -122,6 +129,12 @@ function watchify (b, opts) {
         fwatcherFiles[mfile].push(file);
     }

+    function ignore(file) {
+        return ignored.some(function(mm) {
+            return mm.match(file);
+        });
+    }
+
     function invalidate (id) {
         if (cache) delete cache[id];
         if (fwatchers[id]) {

[codeviking]

Turns out I duplicated this in: https://github.com/substack/watchify/pull/232

Our concern wasn't performance, but an issue where it was watching files in node_modules and throwing a segfault when someone ran an npm update (which removed files it was watching).

I can't see why it would attach watchers to files which the user explicitly requests not to be watched..

[zertosh]

@codeviking that's a very legitimate use case. thanks!

[zertosh]

superseded by https://github.com/substack/watchify/pull/237 and https://github.com/substack/watchify/commit/7cfb8fa15e38989125f8c67546fe2488ca2f32ce