gruntjs / grunt-contrib-watch

Run tasks whenever watched files change.
http://gruntjs.com/
MIT License
1.98k stars 356 forks source link

New files in empty folder not watched #166

Open markgoodyear opened 11 years ago

markgoodyear commented 11 years ago

Hi,

I'm trying to set up watching Sass files and compiling them. Everything works great appart from when creating a new empty directory (or using an existing one which empty), then adding in a new file. Using grunt watch --verbose shows that the folder gets added, but any new files inside aren't recognised. If the folder already has a file inside, new files are recognised.

Is this a known issue?

A stripped down version of my Gruntfile.js for the two tasks involved is:

module.exports = function(grunt) {

  // Initialize Grunt
  grunt.initConfig({

    // Read package.json
    pkg: grunt.file.readJSON('package.json'),

    // Project meta
    meta: {

      // Filepaths
      dir: {
        js: 'js',
        css: 'css',
        sass: 'css/sass',
        img: 'img'
      }
    },

    // Sass
    sass: {
      dist: {
        options: {
          style: 'expanded'
        },
        files: {
          '<%= meta.dir.css %>/main.css': '<%= meta.dir.sass %>/main.scss'
        }
      }
    },

    // Watch
    watch: {
      sass: {
        files: '<%= meta.dir.sass %>/**/*',
        tasks: ['sass'],
      },
    }
  });

  // Load
  grunt.loadNpmTasks('grunt-contrib-sass');
  grunt.loadNpmTasks('grunt-contrib-watch');

  // Default task
  grunt.registerTask('default', ['sass']);

};

EDIT Just thought I should mention I'm running version 0.5.1.

shama commented 11 years ago

This issue should be fixed in >=0.5.0. Could you try npm cache clean && rm -rf node_modules/grunt-contrib-watch && npm install grunt-contrib-watch to ensure it's grabbing the latest version of gaze? Thanks!

markgoodyear commented 11 years ago

Hey @shama, I just tried that and it's still doing the same unfortunately, it did show that gaze is 0.4.1.

Is there anything else I could try, or anything that could help find the issue?

Cheers!

shama commented 11 years ago

Hmm which version of node.js and which os?

markgoodyear commented 11 years ago

I'm running:

luissquall commented 11 years ago

Same here. As far as I can see at least one file following the pattern in files must exist to trigger the watch event. So if target files is something like src/**/*.scss, adding a new .scss file to:

I already updated to latest grunt-contrib-watch. Node v0.10.12 & Mac OSX 10.8.4.

markgoodyear commented 11 years ago

I can confirm what @luissquall describes is exact behavior I'm seeing.

markgoodyear commented 11 years ago

Just wondering if anyone's had chance to have a look at this? Would be awesome to automatically watch new files in empty folders.

@shama — If theres anything I can do to that could help find the issue just let me know.

mnoble01 commented 11 years ago

+1, I can confirm the same behavior as described by @luissquall

shama commented 11 years ago

Thanks everyone! I'll take a look when I get a chance. I have tests for this in gaze so I'm not sure why it isn't working here. We should add a test case for it here and work backwards to figure out why.

mnoble01 commented 11 years ago

Update: the dir/empty case also occurs when dir doesn't have any files, but may contain folders

brett-shwom commented 11 years ago

Any progress made on this one? I'm having the same issue on osx 10.8.4.

shama commented 11 years ago

I looked into it briefly, no real progress yet. Will look into more soon.

markgoodyear commented 11 years ago

@shama — If you need anything testing to help fix it I can try help out.

brett-shwom commented 11 years ago

In gaze.js, how about replacing:

    // If file was added
    current.filter(function(file) {
      return previous.indexOf(file) < 0;
    }).forEach(function(file) {
      // Is it a matching pattern?
      var relFile = path.join(relDir, file);
      // Add to watch then emit event
      self._internalAdd(relFile, function() {
        self.emit('added', path.join(dir, file));
      });
    });

