Skip to content
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

Allow for optional merging of Object type option props #738

Open
thanpolas opened this issue Mar 21, 2013 · 6 comments
Open

Allow for optional merging of Object type option props #738

thanpolas opened this issue Mar 21, 2013 · 6 comments

Comments

@thanpolas
Copy link

Augment this.options() method with one more argument to allow for properties of type Object to merge instead of overwrite.

case:

  multiTask: {
    options: {
      anObject: {
        a: 1
      }
    },
    target: {
      options: {
        anObject: {
          b: 2
        }
      }
    }
  }

// in the task
grunt.registerMultiTask('multiTask', function() {
  var options = this.options({}, true);

  // desired result
  // options.anObject === {a:1, b:2}

});
@cowboy
Copy link
Member

cowboy commented Mar 21, 2013

I think you mean:

options.anObject // {a:1, b:2}

@thanpolas
Copy link
Author

right, corrected

@NickHeiner
Copy link

+1 This would make my life easier. I have a node module that provides default grunt options for other projects, and it would be nicer if it was possible to merge instead of overwrite options.

@shama
Copy link
Member

shama commented Jun 24, 2013

The main issue I see with this is the end user may not know whether a task's options will be merged or overwritten. I think it will cause confusion when their options get merged with some tasks but not others.

IMO a better way would be to let the user handle this to keep it transparent. The user could easily make a helper module for this, if they wanted:

var mergeOpts = (function() {
  var allopts = Object.create(null);
  return function(ns, opts) {
    allopts[ns] = grunt.util._.merge({}, allopts[ns], opts);
    return allopts[ns];
  }
}());
grunt.initConfig({
  task: {
    options: mergeOpts('task', {
      anObject: { a: 1 }
    }),
    target: {
      options: mergeOpts('task', {
        anObject: { b: 2 }
      }),
    }
  }
});

@thanpolas
Copy link
Author

IMO a better way would be to let the user handle this to keep it transparent.

Yes, that makes better sense.

An effort has to be made to better elaborate that what grunt.initConfig() essentially needs is just a plain Object literal. Although that sounds self-evident from this standpoint, it's not quite how a new-comer perceives it...

@doowb
Copy link

doowb commented Jul 8, 2013

@shama @thanpolas I think a better way would be to merge the options inside your task. If you're a plugin author, then I you can provide an option to do this, or just do it based on your situation, and provide documentation on how it works and what's to be expected.

We do this for options that are arrays of strings:

module.exports = function(grunt) {

  var customTaskName = 'customTask';

  grunt.registerMultiTask(customTaskName, 'This is a custom task.', function() {

    var arrayName = 'customArrayName';

    var taskOpts = grunt.config([customTaskName, 'options', arrayName]) || [];
    var targetOpts = grunt.config([customTaskName, this.target, 'options', arrayName]) || [];

    var mergedOpts = grunt.util._.union(taskOpts, targetOpts);
  });
};

You should be able to do this with your entire options object too...

module.exports = function(grunt) {

  var customTaskName = 'customTask';

  grunt.registerMultiTask(customTaskName, 'This is a custom task.', function() {

    var taskOpts = grunt.config([customTaskName, 'options']) || {};
    var targetOpts = grunt.config([customTaskName, this.target, 'options']) || {};

    var mergedOpts = grunt.util._.merge({}, taskOpts, targetOpts);
  });
};

I hope this helps someone.

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

No branches or pull requests

5 participants