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

Fix merging with empty values #185

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

johnlife
Copy link

Adds isEmpty option. Then the following code

const newObject = {
  foo: {
    one: null,
    two: "New text",
    three: ""
  }
}

const defaultObject = {
  foo: {
    one: "Some default text",
    two: "Some other default text",
    three: "Even more default text"
  }
}

const mergedObject = deepmerge(defaultObject, newObject, {
  isEmpty: a => a === null || a === '',
});
console.log(mergedObject);

gives you

{ 
  foo: { 
    one: 'Some default text',
     two: 'New text',
     three: 'Even more default text' 
  } 
}

@TehShrike
Copy link
Owner

This is way too specialized/specific. It's fine as a fork to use in a specific project, but isn't a good feature for a broadly-used utility.

I am open to changing the customMerge function signature to make it easy to solve for your use case without having to modify the code, though.

@pkuczynski
Copy link

@TehShrike I need this too! So I basically want to merge only when the new object has a value (avoiding nulls overriding). Something like here:

https://www.rubydoc.info/gems/deep_merge/1.2.1

:merge_nil_values       DEFAULT: false
  Set to true to merge nil hash values, overwriting a possibly non-nil value

This does not seem to be possible to do using customMerge function? However, it seems to be quite common case, so I would really welcome a new merge option

mergeNilValues DEFAUL: true

This would not break existing behavior but would allow to turn it off.

@n0v1
Copy link

n0v1 commented Apr 28, 2020

I have the same use case to ignore empty strings when merging. I think it would make sense to extend the signature of customMerge to provide values, so this and similar use cases could easily be solved. This would make this package a lot more flexible, in my opinion.

@panec
Copy link

panec commented Jun 5, 2020

I would love to have this as well.

@davidatkinsondoyle
Copy link

+1 To the modification to customMerge signature. Allows users to implement multiple edge cases.

@TheHaff
Copy link

TheHaff commented Oct 5, 2020

+1 here need some way of handling this

@TehShrike
Copy link
Owner

My previous comment stands – I'm open to PRs to make the necessary changes to customMerge to enable this.

TheHaff pushed a commit to TheHaff/deepmerge that referenced this pull request Oct 6, 2020
TheHaff pushed a commit to TheHaff/deepmerge that referenced this pull request Oct 6, 2020
TheHaff pushed a commit to TheHaff/deepmerge that referenced this pull request Oct 6, 2020
TheHaff pushed a commit to TheHaff/deepmerge that referenced this pull request Jan 27, 2021
… or empty string overwrites + a sweet little util function customMergeIgnoreEmptyValues for cuteness
TheHaff pushed a commit to TheHaff/deepmerge that referenced this pull request Jan 27, 2021
… or empty string overwrites + a sweet little util function customMergeIgnoreEmptyValues for cuteness
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

Successfully merging this pull request may close these issues.

7 participants