with

    // If file was added
    current.filter(function(file) {
      return previous.indexOf(file) < 0;
    }).forEach(function(file) {
      // Is it a matching pattern?
      var relFile = path.join(relDir, file);
      var absFile = fs.realpathSync(relFile); //ADDED
      // Add to watch then emit event
        self._internalAdd(absFile, function() { //MODIFIED
        self.emit('added', path.join(dir, file));
      });
    });

?

minimatch.match doesn't seem to be able to handle relative paths. I can submit this as a pull request if it makes sense.

brett-shwom commented 11 years ago

I tried the above code change against the gaze project's tests. The following tests failed:

watchTest.js::addedEmitInSubFolders() matchingTest.js::addedLater()

ideas?

Environment: OSX 10.8.4 Node 0.10.15 Gaze 0.4.1 Grunt 0.4.1 npm 1.3.5

shama commented 11 years ago

How I usually go about these things is to write a new test case that matches the desired behavior. Then adjust the code until the new test passes along with the existing tests. This way we're sure we're working towards the fix we want without breaking any previous fixes made.

brett-shwom commented 11 years ago

I can jive with that. In the meantime, would you have any idea why those 2 particular test cases would be failing with the above code change?

brett-shwom commented 11 years ago

Okay, I figured out what's causing my issue. I was doing the following:

gaze('/Users/brett/Desktop/gazetestfolder/**/*', function(err, watcher) { ... })

i.e. I was using an absolute path instead of a relative path. The above invocation would result in newly created files in /gazetestfolder not registered.

If I use a relative path: gaze('../gazetestfolder/**/*', function(err, watcher) { ... }), then the new file additions to /gazetestfolder are registered.

Is this the intended behavior of gaze?

NB: my case is different than the case that @markgoodyear reported. I tried out his case (making a new directory then adding a file to it) and I do observe the same behavior that he has observed (new files added to that directory are not registered).

Maybe I should open up a separate issue for the absolute path stuff?

shama commented 11 years ago

Yes. There is an open issue for that here: shama/gaze#41 But it is low priority for me though as you can set the cwd option if you really need to pass absolute paths now.

shama commented 11 years ago

@markgoodyear @luissquall @mnoble01 Out of curiosity, are you using absolute paths as well?

markgoodyear commented 11 years ago

Hey @shama,

I've not had chance to test the above, however I do use relative paths in my Gruntfile, if that's what you're meaning?

I'll be able to check what @brett-shwom suggested later today (https://github.com/gruntjs/grunt-contrib-watch/issues/166#issuecomment-22811876).

Cheers

brett-shwom commented 11 years ago

@markgoodyear, I don't think that my suggestion will work for you. When I added that suggestion, I was under the impression that you and I were having the same issue. It turns out that I conflated our issues: I was using absolute paths and so no file creation events were getting triggered, whereas your issue was that file creation events weren't getting triggered when files were created in a folder that was created after the grunt watch task was kicked off. The code change I suggested only seems to add support file creation events that do not occur in new folders when an absolute path is specified in gaze().

mnoble01 commented 11 years ago

@shama, I'm also using relative paths in my Gruntfile...

herebefrogs commented 11 years ago

+1 I'm also running into this issue with new files not being detected (although I hadn't linked it to empty directories)

pkieltyka commented 11 years ago

+1 .. same issue with latest libraries

brett-shwom commented 11 years ago

@pkieltyka what issue are you having?

markgoodyear commented 11 years ago

@brett-shwom I'm still having the same issue as my original post. New files in empty folders do not get picked up when using grunt watch. New folders work, just not a new file in an empty folder.

pkieltyka commented 11 years ago

Yea exactly.. I can see watch sees the new folder, but it should then watch the files in that directory as well.. if they match the glob pattern..

On 2013-09-28, at 1:18 PM, Mark Goodyear notifications@github.com wrote:

@brett-shwom I'm still having the same issue as my original post. New files in empty folders do not get picked up when using grunt watch. New folders work, just not a new file in an empty folder.

— Reply to this email directly or view it on GitHub.

pkieltyka commented 11 years ago

