Doesn't register new files added to a new directory #14

lancejpollard · 2013-01-20T22:05:23Z

Say you are watching this pattern:

gaze('app/templates/**/*.handlebars', function(error, watcher) {
  watcher.on('added', function(filePath) {
    console.log('added', filePath);
  });

  watcher.on('changed', function(filePath) {
    console.log('changed', filePath);
  });

  watcher.on('deleted', function(filePath) {
    console.log('deleted', filePath);
  });
});

Assume this is the initial directory structure:

app
└── templates
    ├── application.handlebars
    ├── users
        ├── index.handlebars
        ├── new.handlebars

The following operations work:

added app/templates/a.handlebars
changed app/templates/a.handlebars
deleted app/templates/a.handlebars

added app/templates/users/a.handlebars
changed app/templates/users/a.handlebars
deleted app/templates/users/a.handlebars

But now say I add the folder app/templates/snippets, and then start adding files to that folder, so we end up with these files:

app
└── templates
    ├── application.handlebars
    ├── users
    │   ├── index.handlebars
    │   ├── new.handlebars
    └── snippets
        ├── a.handlebars
        └── b.handlebars

Gaze won't send notifications that snippets/a.handlebars and snippets/b.handlebars were added/changed/deleted.

You can fix this by modifying this code:

https://github.com/shama/gaze/blob/master/lib/gaze.js#L385-L408

I changed it to be this:

// If file was deleted
_.filter(previous, function(file) {
  return _.indexOf(current, file) < 0;
}).forEach(function(file) {
  var filepath = path.join(dir, file);
  _this.remove(filepath);
  if (!_this._isDir(file)) {
    _this.emit('deleted', filepath);
  }
});

// If file was added
_.filter(current, function(file) {
  return _.indexOf(previous, file) < 0;
}).forEach(function(file) {
  // Is it a matching pattern?
  var relFile = path.join(relDir, file)
    , isDirectory = fs.statSync(relFile).isDirectory();
  if (isDirectory || _this._isMatch(relFile)) {
    // Add to watch then emit event
    _this.add(relFile, function() {
      if (!isDirectory)
        _this.emit('added', path.join(dir, file));
    });
  }
});

But, that isn't perfect yet b/c if you were to delete the folder app/templates/users, it would throw an error. Something along these lines though would be awesome.

Gaze it seems could support such "smart" file watching, where even though app/templates/snippets doesn't minimatch app/templates/**/*.handlebars, it's contents might. This is the default behavior of FSEvents.

Also, it seems like it should work where if you renamed or moved a directory, it would emit deleted for the old paths and added for the new paths for all the files in the nested directories.

Thoughts?

The text was updated successfully, but these errors were encountered:

shama · 2013-01-21T18:25:50Z

Yes this is something I totally want to support. Just haven't gotten to it yet. You've outlined the exact desired behavior. I'll start by adding some test cases for this on a new branch.

lancejpollard · 2013-01-21T21:29:49Z

Awesome, sounds good.

thehydroimpulse · 2013-03-14T04:16:49Z

Any news on this? Or do you want a PR?

shama · 2013-03-14T05:09:19Z

@thehydroimpulse Still on my radar. My wife and I just had a baby so I'm a bit behind on everything :). I have some unfinished work on this. I'm trying to move the file system diff stuff into a separate lib. I appreciate the offer for a PR though. Thanks!

thehydroimpulse · 2013-03-14T05:27:21Z

No worries. (Congratulations btw :)).

rgaskill · 2013-04-18T03:25:55Z

@shama I have been thinking about this one and I want to throw out an idea to see if you are interested. What if gaze was changed to stop using fs.watch functionality and just do straight polling. I think this would greatly reduce the complexity required to maintain the file list. I created a quick example here to demonstrate the simplicity of a polling approach. Any interest? If so I can put together a PR.

thehydroimpulse · 2013-04-18T03:28:12Z

@rgaskill Polling is extremely inefficient. Watching many files using the polling technique will max out your CPU. Polling would work on a smaller file set but since gaze is targeting multiple use cases, I doubt that'd work. Also, doesn't gaze offer a polling only option?

rgaskill · 2013-04-18T03:44:28Z

@thehydroimpulse The current implementation is already polling using the watchFile functionality. Your "max out your CPU" assertion is incorrect. Take a look at my prototype and let me know what you think.

thehydroimpulse · 2013-04-18T04:04:01Z

@rgaskill The CPU maxed out (on macs anyway) using fs.watch not fs.watchFile but that was before 0.9.2 when they implemented a fix for it. Nonetheless polling is still inefficient. Even node recommends using fs.watch instead, but that's not always possible.

shama · 2013-04-18T16:29:56Z

Sorry for taking so long on this issue. I swear I'll get to it soon. @cowboy just released globule which I was hoping for to make this cleaner to implement.

FWIW, the watch in Grunt v0.3 used a combination of polling and fs.watch. I'm for whatever approach is fastest with all current tests passing. Which usually means a compromise between the two.

I benchmarked the difference awhile back so it may have changed. Using fs.watch was only slightly faster with a small amount of files but with a large amount, > 1000 fs.watch was lightyears faster. So I don't think we could switch completely over to a polling only method.

