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

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

Open
cworf opened this issue Dec 13, 2019 · 1 comment
Open

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

cworf opened this issue Dec 13, 2019 · 1 comment

Comments

@cworf
Copy link

cworf commented Dec 13, 2019

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
Copy link
Owner

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.

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

2 participants