tschaub / grunt-newer

Configure Grunt tasks to run with newer files only.
https://npmjs.org/package/grunt-newer
MIT License
945 stars 53 forks source link

Running 'grunt newer:jshint' causes the full jshint task to run #39

Open tigerclaw-az opened 10 years ago

tigerclaw-az commented 10 years ago

I setup my jshint configuration as so:

jshint: {
   src: ['<%= jsPath %>/*.js', '<%= jsPath %>/app/**/*.js', '<%= jsPath %>/nls/**/*.js'],
    options: {
        'browser': true,
                'undef': true
    }
}

Then I tried to run grunt newer:jshint and got the following output:

Running "newer:jshint" (newer) task

Running "newer:jshint:src" (newer) task

Running "jshint:src" (jshint) task
>> 111 files lint free.

Running "newer-postrun:jshint:src:1:node_modules/grunt-newer/.cache" (newer-postrun) task

Done, without errors.

I have ran it a number of times and the same output occurs, I thought it would only lint files that changed after the first run?

tschaub commented 10 years ago

Your example shows a csslint task config, but your output looks like it's from the jshint task. Did you mean to write jshint in the task config?

Any chance you've got a link to your full gruntfile.js?

Also, I'm mildly curious why people use the template syntax (e.g. '<%= jsPath %>/*.js') instead of straight JavaScript (e.g. jsPath + '/*.js'), but this shouldn't make a difference.

tigerclaw-az commented 10 years ago

Yeah, sorry that should have been jshint

I can post the end of my gruntfile.js

grunt.loadNpmTasks('grunt-contrib-csslint');
grunt.loadNpmTasks('grunt-contrib-jshint');
grunt.loadNpmTasks('grunt-contrib-watch');
grunt.loadNpmTasks('grunt-newer');

grunt.registerTask('default', ['newer:csslint', 'newer:jshint']);

I use the template syntax because those variables are defined within the grunt.initConfig options, they aren't set as a global variable to grunt. Plus, I can use the template syntax within quotes, whereas with the variable I would have to use concatenation.

lukaszprus commented 10 years ago

I noticed similar issue when playing with https://github.com/yeoman/generator-angular. See https://github.com/yeoman/generator-angular/issues/620.

tschaub commented 10 years ago

Thanks for the pointer to that ticket @lucasprus - I didn't notice what the issue here was until reading that. The jshint task is a multi-task, and the config above creates one target named src (it could be named anything, src here is the target name, not src as in source files). I'll debug to see why the files config is not being correctly modified by grunt-newer.

mvanroon commented 10 years ago

I have a similar problem. I am using grunt-newer with grunt-imagemin. When I add an image or modify an image, grunt-newer runs imagemin on all of my images:

Running "watch" task
Waiting...
>> File "client/img/test.png" changed.
Running "newer:imagemin:serve" (newer) task

Running "imagemin:serve" (imagemin) task
✔ client/img/deelverhaal.png (already optimized)
✔ client/img/dummy/keyvisual.jpg (already optimized)
✔ client/img/facebook.jpg (already optimized)
✔ client/img/deelverhaal@2x.png (already optimized)
✔ client/img/flowers/flower2.png (already optimized)
✔ client/img/flowers/flower1.png (already optimized)
(etcetera)
Minified 50 images (saved 0 B)

Running "newer-postrun:imagemin:serve:1:/Users/michiel/Sites/new-site/src/node_modules/grunt-newer/.cache" (newer-postrun) task

Done, without errors.

Here's my a stripped version of my Gruntfile:

'use strict';

module.exports = function(grunt) {
    // Load all Grunt tasks
    require('load-grunt-tasks')(grunt);

    grunt.initConfig({
        // Configurable paths
        pkg: grunt.file.readJSON('package.json'),
        watch: {
            images: {
                files: ['src/img/**/*.{jpg,jpeg,png,gif}'],
                tasks: ['newer:imagemin:serve']
            }
        },
        imagemin: {
            serve: {
                files: [{
                    expand: true,
                    cwd: 'src/img/',
                    src: ['**/*.{jpg,jpeg,png,gif}'],
                    dest: 'dist/img/'
                }]
            }
        }
    });
};
rjmackay commented 10 years ago

I found grunt-newer worked if I did

        // JSHint checking of the JS app files
        jshint :
        {
            all : {
                files : ['Gruntfile.js', 'media/js/app/**/*.js'],
            },
            options : {
                jshintrc : '.jshintrc'
            }
        },