@brett-shwom thanks for the PR.. did you test it out btw? .. I did some debugging as well.. and one thing that came to mind is that perhaps it's actually a file glob matching issue.

For example, in my Gruntfile I have:

watch: {
  coffee: {
    files: ['src/**/*.coffee'],
    tasks: ['coffee:build']
  }
}

.. pretty standard. Now, if I create a new directory under src/, I can see the message ">> File 'src/x' added." where "x" is the new empty directory. That is followed by Grunt running my "coffee:build" task.. however, it shouldn't actually run that task because that doesn't match the pattern of *.coffee ... this is a definite problem in the pattern matching that is probably throwing things off as well.

I think it should actually do nothing when a new directory is added, other then perhaps add it to the list of directories to watch because of the \ match, and once it see's a .coffee file that matches the full pattern, then run the associated tasks.


Btw, I found another bug while I was fiddling around. When deleting an entire subfolder that contains watched files, the "watch" task completely misses that the folder and its files were deleted and doesn't even send the "deleted" target to grunt.event.on("watch",..). But, if you delete each watched file in a subfolder individually, it will send the "deleted" event for each file properly, as well execute the associated tasks.

brett-shwom commented 11 years ago

@pkieltyka Hey man,

Yeah, I did test it.

In addition to modifying the mkdirThenAddFile nodeunit test (part of the https://github.com/shama/gaze repo), I also created a sample script that listens to a events on a folder named /test. I ran the sample script, then created a folder, then created a .js file within that folder, and finally observed that a file added event was triggered.

If you want to give that sample script a shot,

1) git clone git@github.com:brett-shwom/gaze.git 2) git checkout track-file-additions-in-newly-created-folders 3) npm install 4) place the below code in [your-file-name].js 5) ensure that the line require('../lib/gaze') is pointing to the gaze script in my branch 6) mkdir [wherever you want]/test 7) ensure that the line gaze('test/**/*.js', is pointing to [wherever you want]/test 8) node [your-file-name].js 9) mkdir [wherever you want]/test/a 10) touch [wherever you want]/test/a/r.js 11) observe [wherever you want]/test/a/r.js was added

var gaze = require('../lib/gaze'); 

// Watch all .js files/dirs in process.cwd()
gaze('test/**/*.js', function(err, watcher) {
  // Files have all started watching
  // watcher === this

  // Get all watched files
  console.log(this.watched());

  // On file changed
  this.on('changed', function(filepath) {
    console.log(filepath + ' was changed');
  });

  // On file added
  this.on('added', function(filepath) {
    console.log(filepath + ' was added');
  });

  // On file deleted
  this.on('deleted', function(filepath) {
    console.log(filepath + ' was deleted');
  });

  // Get watched files with relative paths
  console.log(this.relative());
});
brett-shwom commented 11 years ago

Also, you @pkieltyka wrote: ".. pretty standard. Now, if I create a new directory under src/, I can see the message ">> File 'src/x' added." where "x" is the new empty directory. That is followed by Grunt running my "coffee:build" task.. however, it shouldn't actually run that task because that doesn't match the pattern of *.coffee ... this is a definite problem in the pattern matching that is probably throwing things off as well."

Maybe file that as a separate bug?

brett-shwom commented 11 years ago

@pkieltyka you wrote: "Btw, I found another bug while I was fiddling around. When deleting an entire subfolder that contains watched files, the "watch" tasks completely misses it and doesn't even send the "deleted" target to grunt.event.on("watch",..). This means, But, if you delete each watched file in a subfolder individually, it will send the "deleted" event for each file properly, as well execute the associated tasks."

Perhaps, create a new ticket for that as well :)

floz commented 11 years ago

Same issue here, new files are not tracked. Gaze detect the new file (Gaze.prototype._watchDir is triggered with file creation), but the forEach loop (in Gaze.prototype.relative) is executed with the values inside this._watched. The problem comes from here.

The this._watched[dir] content should be refreshed by comparing the new folder content.

brett-shwom commented 11 years ago

I have a pull request in to fix this: https://github.com/shama/gaze/issues/53

