tschaub / grunt-newer

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

options by task #75

Open mattkime opened 9 years ago

mattkime commented 9 years ago

i've spent some time trying to figure out why this is failing for node 0.8.x. works fine on my box but would like to get to the bottom of it.

mattkime commented 9 years ago

@tschaub I'm considering forking grunt-newer due to lack of activity solving cases where file filtering isn't wanted or isn't directly possible. I'd rather work within the existing grunt-newer project but there's been little in the way of feedback.

tschaub commented 9 years ago

@mattkime I appreciate your effort to try to make this more usable. However, I don't think this change makes a significant improvement. There are now two ways to provide an override, and the second only saves a bit of typing.

In addition, supporting options named like other tasks has two potential issues. First, any option added in the future (regular options for the newer task) could collide with the names of tasks for which people had previously been trying to provide a custom override. For example, your change would support this:

  newer: {
    options: {
      foo: {
        override: overrideForFoo
      }
    }
  }

And in the future, if the newer task supported a foo option, it would break this use without knowing it.

The second issue is that people might wrongly assume that they could provide other per-task options in this same way. Why is the override option special? If there are two ways to provide the override option, it might be surprising that all options don't work the same way.

I know that this is a bit pedantic (especially since there are only two options for the task). But I think a general fault of Grunt is that it provides too many ways to do the same thing (e.g. specify src and dest files), and task handling is sloppy and awkward because of it.

So, I think it would be great to make it easier to make grunt-newer work with LESS imports (or other tasks that use source files not specified in the task directly). I think instead of supporting a different way to specify the override option, it would be more useful to support something like this:

  newer: {
    options: {
      override: overrides({
        less: overrides.less(),
        someOtherTaskName: overrides.someOtherOverride(whateverConfig)
      })
    }
  }

And the grunt-newer package could include a set of common overrides (e.g. one for checking LESS imports).

mattkime commented 9 years ago

@tschaub i'm a bit confused about how to apply the overrides option to the simplest of use cases - if one file changes, lets include all files. (yeah, a bit off topic)

i agree that grunt has its weaknesses, but I've yet to find an alternative that makes switching worthwhile. (gulp does have its virtues, not sure if they're sufficient to switch)

i see your point about task vs newer option namespace collisions. after chewing on this for a while i think it might be better to throw caution to the wind and put the newer task config directly inside the task or target.

uglify: {
    options: {
        newer: { // for all uglify targets
            override: function(){}
        }
    },
    build: {
        newer: { // target specific config ... hm, would this ever be useful? maybe this is a task level thing
            override: function2(){}
        },
        src: 'src/<%= pkg.name %>.js',
        dest: 'build/<%= pkg.name %>.min.js'
    }
}

yes, yes, there may be collisions. but this is how i want to express what needs to happen. perhaps '_newer' instead of 'newer'? or make it configurable via newer task options?

restating the problem - how can we supply config info for a task that modifies other tasks? and can we do it in such a way that allows for additional options?