karma-runner / grunt-karma

Grunt plugin for Karma.
MIT License
468 stars 116 forks source link

Merge options should extend arrays or be customizable #246

Open ghost opened 7 years ago

ghost commented 7 years ago

When overriding options e.g the plugins and frameworks, the default behavior of _.merge returns (for me) surprising results (see below). Therfore I believe it should either be possible to

a. customize merges using a new option e.g mergeCustomizer

b. extend array by default e.g

merge(
  { frameworks: [ "jasmine", "browserify"]}, 
  { frameworks: ["detect-browsers"]
) // -> { frameworks: [ "jasmine", "browserify", "detect-browsers" ] }

_.merge behavior

var object = {
  'a': ["two", "four"]
};

var other = {
  'a': ["one"]
};

_.merge(object, other);
// => { a: [ "one", "four"] }
var object = {
  'a': ["two", "four"]
};

var other = {
  'a': ["one", "five", "six"]
};

_.merge(object, other);
// => { a: [ "one", "five", "six"] }
Krinkle commented 6 years ago

@johnjbarton It's not entirely clear to me which two pieces of configuration are being merged by the code in question. I assume this isn't about task options vs task data, but something else rather. Is that right?

johnjbarton commented 6 years ago

Looks like this is grunt task data (specified in gruntfile) merging with command line 'options'. If the PR had tests this would be clearer (and I would ask for one).

ghost commented 6 years ago

Please have a look at https://github.com/LoveIsGrief/grunt-karma-poc-246 for an example of the behavior I was talking about. It's a contrived example since I cannot remember the exact scenario. It's been too long.

As for writing tests, I think it's out of scope for this bug. Tests don't exist for this repo and it would be a different task to add them just to test my PR. I hope the repo I created is enough.