New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Task Dependencies #968

Open
ryanflorence opened this Issue Nov 5, 2013 · 12 comments

Comments

Projects
None yet
9 participants
@ryanflorence

ryanflorence commented Nov 5, 2013

in rake you can:

task :foo => [:bar, :baz] do
  # do stuff after :bar and :baz
end

In grunt you can:

grunt.registerTask('foo', ['bar', 'baz']);

Would love to finish that off with something like:

grunt.registerTask('foo', ['bar', 'baz'], function() {
  // do stuff after 'bar' and 'baz'
});
@tkellen

This comment has been minimized.

Show comment
Hide comment
@tkellen

tkellen Nov 5, 2013

Member

Using what Grunt currently offers, you can accomplish the same with this:

grunt.registerTask('qux', function() {
...
});

grunt.registerTask('foo', ['bar','baz','qux'])
Member

tkellen commented Nov 5, 2013

Using what Grunt currently offers, you can accomplish the same with this:

grunt.registerTask('qux', function() {
...
});

grunt.registerTask('foo', ['bar','baz','qux'])
@shama

This comment has been minimized.

Show comment
Hide comment
@shama

shama Nov 5, 2013

Member

@tkellen We had a discussion on irc about this. He is asking to be able to be able to specify tasks that must be ran before a given task. Sort of what this.requires() does now but if the task hasn't ran, it would run it first.

Something more akin to this:

grunt.registerTask('foo', ['bar', 'baz']).requires('beforefoo');
// OR
grunt.registerTask('foo', function() {
  this.requires('beforefoo'); // But run the 'beforefoo' task first if it hasnt
});

My main concern with this is, IMO, effectively all tasks in Grunt are modules. Modules that will likely get published to npm and updated over time. So this may seems like a simple helper to mimic what other build systems do but I feel could have much more ramifications in the node.js community.

Here is an example of my concern. We have a minify task that will only minify if jshint passes. The minify task is written and tested with jshint@0.1.0. The minify@0.1.0 task relies on checking grunt.fail.errorcount to know if jshint passed or failed. Everything works.

Later, jshint updates to 0.2.0 and no longer increments grunt.fail.errorcount. Because our minify task is relying on a neighboring jshint task to exist, our minify task will unexpectedly begin failing. This isn't an issue if we're only dealing with a handful of tasks but with Grunt, we have hundreds. I feel this setup will require us to constantly keep tasks updated or never break compatibility. Both of which are really, really hard for us, much less the community to do.

The alternative, if Grunt tasks were just modules (as I believe they will fully be in Grunt v0.5), a task would consume another task like any other dependency:

grunt.registerTask('minify', function() {
  require('grunt-contrib-jshint').run(this.config, function() {
    /* has jshint passed or failed? */
  });
});

Yes this is more verbose and not as elegant but it is safe. The minify task will always work correctly with the jshint task. The end user is free to upgrade their version of jshint and it won't break the minify task. The minify task author now gets time to test and update their task.

Maybe I am over thinking this, I dont know. In my opinion a feature like this, if done incorrectly, will create more peer dependencies and that is something I would like to avoid.

Member

shama commented Nov 5, 2013

@tkellen We had a discussion on irc about this. He is asking to be able to be able to specify tasks that must be ran before a given task. Sort of what this.requires() does now but if the task hasn't ran, it would run it first.

Something more akin to this:

grunt.registerTask('foo', ['bar', 'baz']).requires('beforefoo');
// OR
grunt.registerTask('foo', function() {
  this.requires('beforefoo'); // But run the 'beforefoo' task first if it hasnt
});

My main concern with this is, IMO, effectively all tasks in Grunt are modules. Modules that will likely get published to npm and updated over time. So this may seems like a simple helper to mimic what other build systems do but I feel could have much more ramifications in the node.js community.

Here is an example of my concern. We have a minify task that will only minify if jshint passes. The minify task is written and tested with jshint@0.1.0. The minify@0.1.0 task relies on checking grunt.fail.errorcount to know if jshint passed or failed. Everything works.

Later, jshint updates to 0.2.0 and no longer increments grunt.fail.errorcount. Because our minify task is relying on a neighboring jshint task to exist, our minify task will unexpectedly begin failing. This isn't an issue if we're only dealing with a handful of tasks but with Grunt, we have hundreds. I feel this setup will require us to constantly keep tasks updated or never break compatibility. Both of which are really, really hard for us, much less the community to do.