It would be nice to have as an option though. Take a look at #32. fs.watch will never work on some environments so it would be handy to have an option to switch to haze for polling instead.

rgaskill · 2013-04-20T12:20:25Z

The current implementation of gaze uses fs.watch to watch directories to pickup on file additions and deletions and fs.watchFile to watch files for changes. I did this for reasons described in (#6 and #9). This issue (#14) is caused when a directory is added outside the the initial fileset found by the globing pattern (i.e. the directory created isn't under one of the directories being watched by fs.watch) but is within the globing pattern. The simplest process I can think of to find these additional directories outside the initial fileset found but still within the globing pattern is to re-scan for files using some globing tool (fileset, globule, etc..). This process is the most time consuming process of the entire watch process. For example if my globing pattern was /*/.js it would have to scan my entire hardrive to find all the js files which takes several minutes.

The approach outlined in haze will perform equivalently (maybe a little better because it doesn't track directories) to the current approach except in addition it is doing the re-scan for files using the globing pattern. So, it is slower because of that extra globing pattern scan. One thought I have to avoid additional globing scans is to find the root directory of the globing pattern and fs.watch the root directory and all of it's subdirectories.

Another plus to the haze approach is it's simplicity. There is no longer the process of having to tracking what you are watching and unwatching as files are deleted and added. Basically, it is just a set of filenames that are monitored for changes periodically and compared to a new set every cycle to find additions and deletions.

There seems to be two points of discussion:

How large of a set of files should be considered supported? i.e. what is the benchmark (how many files/how fast 1000 files was mentioned by @shama but how fast should it be able to detect a change watching that many files)
Is there a faster way to find files outside the initially found file set but within the globing pattern without doing another scan using the globing pattern? I will take a stab finding the root directory of a globing pattern, it will just require a little parsing of the globing pattern. I am interested in simpler ideas though. Regardless of this approach an initial scan will always be required.

shama · 2013-04-20T16:58:00Z

Just added this issue to fix the benchmarks: #33. Ideally it would be awesome to see the impact of each commit we make. Even more awesome if it could do it retroactively. That should aid in this research.

I honestly don't care how files are watched just as long as tests pass in OSX, Linux and Windows and benchmarks prove improvement (which I hope to do better going forward).

OliverJAsh · 2013-06-10T12:35:09Z

Can't wait to see this fixed. @shama Does your commit also add support for detecting changes to file names?

shama · 2013-06-11T01:38:31Z

@OliverJAsh I want to say yes but ill double check (and add a test).

OliverJAsh · 2013-06-28T13:07:22Z

Is this in release?

demisx · 2014-09-01T16:53:53Z

We also have a problem with being able to watch directories and would love to see this implemented. For example, no events are being fired when directory is being deleted floatdrop/gulp-watch#70.

shama mentioned this issue Mar 5, 2013

An error occurs if delete a directory gruntjs/grunt-contrib-watch#60

Closed

shama mentioned this issue Mar 17, 2013

Watch doesn't fire when a new file is created shama/grunt-hub#19

Closed

shama mentioned this issue Apr 12, 2013

New subfolders aren't watched until watch task is restarted gruntjs/grunt-contrib-watch#70

Closed

shama mentioned this issue May 4, 2013

Fallback to polling when EMFILE is hit gruntjs/grunt-contrib-watch#83

Open

shama added a commit that referenced this issue Jun 8, 2013

Register new files when added to new directory. Fixes GH-14.

a7f3f58

callumlocke mentioned this issue Jun 17, 2013

added event never fires #41

Closed

shama closed this as completed in f6be587 Jul 1, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Doesn't register new files added to a new directory #14

Doesn't register new files added to a new directory #14

lancejpollard commented Jan 20, 2013

shama commented Jan 21, 2013

lancejpollard commented Jan 21, 2013

thehydroimpulse commented Mar 14, 2013

shama commented Mar 14, 2013

thehydroimpulse commented Mar 14, 2013

rgaskill commented Apr 18, 2013

thehydroimpulse commented Apr 18, 2013

rgaskill commented Apr 18, 2013

thehydroimpulse commented Apr 18, 2013

shama commented Apr 18, 2013

rgaskill commented Apr 20, 2013

shama commented Apr 20, 2013

OliverJAsh commented Jun 10, 2013

shama commented Jun 11, 2013

OliverJAsh commented Jun 28, 2013

demisx commented Sep 1, 2014

Doesn't register new files added to a new directory #14

Doesn't register new files added to a new directory #14

Comments

lancejpollard commented Jan 20, 2013

shama commented Jan 21, 2013

lancejpollard commented Jan 21, 2013

thehydroimpulse commented Mar 14, 2013

shama commented Mar 14, 2013

thehydroimpulse commented Mar 14, 2013

rgaskill commented Apr 18, 2013

thehydroimpulse commented Apr 18, 2013

rgaskill commented Apr 18, 2013

thehydroimpulse commented Apr 18, 2013

shama commented Apr 18, 2013

rgaskill commented Apr 20, 2013

shama commented Apr 20, 2013

OliverJAsh commented Jun 10, 2013

shama commented Jun 11, 2013

OliverJAsh commented Jun 28, 2013

demisx commented Sep 1, 2014