but not for

        // JSHint checking of the JS app files
        jshint :
        {
            all : ['Gruntfile.js', 'media/js/app/**/*.js'],
            options : {
                jshintrc : '.jshintrc'
            }
        },

It seems to only understand one syntax for files but grunt supports multiple ways of specifying.

tschaub commented 10 years ago

@rjmackay would you mind opening a new issue with just this bit of detail? It is clear that there is a problem when people provide a files config as the value of the target name for a multi-task. I can provide a fix for this (with a bit of grumbling about Grunt's myriad of file config options).

svlada commented 9 years ago

I have the same problem, here is my gruntfile.js

module.exports = function(grunt) {

grunt.initConfig({
pkg: grunt.file.readJSON('package.json'),

handlebars: {
    options: {
        // namespace: 'Handlebars',         
        amd: true
    },
    all: {
        files: {
            "js/templates/templates.js": ["templates/**/*.hbs"]
        }
    }
},

clean: {
    options: { force: true },
    resources: ['../resources/js', '../resources/css', '../resources/fonts', '../resources/img']
},

copy: {
    main: {
        files: [
            {expand: true, src: ['js/**'], dest: '../resources', filter: 'isFile'},
            {expand: true, src: ['css/**'], dest: '../resources', filter: 'isFile'},
            {expand: true, src: ['fonts/**'], dest: '../resources', filter: 'isFile'},
            {expand: true, src: ['img/**'], dest: '../resources', filter: 'isFile'}
        ]
    }
},

watch: {
    handlebars: {
        files: ["templates/**/*.hbs"],
        tasks: ["handlebars"]
    },
    copy: {
        files: ["js/**", "css/**"],
        tasks: ["newer:copy:main"]          
    }
}

});

grunt.loadNpmTasks('grunt-contrib-copy');
grunt.loadNpmTasks('grunt-newer');
grunt.loadNpmTasks('grunt-contrib-handlebars');
grunt.loadNpmTasks('grunt-contrib-watch');
grunt.loadNpmTasks('grunt-contrib-clean');

// Default task(s).
grunt.registerTask('default', ['clean:resources', 'handlebars', 'copy', 'watch']);
};

And here is the console output:

Running "newer:copy:main" (newer) task Running "copy:main" (copy) task Copied 112 files Running "newer-postrun:copy:main:1:webapp\src\main\webapp\client\node_modules\grunt-newer.cache" (newer-postrun) task

Done, without errors. Completed in 0.890s at Thu Mar 19 2015 16:20:11 GMT+0100 (Central Europe Standard Time) - Waiting...

ts-web commented 9 years ago

I created a simple POC for this bug. The expanded syntax is what broke it.

Gruntfile.js
module.exports = function(grunt) {
  grunt.initConfig({
    copy: {
        // works with newer -- only copies when changed
        stuff: {
            src: 'app/file.txt',
            dest: 'dist/'
        },
        // fails with newer -- copies every time
        stuff2: {
            expand: true,
            cwd: 'app',
            src: 'file.txt',
            dest: 'dist'
        }
    }
  });

  grunt.loadNpmTasks('grunt-contrib-copy');
  grunt.loadNpmTasks('grunt-newer');

  // both of these should do the same thing: only copy the file if it has changed
  // this one works (only copies when newer)
  grunt.registerTask('default', ['newer:copy:stuff']);
  // this one doesn't work (always copies the file)
  grunt.registerTask('bug', ['newer:copy:stuff2']);
};

Note the difference of the targets: one uses the simple src and dest properties, and the other uses expand, cwd, src, and dest. One works, and the other doesn't.

ts-web commented 9 years ago

This bug is a pretty big deal for me. Because (correct me if I'm wrong), without using the expanded syntax, there is no other way to specify files in bulk (from a subdirectory).

The problem with normal src-dest mappings is this: If I specify src: 'app/*.html', dest: 'dist/', then the resulting file path is dist/app/<files>.html (Note the unintended app folder). There is no way to omit the src path prefix in order to make the resulting file path dist/<files>.html.

Src-dest mapping is meant for files specified by name, like src: 'app/special-file-1.html', dest: 'dist/special-file-1.html'. But for bulk tasks such as file copying, I need to use wildcards and subdirectories. The only way to do that is by using expand.

Please correct me if I'm wrong, but I know of no way of accomplishing bulk file operations without using expand.

blowsie commented 8 years ago

This bug is a pretty big deal for me. Because (correct me if I'm wrong), without using the expanded syntax, there is no other way to specify files in bulk (from a subdirectory).

I agree.

@tschaub do you have any idea what the issue is here? I can try to fix if I understand the issue.

hongaar commented 8 years ago

Is anyone working on this issue already, or anyone who would have some pointers? I would love to look into this as a solution to this bug would mean a huge speed increase for my grunt-watch tasks.

ts-web commented 8 years ago

Workaround

There is a way to run the currently-modified file. But it doesn't use newer.

You have to listen for the 'watch' event, and in your event handler, set up a jshint task to run only that one file. Here is the detailed explanation, from my project's Gruntfile:

First create a new jshint task for running only one file. Don't give it any source files — those will be added dynamically in the watch event handler.

jshint: {
  // ...
  one: {
    options: { /* ... */},
    files: [] // empty. Populated by watch event handler
  }
}

Then add a watch target for the watch task. This can be in addition to an existing watch task for the same js files. The tasks will be run after the watch event handler modifies their files objects (see below). Nospawn is required or else the watch task runs in a child process which cannot edit the other tasks' configs. Note: I am running jscs as well as jshint.

watch: {
  // ...
  js_linting: {
    options: {
      // nospawn or else the event handler can't change the jscs/jshint:one configs
      nospawn: true
    },
    files: '<%= config.app %>/scripts/**/*',
    tasks: ['jshint:one', 'jscs:one']
  }
}

Finally, the 'watch' event handler. A few notes: the target parameter corresponds to the watch task's config object's name, which I named js_linting (see code above).

// Set up watch listeners for individual files
grunt.event.on('watch', function watchEventListener(action, filepath, target) {
  // we don't need to run anything when a file is deleted
  if (action === 'deleted') {return;}

  // filter by watch target name
  if (target === 'js_linting') handleJSLinting();
  if (target === 'sass_linting') handleScssLinting();

  // ---------------------
  function handleJSLinting() {
    // tell the tasks what file to run. Watch target must be nospawn to have effect.
    grunt.config.set('jscs.one.files', [{src: filepath}]);
    grunt.config.set('jshint.one.files', [{src: filepath}]);
  }
  function handleScssLinting() {
    // tell the tasks what file to run. Watch target must be nospawn to have effect.
    grunt.config.set('scsslint.one.files', [{src: filepath}]);
  }
});

In a more complex scenario such as running karma, the task's file object would not be completely empty, since you'd need to include other scripts. You'd think you could just append the currently-edited file to that array, but no. Because the array persists for the whole duration of the watch task -- you'd end up adding many files. So instead, you would have to make a reference copy of the original files array, and use something like grunt.config.set('karma.one.files', [].concat(orig_karma_files, filepath))

codephobia commented 8 years ago

I wanted to add that we have a few machines in our office that have this issue, but it seems to only be happening on the windows 10 machines. Perhaps there is something to that.

ergose commented 8 years ago

I've got the same issue with "grunt-config-copy". I am using the "expand" syntax.

My Gruntfile has:

        copy: {
            main: {
                expand: true,
                cwd: 'copy2build',
                src: '**',
                dest: 'build',
                flatten: true,
                filter: 'isFile'
            }
        },

and the end result of "newer:copy":

$ grunt
Running "newer:jade" (newer) task

Running "newer:jade:compile" (newer) task
No newer files to process.

Running "newer:sass" (newer) task

Running "newer:sass:preview" (newer) task
No newer files to process.

Running "newer:coffee" (newer) task

Running "newer:coffee:0" (newer) task
No newer files to process.

Running "newer:copy" (newer) task

Running "newer:copy:main" (newer) task

Running "copy:main" (copy) task
Copied 33 files

Running "newer-postrun:copy:main:1:C:\Users\Brian\Development\BitBuckets\Web\salem289site\node_modules\grunt-newer\.cache" (newer-postrun) task

Done, without errors.

None of the 33 files have been touched, but they copy every time. I was going to use the workaround (many kudos @ts-web), but for 33 files, eh, it's annoying to have them copy, but it's not really a problem yet. I'd still like to see it fixed though.

krnlde commented 8 years ago

I'm getting the same issue on windows 10, node 5.8.0 in a huge (~1700 files) project. Pretty annoying. is there any approach or hint working with grunt-newer?

Darksoulsong commented 7 years ago

Issue opened since 2014. Any news?