The alternative, if Grunt tasks were just modules (as I believe they will fully be in Grunt v0.5), a task would consume another task like any other dependency:

grunt.registerTask('minify', function() {
  require('grunt-contrib-jshint').run(this.config, function() {
    /* has jshint passed or failed? */
  });
});

Yes this is more verbose and not as elegant but it is safe. The minify task will always work correctly with the jshint task. The end user is free to upgrade their version of jshint and it won't break the minify task. The minify task author now gets time to test and update their task.

Maybe I am over thinking this, I dont know. In my opinion a feature like this, if done incorrectly, will create more peer dependencies and that is something I would like to avoid.

@vladikoff

This comment has been minimized.

Show comment
Hide comment
@vladikoff

vladikoff Nov 5, 2013

Member

Hah I'm still lost about what @shama is talking about.
.requires('beforefoo'); that's not a node module, that is a task registered in the Grunt initConfig. This can be .requires('beforefoo:ANY_TARGET_HERE');

We drafted this example:

jshint: {
  // jshint, src, dest
},
copy: {
  // copy config , src, dest
}

'gh-pages': {
  dependsOn: ['build'] //*** define this ****
  dependsOn: ['jshint', 'copy'] //*** OR define this ****
  // gh-pages config .... 
  }
},


grunt.registerTask('build',  ['jshint', 'copy']);

For local tasks

grunt.registerTask('gh-pages', ... ).dependsOn('build');
// Simple example: 
// `grunt gh-pages` --> 'dependsOn: build'
// I see you are trying to run gh-pages,
   //  I need to run 'build' first, otherwise this task will be useless


// Advanced 'make'-like example: `grunt gh-pages` --> build --> 
  // jshint: have the files that were validated earlier change?  
       // --> YES: run this task , NO: don't run this task
  // copy are the files in 'dest'? (did the source of this task change since last time?) 
       // --> YES: don't run this task, NO: don't run the task

The point of this feature is to avoid useless Alias tasks like this:
grunt.registerTask('ghPagesForRealThisTime', ['build','gh-pages'])

To me this feature has nothing to do with node modules or loaded plugins (or npm modules, etc), that is all done separately and should not affect it. I think @shama is talking about a different feature where you can use existing plugins that people built in your local tasks. Example: I'm building a thing to deploy apps, I want to use grunt-contrib-compess and use it to zip my app instead of rewriting zip logic.

Member

vladikoff commented Nov 5, 2013

Hah I'm still lost about what @shama is talking about.
.requires('beforefoo'); that's not a node module, that is a task registered in the Grunt initConfig. This can be .requires('beforefoo:ANY_TARGET_HERE');

We drafted this example:

jshint: {
  // jshint, src, dest
},
copy: {
  // copy config , src, dest
}

'gh-pages': {
  dependsOn: ['build'] //*** define this ****
  dependsOn: ['jshint', 'copy'] //*** OR define this ****
  // gh-pages config .... 
  }
},


grunt.registerTask('build',  ['jshint', 'copy']);

For local tasks

grunt.registerTask('gh-pages', ... ).dependsOn('build');
// Simple example: 
// `grunt gh-pages` --> 'dependsOn: build'
// I see you are trying to run gh-pages,
   //  I need to run 'build' first, otherwise this task will be useless


// Advanced 'make'-like example: `grunt gh-pages` --> build --> 
  // jshint: have the files that were validated earlier change?  
       // --> YES: run this task , NO: don't run this task
  // copy are the files in 'dest'? (did the source of this task change since last time?) 
       // --> YES: don't run this task, NO: don't run the task

The point of this feature is to avoid useless Alias tasks like this:
grunt.registerTask('ghPagesForRealThisTime', ['build','gh-pages'])

To me this feature has nothing to do with node modules or loaded plugins (or npm modules, etc), that is all done separately and should not affect it. I think @shama is talking about a different feature where you can use existing plugins that people built in your local tasks. Example: I'm building a thing to deploy apps, I want to use grunt-contrib-compess and use it to zip my app instead of rewriting zip logic.

@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Nov 5, 2013

Member

I really don't want tasks to be able to require other tasks in this way, because it would really become a dependency management nightmare.

I don't think I mind the idea of defining an anonymous function task inline in an array of tasks like so, however:

grunt.registerTask('foo', 'my awesome foo task', ['bar', 'baz', function() {
  // do stuff after 'bar' and 'baz' but before 'qux'
}, 'qux']);

I need to think about it more.

Member

cowboy commented Nov 5, 2013

I really don't want tasks to be able to require other tasks in this way, because it would really become a dependency management nightmare.

I don't think I mind the idea of defining an anonymous function task inline in an array of tasks like so, however:

grunt.registerTask('foo', 'my awesome foo task', ['bar', 'baz', function() {
  // do stuff after 'bar' and 'baz' but before 'qux'
}, 'qux']);

I need to think about it more.

@tkellen

This comment has been minimized.

Show comment
Hide comment
@tkellen

tkellen Nov 5, 2013

Member

Ahh @vladikoff, that makes more sense now. @cowboy, that seems interesting but it feels weird that the command line arguments would only be passed into the anonymous function.

Member

tkellen commented Nov 5, 2013

Ahh @vladikoff, that makes more sense now. @cowboy, that seems interesting but it feels weird that the command line arguments would only be passed into the anonymous function.

@cowboy

This comment has been minimized.

Show comment
Hide comment
@cowboy

cowboy Nov 5, 2013

Member

Yeah, I don't think that I like it.

Member

cowboy commented Nov 5, 2013

Yeah, I don't think that I like it.

@thanpolas

This comment has been minimized.

Show comment
Hide comment
@thanpolas

thanpolas Nov 12, 2013

isn't this the whole deal with the 0.5.x series? The node-task @tkellen has been working on?

thanpolas commented Nov 12, 2013

isn't this the whole deal with the 0.5.x series? The node-task @tkellen has been working on?

@tkellen

This comment has been minimized.

Show comment
Hide comment
@tkellen

tkellen Nov 12, 2013

Member

I've been out of commission for the last 6 months. A lot of work remains. I'd love to see node-task adopted, but Ben and I need to spend more time discussing what that actually looks like.

Member

tkellen commented Nov 12, 2013

I've been out of commission for the last 6 months. A lot of work remains. I'd love to see node-task adopted, but Ben and I need to spend more time discussing what that actually looks like.

@wilmoore

This comment has been minimized.

Show comment
Hide comment
@wilmoore

wilmoore Nov 13, 2013

@rpflorence: Would be great to also see an example of how you were thinking this might be used. That may better inform the actual implementation.

wilmoore commented Nov 13, 2013

@rpflorence: Would be great to also see an example of how you were thinking this might be used. That may better inform the actual implementation.

@vladikoff

This comment has been minimized.

Show comment
Hide comment
@vladikoff
Member

vladikoff commented Nov 14, 2013

@wilmoore agreed //cc @rpflorence

@tbranyen

This comment has been minimized.

Show comment
Hide comment
@tbranyen

tbranyen Jan 30, 2014

Member

This could be used in cases where a linear build path doesn't necessarily make sense. If I want to add a new build task into my system. I should be able to just throw it anywhere in the list of tasks to run and it will only execute once it's dependencies have been resolved.

This would also open the door to parallel tasks being able to run as we are now depending on the tree instead of a linear process that has no idea. It's essentially the argument between script tags and something like AMD.

Member

tbranyen commented Jan 30, 2014

This could be used in cases where a linear build path doesn't necessarily make sense. If I want to add a new build task into my system. I should be able to just throw it anywhere in the list of tasks to run and it will only execute once it's dependencies have been resolved.

This would also open the door to parallel tasks being able to run as we are now depending on the tree instead of a linear process that has no idea. It's essentially the argument between script tags and something like AMD.

@paulbhartzog

This comment has been minimized.

Show comment
Hide comment
@paulbhartzog

paulbhartzog Nov 11, 2014

Not sure if I follow correctly but since someone asked for an example here's mine. :-)

If you use SASS and CSS validation, then wouldn't you need to make sure that your SASS has been converted to CSS before you fire off the CSS validation task?

If I'm missing something here please enlighten me :-)

paulbhartzog commented Nov 11, 2014

Not sure if I follow correctly but since someone asked for an example here's mine. :-)

If you use SASS and CSS validation, then wouldn't you need to make sure that your SASS has been converted to CSS before you fire off the CSS validation task?

If I'm missing something here please enlighten me :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment