gaze icon indicating copy to clipboard operation
gaze copied to clipboard

Gaze.prototype._addToWatched repeating readDir of same directory on each call

Open smithwib opened this issue 8 years ago • 6 comments

Thanks for the module - super cool.

We're running into a delay when a project site got past a couple dozen files and headed into the hundreds, we noticed the processing time was taking longer and longer -- minutes. In debugging, I focused in on this:

    var dirname = (helper.isDir(file)) ? filepath : path.dirname(filepath);
    dirname = helper.markDir(dirname);
..
    var readdir = fs.readdirSync(dirname);
    for (var j = 0; j < readdir.length; j++) {
      var dirfile = path.join(dirname, readdir[j]);
      if (fs.lstatSync(dirfile).isDirectory()) {
        helper.objectPush(this._watched, dirname, dirfile + path.sep);
      }
    }  

When supplied a glob like ./data/data.json, it set dirname = ./data (the parent folder), and then it readdirSync that directory and added its subdirectories to the watchlist.

Two strange things:

  • I'm not sure why it would push the subdirectories, since I was just looking for one file
  • When you process 100 files in the same folder, it is doing the same thing over and over -- readdirSync the parent folder. That seems unnecessary.

Can you help?

smithwib avatar Mar 18 '16 19:03 smithwib

This is what it seems it should be, but I'm not familiar with all use cases:

// Adds files and dirs to watched
Gaze.prototype._addToWatched = function(files) {
  for (var i = 0; i < files.length; i++) {
    var file = files[i];  
    var filepath = path.resolve(this.options.cwd, file);

    var dirname = (helper.isDir(file)) ? filepath : path.dirname(filepath);
    dirname = helper.markDir(dirname);

    // If a new dir is added
    if (helper.isDir(file) && !(filepath in this._watched)) {    
      helper.objectPush(this._watched, filepath, []);

      // add folders into the mix
      var readdir = fs.readdirSync(dirname);
      for (var j = 0; j < readdir.length; j++) {
        var dirfile = path.join(dirname, readdir[j]);
        if (fs.lstatSync(dirfile).isDirectory()) {
          helper.objectPush(this._watched, dirname, dirfile + path.sep);
        }
      }       
    }

    if (file.slice(-1) === '/') { filepath += path.sep; }
    helper.objectPush(this._watched, path.dirname(filepath) + path.sep, filepath);

  }
  return this;
};

smithwib avatar Mar 18 '16 20:03 smithwib

Hey! Thanks for diving into this. As long as all the tests pass with npm test all the use cases we support should be good. Feel free to open a PR! Thanks!

shama avatar Mar 19 '16 17:03 shama

My pleasure, but unfortunately I ran npm test and did see a slew of errors like below. I fear it may be because I am on a PC with reduced admin rights, a dated version of node (0.12.7), and am behind a firewall, (aka developer heaven). Example:

Error: 'added.js' == 'Project (LO)' at Object.equal (C:\wamp\www\gaze\node_modules\grunt-contrib-nodeunit\node_mo dules\nodeunit\lib\types.js:83:39) at Gaze. (C:\wamp\www\gaze\test\patterns_test.js:30:14) at Gaze.emit (events.js:107:17) at Gaze.emit (C:\wamp\www\gaze\lib\gaze.js:128:32) at Gaze. (C:\wamp\www\gaze\lib\gaze.js:401:18) at null._onTimeout (C:\wamp\www\gaze\lib\gaze.js:429:24) at Timer.listOnTimeout (timers.js:119:15)

Should I proceed with the PR and lean on you to do a real npm test?

smithwib avatar Mar 21 '16 14:03 smithwib

Hi,

I ran into the same issue as @smithwib. Making a fix, I doubt if the test is correct. This line bothers me: var expected = { '.': ['Project (LO)/', 'nested/', 'one.js', 'sub/'], };

The problem I see here is that we didn't add sub dir into the watch list. The test adds folders Project (LO) and nested only.

This may have unpleasant side-effects. For example, gaze is used in metalsmith-watch library. Metalsmith is a static website generator written in Node.JS. Common folder structure is

  • root
    • src // pages
    • build // ready website
    • node_modules

You configure to watch for changes in src folder and gaze also starts monitoring node_modules folder as well. Is it an expected behavior?

@shama Kyle, what do you think?

chebum avatar Jun 28 '16 19:06 chebum

Has this been solved?

ghost avatar Nov 14 '16 06:11 ghost

Is there a reason that sub folders are scanned and added to the watch list when a single file is specified to be watched and not a file glob that could potentially include sub-folders? This is causing issues in hyper where only a single file should be watched.

I added the following issue in the hyper repo:

There are a number of issues being raised in hyper that are directly related to the fact that gaze is scanning sub-folders in a users HOME when a single file (.hyper.js) is added to be watched.

paulbouwer avatar Apr 29 '17 04:04 paulbouwer