Don't merge if value is an empty string

[TidyIQ]

I need to merge some data that originates from a .yaml file and is converted to a JS object using GraphQL (I'm using Gatsby with Netlify CMS if you need a real world use case).

As you may know, yaml does not allow undefined values. The closest alternative is to set the value to null or an empty string ('').

For example, let's say I have the following GraphQL query:

query MyQuery {
  foo {
    one
    two
    three
  }
}

Each field (one, two and three) are optional fields.

For example, if only two field has a value then the yaml file will be:

foo:
  two: Some text

If I attempt to run the query on that data, GraphQL will throw an error as fields one and three don't exist.

The best I can do is to set a default value for all fields as '' (an empty string). I can't set it to null due to an issue where users who change the value from the default null value, then delete the value, will save the value as an empty string.

For example, if only the two field has a value then the yaml file will be:

foo:
  one: ''
  two: New text
  three: ''

And after running the GraphQL query, it will return the following object:

{
  foo: {
    one: "",
    two: "New text",
    three: ""
  }
}

If I wanted to merge it with the following object:

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

Then run deepmerge() on the two objects, it will simply return the first object:

{
  foo: {
    one: "",
    two: "New text",
    three: ""
  }
}

Having an option in deepmerge to ignore empty strings would be a lifesaver for this, for example:

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

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

const mergedObject = deepmerge(newObject, defaultObject, ignoreEmptyStrings);

Would result in mergedObject being equal to:

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

[TehShrike]

That is the current behavior: https://codepen.io/TehShrike/pen/Jjjrwzo

What version of deepmerge are you using? A bug with falsey values not being handled properly was recently fixed in #172

[TidyIQ]

Ah ok. I'm using 4.0 but I see 4.2 is available now. I'll try upgrading and see if it fixes it. Thanks!

[johnlife]

That is the current behavior: https://codepen.io/TehShrike/pen/Jjjrwzo

What version of deepmerge are you using? A bug with falsey values not being handled properly was recently fixed in #172

In your codepen with 4.2.2 version neither deepmerge(newObject, defaultObject) nor deepmerge(defaultObject, newObject) produce

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

Why is this closed? Can we have an option to specify values to treat as non-existent?

[TehShrike]

I see. You had the arguments backwards in your original post – it sounds like you want deepmerge(defaultObject, newObject).

merge(x, y, [options])
If an element at the same key is present for both x and y, the value from y will appear in the result.
– the readme

Unfortunately, I don't believe there's a way to twist isMergeableObject + customMerge to fit your use case, since isMergeableObject gets called on the target first, and then the target's keys get iterated over before the custom merge function gets called.

There have been some other edge case requests like this that have come up. I'm tempted to alter the customMerge function signature so that the first function would get passed both values as well as just the key, so you could do any wacky pre-merge logic in that function as well. That would allow you to do this (which isn't currently possible, but I think would solve your use case):

const mergedObject = deepmerge(defaultObject, newObject, {
  customMerge(key, a, b) {
    if (a === '') return () => b
  },
})

[johnlife]

Nah, it' not my original post, I'm just fighting with the same issue now. For me API returns nulls and undefineds, that should not replace the default values during merge.

This can be fixed with replacing

  if (propertyIsOnObject(target, key) && options.isMergeableObject(source[key])) {
    destination[key] = getMergeFunction(key, options)(target[key], source[key], options)
  } else {

with

  if (propertyIsOnObject(target, key)) {
    if (options.isEmpty && options.isEmpty(source[key])) {
      destination[key] = cloneUnlessOtherwiseSpecified(target[key], options)
    } else if (options.isMergeableObject(source[key])) {
      destination[key] = getMergeFunction(key, options)(target[key], source[key], options)
    } else {
	destination[key] = cloneUnlessOtherwiseSpecified(source[key], options)
    }
  } else {

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' 
  } 
}

PR Created:
#185

[johnlife]

const mergedObject = deepmerge(defaultObject, newObject, {
  customMerge(key, a, b) {
    if (a === '') return () => b
  },
})

I think this requires deep understanding of an internal code of a library and I'd go with more human-readable options, but you're maintainer.

But shouldn't your code be

const mergedObject = deepmerge(defaultObject, newObject, {
  customMerge(key, a, b) {
    if (b === '' || b === null || b === undefined) return () => a
  },
})

Or is it somewhere swapped the order of parameters?

[n0v1]

I'd favor the more generalized customMerge way to solve this because in my case I'd only want to ignore empty strings and undefined but keep null values. Others may have other requirements and customMerge(key, a, b) would solve all of them.

If this is documented in the readme with examples like posted in this issue, this shouldn't be too hard to use.