deepmerge icon indicating copy to clipboard operation
deepmerge copied to clipboard

#185 change customMerge signature for merging empty values

Open TheHaff opened this issue 4 years ago • 4 comments

@TehShrike original pr: https://github.com/TehShrike/deepmerge/pull/205 My rebase got in a weird state so i just redid it and cleaned it up a bit

TheHaff avatar Jan 27 '21 15:01 TheHaff

Should this not be

deepmerge.customMergeIgnoreEmptyValues = (key, target, source) => !target || target === ''
	? () => source
	: deepmerge;

otherwise the you only get a shallow copy? Though you could always supply your own function I guess.

timberkeley avatar Feb 09 '21 11:02 timberkeley

Should this not be

deepmerge.customMergeIgnoreEmptyValues = (key, target, source) => !target || target === ''
	? () => source
	: deepmerge;

otherwise the you only get a shallow copy? Though you could always supply your own function I guess.

No it's meant to be used as customMerge option. if you look at the test i provided on test/merge.js line: 680. Maybe i should have named it differently? Also i might be misunderstanding what you mean.

TheHaff avatar Feb 10 '21 10:02 TheHaff

No it's meant to be used as customMerge option. if you look at the test i provided on test/merge.js line: 680. Maybe i should have named it differently? Also i might be misunderstanding what you mean.

If you change the test a little:

src = { someNewVariable: { key: "herp"}, very: { nested: { thing: "", another: "derp" } } };
target = { very: { nested: { thing: "derp", another: "" } } };

And run the test with the default merge you will get the overwritten thing : { very: { nested: { thing: '', another: 'derp' } }, someNewVariable: { key: 'herp' } }

With the customMergeIgnoreEmptyValues merge function you get: { very: { nested: { thing: 'derp', another: '' } }, someNewVariable: { key: 'herp' } } What I want to achieve is: { very: { nested: { thing: 'derp', another: 'derp' } }, someNewVariable: { key: 'herp' } }

It does not look trivial to implement but is also what I expected.

timberkeley avatar Feb 10 '21 16:02 timberkeley

No it's meant to be used as customMerge option. if you look at the test i provided on test/merge.js line: 680. Maybe i should have named it differently? Also i might be misunderstanding what you mean.

If you change the test a little:

src = { someNewVariable: { key: "herp"}, very: { nested: { thing: "", another: "derp" } } };
target = { very: { nested: { thing: "derp", another: "" } } };

And run the test with the default merge you will get the overwritten thing : { very: { nested: { thing: '', another: 'derp' } }, someNewVariable: { key: 'herp' } }

With the customMergeIgnoreEmptyValues merge function you get: { very: { nested: { thing: 'derp', another: '' } }, someNewVariable: { key: 'herp' } } What I want to achieve is: { very: { nested: { thing: 'derp', another: 'derp' } }, someNewVariable: { key: 'herp' } }

It does not look trivial to implement but is also what I expected.

oh crap that's what i want as well. Let me digg a little more.

TheHaff avatar Feb 12 '21 09:02 TheHaff