yeoman / generator

Rails-inspired generator system that provides scaffolding for your apps
http://yeoman.io
BSD 2-Clause "Simplified" License
1.22k stars 299 forks source link

[Composability] Gruntfile API #432

Closed SBoudrias closed 10 years ago

SBoudrias commented 10 years ago

In order to bring composable Generators, it seems obvious some file will need to be edited by multiple generators.

I think the main friction point here is the Gruntfile.js.

In order to allow this file from being easily edited, I'd like to propose a centralized API for interaction with the Gruntfile AST.

insertConfig

To add section of Gruntfile config.

/**
 * Basic signature
 * @param {String} config name
 * @param {String} config content
 */
this.gruntfile.insertConfig("compass", "{ watch: { options: { 'watch': true } } }");

// And in a more probable way
this.gruntfile.insertConfig("compass", this.src.read("grunt-compass"));

If multiple insert the same config, the last win.

registerAs

To add a task in a named task group.

/**
 * Basic signature
 * @param {String} config name
 * @param {String} task name
 */
this.gruntfile.registerAs("compass:prod", "default");
this.gruntfile.registerAs("jshint", "default");

// or maybe this one is better
this.gruntfile.registerTask({ "default": "compass:prod" });
this.gruntfile.registerTask({ "default": "jshint" });

// or multiple at once
this.gruntfile.registerTask({ "default": [ "jshint", "compass:prod" ] });

// Who'd output
grunt.registerTask("default", [ "compass:prod", "jshint" ]);

Maybe we should add a param to prepend a task rather than always appending.

Implication

This mean the Generator system would be responsible for creating the basic Gruntfile structure. Unless there's already a Gruntfile.js at the root of the project, then we use this one as a base.

This also mean we probably should suggest defaults named grouped tasks like "test", "default", "release", etc.

addyosmani commented 10 years ago

The proposed API actually LGTM. What about API methods for injecting arbitrary functions or content to the end of the Gruntfile? Is this something we want to enable, even if not encourage directly?.

I also wonder if the introduction of this API would mean that instead of the current strategy of including template strings in the Gruntfile (e.g for data population, conditional inclusion) one would rely on this API for injection directly from inside the generator.

+1 on defaults.

SBoudrias commented 10 years ago

What about API methods for injecting arbitrary functions or content to the end of the Gruntfile?

Do you have an example?

I also wonder if the introduction of this API would mean that instead of the current strategy of including template strings in the Gruntfile (e.g for data population, conditional inclusion) one would rely on this API for injection directly from inside the generator.

Yes exactly, you could template each independant task template string prior to including in the Gruntfile.

stephenplusplus commented 10 years ago

Looks sweet! I couldn't tell if this is already in your proposal, but does this cover inserting a target into an already existing task? I imagine a lot of tools would be using watch, for example.

Re: syntax, I prefer this.gruntfile.registerTask over this.gruntfile.registerAs, since it's more true to what we're doing (e.g. "registering a task in the gruntfile" versus "registering gruntfile as").

SBoudrias commented 10 years ago

@stephenplusplus Oh, good comment about target. Maybe we should force inserted task to use a target so multiple one can be added without overriding another one config.

SBoudrias commented 10 years ago

Also, what default task "groups" should we always keep?

And maybe we should add some as life cycle events; for example beforeServe could keep all task expected to run before someone launch grunt serve.

SBoudrias commented 10 years ago

Also, some generator register custom tasks: https://github.com/yeoman/generator-angular/blob/master/templates/common/Gruntfile.js#L399-L412

Maybe we should allow full task content specified with this.gruntfile.registerTask, and this.gruntfile.addTaskInGroup to push task in the group array.

addyosmani commented 10 years ago

Looks like for many of the things generator-webapp may need we have micro-generators available that could be 'composed' to achieve what we do today: https://gist.github.com/addyosmani/8059506. Many are missing the exact boilerplate code we generate but this could be fixed.

addyosmani commented 10 years ago

Re: task groups, I would say keep: default, test and release. Some lifecycle events would be great. I can see a use for before/afterServe.

Maybe we should allow full task content specified with this.gruntfile.registerTask, and this.gruntfile.addTaskInGroup to push task in the group array.

I agree.

Will we also need to consider an API for Gulp?

SBoudrias commented 10 years ago

Will we also need to consider an API for Gulp?

Maybe if we decide to work more with it. But I'd really start with Grunt as it support a broader range of utility (live reloads, server, etc). Also, Gruntfile is easy to compose, as it is a series of settings. Gulp working on stream would make composing tasks much harder. A potential API would have to be think differently.

addyosmani commented 10 years ago

Sounds good to me :)

grawk commented 10 years ago

