browserify / watchify

watch mode for browserify builds
Other
1.79k stars 181 forks source link

prevent inifinte rebundle (do not watch dirs) #89

Closed ztmr closed 10 years ago

ztmr commented 10 years ago

do not watch directories since it (for some reason) makes wachify to rebundle again and again with no reason, even if no file has changed.

after some debugging, it seems the rebundle trigger is the act of invalidating the root project's directory itself. we wouldn't care so much on repetitive rebundling if the rebundle worked well, but the result of these rebundles never affected the original output, so the watchify didn't produce an update bundle anymore until its process has been restarted.

once this patch has been applied, everything started to work nicely.

ghost commented 10 years ago

In chokidar each of the events gets a stat instance as a second argument, so that can avoid the sync call.

ghost commented 10 years ago

@ztmr does this patch also fix the problem?

From 33f1ad15d48e9d56762ee8fb3ef276aebdaf1268 Mon Sep 17 00:00:00 2001
From: James Halliday <mail@substack.net>
Date: Mon, 22 Sep 2014 22:05:14 +0300
Subject: [PATCH] check stat

---
 index.js |   15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/index.js b/index.js
index 4e9a661..861f34c 100644
--- a/index.js
+++ b/index.js
@@ -68,16 +68,15 @@ function watchify (b, opts) {
         });
     });

-    function watchFile (file) {
-        if (fs.lstatSync (file).isDirectory ()) return;
+    function watchFile (file, stat) {
         if (!fwatchers[file]) fwatchers[file] = [];
         if (!fwatcherFiles[file]) fwatcherFiles[file] = [];
         if (fwatcherFiles[file].indexOf(file) >= 0) return;

         var w = chokidar.watch(file, {persistent: true});
         w.on('error', b.emit.bind(b, 'error'));
-        w.on('change', function () {
-            invalidate(file);
+        w.on('change', function (p, stat) {
+            invalidate(file, stat);
         });
         fwatchers[file].push(w);
         fwatcherFiles[file].push(file);
@@ -90,14 +89,16 @@ function watchify (b, opts) {

         var w = chokidar.watch(file, {persistent: true});
         w.on('error', b.emit.bind(b, 'error'));
-        w.on('change', function () {
-            invalidate(mfile);
+        w.on('change', function (p, stat) {
+            invalidate(mfile, stat);
         });
         fwatchers[mfile].push(w);
         fwatcherFiles[mfile].push(file);
     }

-    function invalidate (id) {
+    function invalidate (id, stat) {
+        if (stat && stat.isDirectory()) return;
+        
         delete cache[id];
         if (fwatchers[id]) {
             fwatchers[id].forEach(function (w) {
-- 
1.7.9.5
ztmr commented 10 years ago

@substack the patch looks fine but it does not work :( it seems there is a bug somewhere else because the stat passed as a second argument is always different (for the same file!) while the lstatSync called by me is still the same. If we did something like this:

    function invalidate (id, stat) {
        console.log('--');
        console.log(id);
        console.log(stat);
        console.log(fs.lstatSync (id));
        if (stat && stat.isDirectory()) return;
        [...]

...we get the following results, repetitively, still again and again, while the path and second stat structure are constant and the first stat structure (the one delivered by chokidar) is always different (not just the modification time, but also the inode and other stuff).

/still/the/same/path/the/root/of/my/project
{ dev: 2065,
  mode: 33188,
  nlink: 1,
  uid: 1000,
  gid: 100,
  rdev: 0,
  blksize: 4096,
  ino: 5878683,
  size: 1509018,
  blocks: 2952,
  atime: Tue Sep 23 2014 10:01:39 GMT+0200 (CEST),
  mtime: Tue Sep 23 2014 10:01:44 GMT+0200 (CEST),
  ctime: Tue Sep 23 2014 10:01:44 GMT+0200 (CEST) }
{ dev: 2065,
  mode: 16877,
  nlink: 7,
  uid: 1000,
  gid: 100,
  rdev: 0,
  blksize: 4096,
  ino: 5873891,
  size: 4096,
  blocks: 8,
  atime: Sun Aug 03 2014 14:42:20 GMT+0200 (CEST),
  mtime: Mon Sep 22 2014 17:18:42 GMT+0200 (CEST),
  ctime: Mon Sep 22 2014 17:18:42 GMT+0200 (CEST) }

I don't know what's the entity that is being changed again and again and why (there is no other node process or anything else touching the source tree at the moment, I also turned off the editor to be sure it does not do any autosave or so). But I have also noticed that even if I used the lstatSync, the bundle infinetely grew up in its size. I wrote a bugreport yesterday but when I was almost done, I accidentally closed the browser tab, so I didn't write it again. But I would expect (and watchify readme seems to confirm that) that once I add a line into the code, save, the bundle will grow, and once I remove the line back to the original state, the bundle will be back at the original size. The behavior I noticed was other: the bundle grew everytime on any event, no matter if we add a new code, delete anything or remove some dependencies. I am starting to think these issues might be entangled somehow, yet I don't have clue how...

Also tried to use async stat as the chokidar does, but it gave is the right results -- the same as lstatSync and stat OS command. So perhaps it is a problem in chokidar? But it calls the stat on the same filename that is being passed here to us... hmm.

At the moment, I'd stay with the original lstatSync solution until we figure out the source of the problem. Is that acceptable?

PS: I also thought it might be because of filesystem because we use ZoL (ZFS on Linux) but to my surprise, the problem appears even after moving to another (XFS) volume... so it does not seem to be a filesystem issue.

ghost commented 10 years ago

Is there anything special about your setup, such as an abundance of symlinks? I'll merge an async version of your patch right now probably but I'm just curious about what might be causing this. I've never personally run into this behavior.

ghost commented 10 years ago

I just merged in asynchronous version of your fix in 1.0.3 and also fixed some broken tests (that were already broken, nothing you did). If you have time, please verify that this fixes your failing case!

ztmr commented 10 years ago

@substack I've just tried luck with 1.0.5. Well, it didn't work out of the box until I changed it a bit (a diff below). Since that moment, everything seems to work fine!

--- index.js.orig       2014-09-25 13:57:07.357889568 +0200
+++ index.js    2014-09-25 14:00:00.901885220 +0200
@@ -68,14 +68,14 @@
         });
     });

-    function watchFile_ (file) {
+    function watchFile (file) {
         fs.lstat(file, function (err, stat) {
             if (err || stat.isDirectory()) return;
-            watchFile(file);
+            watchFile_(file);
         });
     }

-    function watchFile (file) {
+    function watchFile_ (file) {
         if (!fwatchers[file]) fwatchers[file] = [];
         if (!fwatcherFiles[file]) fwatcherFiles[file] = [];
         if (fwatcherFiles[file].indexOf(file) >= 0) return;

When it comes to the setup, I don't know about anything special, no symlinks, no circle references, ... but for now, it seems to be OK. The only thing is that the bundle grows everytime it is rebuilt:

10317493 bytes written to dist/js/idea-erp-ui-v2.dev.js (17.13 seconds)
10317532 bytes written to dist/js/idea-erp-ui-v2.dev.js (3.30 seconds)
10317563 bytes written to dist/js/idea-erp-ui-v2.dev.js (3.28 seconds)

The first one is initial build, the second is after I have added an empty line to one of the sources, and the third one is after removing that empty line. One would say that the empty lines and comments shouldn't change the size, but that growing tendency even after removing the code is even more weird.

So I am wondering if it may have something to do with our problem, but if there was another file that caused the rebundle, it should be logged from the watchFile (where I had a debug message).

ghost commented 10 years ago

Good catch, updated in 1.0.6. I'm not sure at all what could be causing the constant growth in bundle size. I've never run into that bug myself.