gruntjs / grunt-contrib-watch

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

Less hacky way to pass only changed files to tasks needed #149

Open mgol opened 11 years ago

mgol commented 11 years ago

For large projects, default grunt-contrib-watch behavior of running tasks for every single file in the matched set instead of only the changed ones is not feasible. The problem is, the workaround described in README is hacky and doesn't play nice with Grunt conventions. This approach doesn't play nice with tasks with multiple targets, especially if the task is called with different targets based on which files were changed.

Another problem is that the nospawn: true setting causes buggy behavior and makes the watch task fail on test failures so that's not a good solution. And with nospawn: false there's no way to pass the file path to the fired task.

My use case is that I use grunt-contrib-jshint with 3 targets - app, tests, and gruntfile, each with their own set of options. How would I know which one of them I need to patch, to which field should I write the path to the changed file (as there are various ways to define associated files for Grunt tasks), etc. etc.

shama commented 11 years ago

That was the least hacky way I could think of. The alternatives would be sockets, writing to files and other nonsense to pass data between processes. I also don't think the watch task should even handle that part as edited files don't always indicate that files that should be compiled. So for most use cases, the watch task giving you changed files isn't even what you actually want.

A better method might be to utilize the filter option of grunt to build a smarter way to compile as needed. Check out this comment: https://github.com/gruntjs/grunt-contrib-copy/issues/78#issuecomment-19021639 That is a start for using it to only compile new files. Still would need to handle dependencies and include some kind of filtering mechanism to avoid having to stat every file on every run.

It's simple to write a solution for a specific use case when it comes to compiling files as needed. But very difficult to write a solution for a OSS release that needs to handle the wide range of use cases. This watch task attempts (albeit poorly) to at least provide an interface for users to implement their own specific use cases.

Also, FWIW, those issues with nospawn should get fixed with the new version of gaze.

ilanbiala commented 11 years ago

Also, for the code provided in the https://github.com/gruntjs/grunt-contrib-watch#compiling-files-as-needed section:

grunt.event.on('watch', function(action, filepath) {
    grunt.config('default', filepath);
});

This code still runs the tasks on all the corresponding files for me instead of just the saved one. If I have two .css files and I saved just one, it will run the task for my .css files for both files, not just the one I saved. Does this happen for you as well?

shama commented 11 years ago

@nbatothemax Sounds like you're not setting the correct portion of your config. grunt.config('default', filepath); would set the following value:

grunt.initConfig({
  'default': filepath
});
ilanbiala commented 11 years ago

How should it be set up then?

shama commented 11 years ago

@nbatothemax It depends on your config. If your config looks like this:

grunt.initConfig({
  task: {
    target: {
      src: ['css/*.css'],
      dest: 'build/css/style.css'
    }
  }
});

Then you could alter the task.target.src of your config with: grunt.config('task.target.src', filepath);.

ichernev commented 11 years ago

+1

I think this would need grunt support -- the changed file has to be passed to grunt, and then matched against all the provided filters, globs and so on, without having to stat any other files. Only after that the task can be invoked with a subset of files.

shama commented 11 years ago

There really isn't anything to +1 on this issue. Pretty much compiling files as needed will remain as is in this watch task (except for bug fixes of course). Unless someone else has a better solution?

My suggestion is we should handle compiling as needed with another module using Grunt's filter option.

ilanbiala commented 11 years ago

Depending on what file is saved in my source directory, a certain task out of many is run..so I am not really sure how you can just set it to run one task based on the source file.

shama commented 11 years ago

@nbatothemax Not sure I follow, isn't that just normal watch behavior?

watch: {
  one: {
    files: ['src/one.js'],
    tasks: ['onetask']
  },
  two: {
    files: ['src/two.js'],
    tasks: ['twotask']
  },
}
mgol commented 11 years ago

@shama The filter function way you proposed seems more robust, I'll check that, thanks!

ilanbiala commented 11 years ago

My watch code looks more like this:

watch: {
    html: ['/src/**/*.html'],
    js: ['/src/**/*.js'],
    css: ['/src/**/*.css']
}

And I have tasks registered for each specific type of file I am watching. Only one of them should be run and the output is usually dynamic based on multiple factors.

mgol commented 11 years ago

@shama Actually, the filter approach is hard to use with tasks like jshint that don't have targets - there's no file to compare against... And since watch seems to reload Gruntfile.js after each task spawning, there's no way to save previous time of execution (unless in external files which is ugly).

shama commented 11 years ago

@nbatothemax That syntax isn't supported in this watch task. How does that even run?

@mzgol Ah you're right, that would still have the same problem. Did you see this proposal #156? I think that might be a more elegant way to solve this. Let me know your thoughts.

mgol commented 11 years ago

@shama I replied to #156 with my thoughts. I finally went with an idea to not define any filter in the initial configuration and to create clones of all targets that are supposed to have filtered files on watch (e.g. for a target jshint:app I created a target jshint:appFiltered) and I run these modified tasks on watch. The filter function is simple, it just checks if a file was modified later than 5 seconds ago.

It works fine, although with one problem - I can't now just run the jshint task without providing a target because it would run all those modified tasks. This problem could be solved by modifying the task name, not the target name, though I don't know if it's possible to use grunt-contrib-jshint with both jshint and jshintFiltered tasks since the plugin registers only the jshint task.

The simplified config is sth like that:

'use strict';

module.exports = function (grunt) {
    var filteredTasks = [
            ['jshint', 'app'],
            ['defs', 'app'],
        ];

    function getFilterFunction() {
        return function (filepath) {
            var srcTime = fs.statSync(filepath).mtime.getTime();
            return srcTime > Date.now() - 5000; // don't watch files changed before last 5 seconds
        };
    }

    grunt.initConfig({
        watch: {
            'jshint.app': {
                files: ['app/**/*.js'],
                tasks: ['jshint:appFiltered'],
            },
            'defs.app': {
                files: ['app/**/*.js'],
                tasks: ['defs:appFiltered'],
            },
        },
    });

    // Add copies of watched tasks with an added filter option.
    filteredTasks.forEach(function (taskAndTarget) {
        var newTaskAndTarget = taskAndTarget.slice(0);
        newTaskAndTarget[newTaskAndTarget.length - 1] = newTaskAndTarget[newTaskAndTarget.length - 1] + 'Filtered';

        grunt.config(newTaskAndTarget, grunt.config(taskAndTarget));
        grunt.config(newTaskAndTarget.concat(['filter']), getFilterFunction());
    });

    // ...