:+1: on this feature. I don't suppose this is already planned as a core enhancement? While I'd love it as a feature I don't have the disposable hours currently to submit a pull request.

SBoudrias commented 10 years ago

@grawk It is planned and I'm working on it ATM.

My work is currently on my fork branch: https://github.com/SBoudrias/generator/tree/gruntfile-api

But I've hit some limitation with AST-query, so I'm currently working on a complete rewrite. Once this is completed, I think we'll be good to finish the API inside the core.

So, it is for sooner than later. But there's still a lot of work to do.

grawk commented 10 years ago

@SBoudrias thanks! Will look for further updates to this thread.

bezoerb commented 10 years ago

@SBoudrias I'm working on some similar library at the moment. Maybe we can bring some things together. https://github.com/bezoerb/gruntfile-api/

SBoudrias commented 10 years ago

@bezoerb That's pretty neat! I'll check it out more, but if it match our use case, we may implement it directly with your tool!

SBoudrias commented 10 years ago

@bezoerb I have a couple questions on your gruntfile-api projects.

  1. How insertConfig (and getJsonTasks) react if we pass in an object with functions and dates objects? And how would it react if it uses variables? Looks like the variable will be lost and replace by its core value; meaning addGlobalDeclaration is not usefull.
  2. How does registerTask react if the the identifier is already used. Anyway to append/prepend tasks to a group?
  3. Does insertConfig allow to override/extend a previous configuration?
bezoerb commented 10 years ago

@SBoudrias Thanks for the feedback. I'll try to answer your questions

  1. Good point. Until now the variable as well as function calls like (new Date()).getTime()will be replaced by its core value. I'll have to figure out a way to make the use of those values possible without loosing the ability to paste plain objects instead of text.
  2. Right now, there would be two tasks registered with the same identifier. I'll address append/prepend option
  3. insertConfig should automatically extends the task if there is a previous configuration until you don't overwrite things. When there already is a watch task like
watch: {
      lib: {
          files: '<%= jshint.lib.src %>',
          tasks: ['jshint:lib', 'nodeunit']
      },
      test: {
          files: '<%= jshint.test.src %>',
          tasks: ['jshint:test', 'nodeunit']
      }
 }

insertConfig would merge a configuration like insertConfig('watch',{js: { ... }}) but it will skip another lib or test configuration. Right now i'm not quite sure untill which level merging should be allowed without messing up any previous configuration. Maybe adding an extra options parameter to the functions to make things more flexible.

This project is in a very early state and i appreciate every feedback and feature requests to enhance usability. Feel free to add some issues ;) And if you have good answers to those questions let me know.

SBoudrias commented 10 years ago

Well, for n. 1, I think the only solution is to pass text rather than objects. (At least allow passing raw text as argument and simply replacing the block)

The biggest issue I see is that you've considered the configuration object passed to grunt.initConfig to be valid JSON. IMO, this is a design mistake.

bezoerb commented 10 years ago

For 1: What about using some sort of patterm like {test: '@(new Date()).getTime()'which mark the text as use this without evaluating and add an insertRawConfig function to allow plain text. Do you have an example where valid json doesn't fit the needs?

bezoerb commented 10 years ago

lets move this discussion to https://github.com/bezoerb/gruntfile-api/issues/2

bezoerb commented 10 years ago

For now, 1 and 2 should be solved. Configuration of 3 is on its way

bezoerb commented 10 years ago

@SBoudrias, @addyosmani I just published WIP version of generator-spritesmith which uses gruntfile-api to modify an existing Gruntfile so it can be used for any project without dropping the existing configuration. Maybe you could take a look and make some suggestions for improvement. Thanks

bezoerb commented 10 years ago

@SBoudrias, @addyosmani ping

SBoudrias commented 10 years ago

Hey @bezoerb, I just send a similar commit on Yeoman. See #525.

I had that on a branch for quite a while now and just took some time to wrap it up today. I use AST-Query to modify the AST to keep my sanity. It'd be good to eventually get this out of the core and on a standalone module like yours. Would you be interested in moving from raw esprima to AST-Query? (I'm personally reticent to use your module ATM because it'll be quite a hurdle to change and update in the future)

bezoerb commented 10 years ago

Hi @SBoudrias,

I would like to provide my module to yeoman but i'll first have to evaluate if AST-Query provides all the functionality needed for my module. If it matches all my needs i could move from raw esprima to your module. If not it might be an option to modularize/cleanup my code a bit more so that it's not that big hurdle to change/update in the future. Currently i don't have much time but I'll see when i can check in for that.

SBoudrias commented 10 years ago

@bezoerb As we needed something for Yeoman, I created a standalone module over here: https://github.com/SBoudrias/gruntfile-editor/

I added you as a collaborator so feel free to hack around!