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

Merge options should extend arrays or be customizable #246

Open
LoveIsGrief opened this issue Apr 12, 2017 · 3 comments
Open

Merge options should extend arrays or be customizable #246

LoveIsGrief opened this issue Apr 12, 2017 · 3 comments

Comments

@LoveIsGrief
Copy link

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"] }
LoveIsGrief added a commit to LoveIsGrief/grunt-karma that referenced this issue Apr 12, 2017
By default `_.merge` incrementally overwrites array when merging.
This can lead to surprising behavior when default options exist.

This will let values be added to existing arrays.

Closes karma-runner#246
LoveIsGrief added a commit to LoveIsGrief/grunt-karma that referenced this issue Apr 12, 2017
Missing space between function name and paranthesis

Part of karma-runner#246
LoveIsGrief pushed a commit to LoveIsGrief/grunt-karma that referenced this issue Apr 12, 2017
By default `_.merge` incrementally overwrites array when merging.
This can lead to surprising behavior when default options exist.

This will let values be added to existing arrays.

Closes karma-runner#246
@Krinkle
Copy link
Collaborator

Krinkle commented Aug 29, 2018

@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
Copy link
Contributor

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).

LoveIsGrief added a commit to LoveIsGrief/grunt-karma-poc-246 that referenced this issue Nov 9, 2018
@LoveIsGrief
Copy link
Author

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.

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

No branches or pull requests

3 participants