TehShrike / deepmerge

A library for deep (recursive) merging of Javascript objects
MIT License
2.75k stars 216 forks source link

Pass "key" into "isMergeableObject" function? #184

Open cworf opened 4 years ago

cworf commented 4 years ago

Great piece of code! Its been a great help.

Im running into a difficult issue to solve because I need to create a custom merge for a single key "quantity" where the value is just a number, such that each time the objects are merged, the quantity values are added.

ive tried this

const sumQuantities = (q1, q2) => {
    return q1 + q2
}
options = {
    isMergeableObject: (value) => value && (typeof value === 'object' || typeof value === 'number'),
    customMerge: (key) => {

        if (key === 'quantity') {

            return sumQuantities
        } 
    }
}

Which works to add SOME of the quantities (which are placed in multiple levels of the object), however it has been a nightmare trying to get it to ignore all other number values for some reason. Either way its a major performance issue to convert all number values in these large objects over to mergeable objects only to ignore them inside the customMerge function.

Is there a clever way to accomplish this I am not seeing? The best solution I can think of would be if I could access the keys within the isMergeableObject function like this

const sumQuantities = (q1, q2) => {
    return q1 + q2
}
options = {
    isMergeableObject: (key, value) => value && (typeof value === 'object' || key === 'quantity'),
    customMerge: (key) => {
        if (key === 'quantity') {
            return sumQuantities
        } 
    }
}
TehShrike commented 4 years ago

hmm, that ask makes sense. I think you can do that with the current functions:

const numberCapableMerge = (target, source, options) => {
    if (typeof source === 'number') {
        return source
    } else {
        return require('deepmerge')(target, source, options)
    }
}

const sumQuantities = (q1, q2) => q1 + q2

const options = {
    isMergeableObject: value => require('is-mergeable-object')(value) || typeof value === 'number',
    customMerge(key) {
        if (key === 'quantity') {
            return sumQuantities
        } else {
            return numberCapableMerge
        }
    }
}

export default numberCapableMerge

It's a little awkward, but seems less scary to me than making isMergeableObject aware of keys or making customMerge aware of values, which seems conceptually fraught.

Saying "this merge function handles numbers" and then making a custom merge that says "numbers are merged by just taking the value from the source object, unless we're talking about quantity" feels reasonable.