We're waiting on @shama to approve/deny.

floz commented 11 years ago

Hmmm... With your fix my new files are still not watched. But it trigger the watch event when creating a new folder. Is that what it's supposed to do?

brett-shwom commented 11 years ago

How did you test the fix?

Did you check out my fork of gaze https://github.com/brett-shwom/gaze , switch to the track-file-additions-in-newly-created-folders branch, and run a test against that?

If so, could you provide me with the steps you took in your test?

floz commented 11 years ago

I've took the gaze.js from your branch.

And, in a personal task, with that configuration of GruntFile:

watch: {
    coffee: {
        files: [ "./src/coffee/**/*.coffee" ],
        tasks: [ "coffee:compile" ]
    }
}

Then: Adding a file in ./src/coffee => nothing appear in the terminal Adding a folder named "test" in ./src/coffee => trigger the coffee:compile task Adding a file in ./src/coffee/test => nothing appear in the terminal

brett-shwom commented 11 years ago

@floz I looked into your case. The issue seems to be with the ./ in ./src/coffee/**/*.coffee

If you replace ./src/coffee/**/*.coffee with src/coffee/**/*.coffee

I think you'll see that the fix works. Not ideal, but at least you have a workaround.

floz commented 11 years ago

Good catch... ! Thanks for the feedback, and interesting case :)

vict-shevchenko commented 10 years ago

Hi! I have the same issue on Windows 7. When add new file to an empty folder(inside of watching dir), watch task does not launch.

fi5u commented 10 years ago

Hi, I think I have the same issue. For me I am trying to get watch to watch for any changes to HTML files. This works if the file's directory existed at the time watch was called, but if the directory is created after watch was called, no HTML inside is watched. My problem is explained in detail in my StackOverflow question: Grunt globbing pattern not working as expected within grunt-contrib-watch Gruntfile: Gruntfile.js Node: 0.10.15 OS: OSX 10.8.5

brett-shwom commented 10 years ago

I have a pending pull request to fix this issue. It's on @shama to approve :)

shama commented 10 years ago

@brett-shwom Thanks again for the PR although Im not sure if it fixes this issue. It should handle grunt.file.write('new_dir/tmp.js', ''); the same as fs.mkdirSync('new_dir'); fs.writeFileSync('new_dir/tmp.js'); and doesn't should we should figure out why before changing the test.

Also to everyone reporting they have the same issue, could you please post your Gruntfile, node and operating system version and what you tried? Sometimes it may appear to be the same issue but after further reviewal; becomes an entirely different issue (and one that may have previously been solved). Thanks!

brett-shwom commented 10 years ago

@shama Are you saying that the fix that I provided fails for grunt.file.write('new_dir/tmp.js', ''); but does not fail for fs.mkdirSync('new_dir'); fs.writeFileSync('new_dir/tmp.js'); ?

shama commented 10 years ago

@brett-shwom Your PR is suggesting that: https://github.com/shama/gaze/pull/53/files#diff-f8ef63adf962260060d4f416fec629bcR275

brett-shwom commented 10 years ago

@shama That's easily tested :) If you revert the test in watch.js to use grunt.file.write('new_dir/tmp.js', ''); while keeping the gaze.js you'll see that the tests still pass.

brett-shwom commented 10 years ago

@shama given my latest comment, what else might I be able to do to convince you to accept the PR?

shama commented 10 years ago

@brett-shwom Add a test case that shows the error being fixed as opposed to editing the existing one. As we should support both of those use cases if they behave differently.

This line in your PR:

fs.mkdirSync('new_dir'); //fs.mkdirSync([folder]) seems to behave differently than grunt.file.write('[folder]/[file]')

tells me that I need to look into why you said they behave differently before merging. Which is why I left the PR open.

brett-shwom commented 10 years ago

@shama the test case I wrote shows the error being fixed. I don't really understand what you mean.

brett-shwom commented 10 years ago

@shama the original test case with grunt.file.write('new_dir/tmp.js', ''); was flawed.