Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Merge `custom-options` to allow additional tasks #426

Closed
wants to merge 1 commit into from

4 participants

@pangratz

Changes in 2ae12fb and fe0f687 made it possible to specify custom grunt configurations. The idea is to specify custom tasks inside the tasks/custom-options folder. For example there are already tasks for the copy plugin defined in tasks/options/copy.js, but we can specify a new task in tasks/custom-options/copy.js:

module.exports = {
  custom_copy_task: { ... }
}

The problem with the current implementation is that the custom copy configuration would completely overwrite the already defined one. That's because Lo-Dash's _.extend is used, which ... will overwrite property assignments of previous sources.

By using _.merge instead, the properties are merged since it ... recursively merges own enumerable properties of the source object(s), that don't resolve to undefined into the destination object so it is possible to specify a custom configuration and preserve the existing one, as illustrated with the following simple example:

var a = { first: { a: "a" } };
var b = { first: { b: "b" } };

grunt.file.write("tmp/extend.js", JSON.stringify(_.extend({}, a, b), null, 2));
grunt.file.write("tmp/merge.js", JSON.stringify(_.merge({}, a, b), null, 2));

diff tmp/extend.js tmp/merge.js --side-by-side results in the following diff:

{                          {
  "first": {                 "first": {
                   >           "a": "a",
    "b": "b"                   "b": "b"
  }                          }
}                          }

A task _log:config has been added which writes the used grunt configuration to tmp/grunt_config.js so the resulting configuration can be inspected to check if the custom tasks are configured correctly.


If this PR is merged, I'll give the gh-pages branch some :heart: and add documentation on how to specify custom tasks.

cc @MajorBreakfast @Pradeek

@pangratz pangratz Merge `custom-options` to allow additional tasks
Changes in 2ae12fb and
fe0f687  made it possible to specify
custom grunt configurations. The idea is to specify custom tasks inside the
`tasks/custom-options` folder. For example there are already tasks for the `copy`
plugin defined in `tasks/options/copy.js`, but we can specify a new task
in `tasks/custom-options/copy.js`:

```javascript
module.exports = {
  custom_copy_task: { ... }
}
```

The problem with the current implementation is that the custom `copy`
configuration would completely overwrite the already defined one. That's
because Lo-Dash's `_.extend` is used, which [`... will overwrite property
assignments of previous sources.`](http://lodash.com/docs#assign).

By using `_.merge` instead, the properties are merged since it
[`... recursively merges own enumerable properties of the source object(s),
that don't resolve to undefined into the destination
object.`](http://lodash.com/docs#merge) so it is possible to specify a
custom configuration and preserve the existing one, as illustrated with
the following simple example:

```javascript
var a = { first: { a: "a" } };
var b = { first: { b: "b" } };

grunt.file.write("tmp/extend.js", JSON.stringify(_.extend({}, a, b), null, 2));
grunt.file.write("tmp/merge.js", JSON.stringify(_.merge({}, a, b), null, 2));
```

`diff tmp/extend.js tmp/merge.js --side-by-side` results in the following diff:

```
{                          {
  "first": {                 "first": {
                   >           "a": "a",
    "b": "b"                   "b": "b"
  }                          }
}                          }
```

A task `_log:config` has been added which writes the used grunt
configuration to `tmp/grunt_config.js` so the resulting configuration
can be inspected to check if the custom tasks are configured correctly.
ac40962
@pangratz pangratz commented on the diff
Gruntfile.js
@@ -58,23 +59,28 @@ module.exports = function(grunt) {
// Loads task options from `tasks/options/` and `tasks/custom-options`
// and loads tasks defined in `package.json`
- var config = _.extend({},
- require('load-grunt-config')(grunt, {
- configPath: path.join(__dirname, 'tasks/options'),
- loadGruntTasks: false,

@MajorBreakfast do you recall why you specified loadGruntTasks: false here?

To avoid loading them twice.

Ok, so I think I should bring this back, right?

Here's the right explanation:

loadGruntTasks: false for not loading them twice
init: false is because we initialize the config at the very bottom of Gruntfile.js. For example the concurrent task gets configured about in the middle of Gruntfile.js

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

seems like a good thing to try out.

@pangratz pangratz commented on the diff
Gruntfile.js
((25 lines not shown))
);
grunt.loadTasks('tasks'); // Loads tasks in `tasks/` folder
config.env = process.env;
-
+ // Debug task to log the used Grunt configuration
+ grunt.registerTask('_log:config', "Write the used Grunt configuration to tmp/grunt_config.js", function() {
+ grunt.file.write("tmp/grunt_config.js", JSON.stringify(config, null, 2));
+ grunt.log.writeln("Used Grunt configuration written to tmp/grunt_config.js");
+ });

Thoughts about removing this debugging task and always write the used grunt configuration to tmp/used_grunt_config.js?

We could do that. But file ending for json is ".json"

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

Overwriting is the desired thing because it's deterministic. The idea is that if you want to build upon the default config then you require it and add your custom stuff.

  • I once used the very popular php based shop system "Magento" and they had a very complex inheritance hierachy. At the end it was very very hard to know what was overwritten by what and where. It's best to keep it nice and simple.
  • merge also merges deeper levels. Level that we don't want merged.

=> _.extend is what we should use

var config = require('../config/some-task');
config.someChange = { ... };
module.exports = config;

or

var _ = require('grunt').utils._;
module.exports _.merge({ // or _.extend, depends on what you want
  ...
}, require('../config/some-task'));
@MajorBreakfast

Btw. documenting that is a good idea :)

@MajorBreakfast

Btw.

newObject = _.extend(someObject, anotherObject) // Overwrites someObject
newObject = _.extend({}, someObject, anotherObject) // Better (unless your object is super large)
@MajorBreakfast

I think this PR can be closed, right? -> No merge
@pangratz Is that okay for you?

@pangratz

@MajorBreakfast jup, sounds good. I'll try to find some time to update the docs and maybe include a sample task in tasks/custom-options. Thanks for your input! Very valuable.

@pangratz pangratz closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 18, 2014
  1. @pangratz

    Merge `custom-options` to allow additional tasks

    pangratz authored
    Changes in 2ae12fb and
    fe0f687  made it possible to specify
    custom grunt configurations. The idea is to specify custom tasks inside the
    `tasks/custom-options` folder. For example there are already tasks for the `copy`
    plugin defined in `tasks/options/copy.js`, but we can specify a new task
    in `tasks/custom-options/copy.js`:
    
    ```javascript
    module.exports = {
      custom_copy_task: { ... }
    }
    ```
    
    The problem with the current implementation is that the custom `copy`
    configuration would completely overwrite the already defined one. That's
    because Lo-Dash's `_.extend` is used, which [`... will overwrite property
    assignments of previous sources.`](http://lodash.com/docs#assign).
    
    By using `_.merge` instead, the properties are merged since it
    [`... recursively merges own enumerable properties of the source object(s),
    that don't resolve to undefined into the destination
    object.`](http://lodash.com/docs#merge) so it is possible to specify a
    custom configuration and preserve the existing one, as illustrated with
    the following simple example:
    
    ```javascript
    var a = { first: { a: "a" } };
    var b = { first: { b: "b" } };
    
    grunt.file.write("tmp/extend.js", JSON.stringify(_.extend({}, a, b), null, 2));
    grunt.file.write("tmp/merge.js", JSON.stringify(_.merge({}, a, b), null, 2));
    ```
    
    `diff tmp/extend.js tmp/merge.js --side-by-side` results in the following diff:
    
    ```
    {                          {
      "first": {                 "first": {
                       >           "a": "a",
        "b": "b"                   "b": "b"
      }                          }
    }                          }
    ```
    
    A task `_log:config` has been added which writes the used grunt
    configuration to `tmp/grunt_config.js` so the resulting configuration
    can be inspected to check if the custom tasks are configured correctly.
This page is out of date. Refresh to see the latest.
Showing with 18 additions and 12 deletions.
  1. +18 −12 Gruntfile.js
View
30 Gruntfile.js
@@ -48,7 +48,8 @@ module.exports = function(grunt) {
var Helpers = require('./tasks/helpers'),
filterAvailable = Helpers.filterAvailableTasks,
_ = grunt.util._,
- path = require('path');
+ path = require('path'),
+ loadGruntConfig = require('load-grunt-config');
Helpers.pkg = require("./package.json");
@@ -58,23 +59,28 @@ module.exports = function(grunt) {
// Loads task options from `tasks/options/` and `tasks/custom-options`
// and loads tasks defined in `package.json`
- var config = _.extend({},
- require('load-grunt-config')(grunt, {
- configPath: path.join(__dirname, 'tasks/options'),
- loadGruntTasks: false,

@MajorBreakfast do you recall why you specified loadGruntTasks: false here?

To avoid loading them twice.

Ok, so I think I should bring this back, right?

Here's the right explanation:

loadGruntTasks: false for not loading them twice
init: false is because we initialize the config at the very bottom of Gruntfile.js. For example the concurrent task gets configured about in the middle of Gruntfile.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- init: false
- }),
- require('load-grunt-config')(grunt, { // Custom options have precedence
- configPath: path.join(__dirname, 'tasks/custom-options'),
- init: false
- })
+ var config = _.merge(
+ loadGruntConfig(grunt, {
+ configPath: path.join(__dirname, 'tasks/options'),
+ init: false
+ }),
+
+ // Custom options have precedence
+ loadGruntConfig(grunt, {
+ configPath: path.join(__dirname, 'tasks/custom-options'),
+ init: false
+ })
);
grunt.loadTasks('tasks'); // Loads tasks in `tasks/` folder
config.env = process.env;
-
+ // Debug task to log the used Grunt configuration
+ grunt.registerTask('_log:config', "Write the used Grunt configuration to tmp/grunt_config.js", function() {
+ grunt.file.write("tmp/grunt_config.js", JSON.stringify(config, null, 2));
+ grunt.log.writeln("Used Grunt configuration written to tmp/grunt_config.js");
+ });

Thoughts about removing this debugging task and always write the used grunt configuration to tmp/used_grunt_config.js?

We could do that. But file ending for json is ".json"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
// App Kit's Main Tasks
Something went wrong with that request. Please try again.