Apply default merging if the custom merger returns undefined.

[RebeccaStevens]

Would it be possible to make the custom merge code a bit shorter by not having to have to code the default merging?

So that writing only return; would be equal to return utils.defaultMergeFunctions.mergeArrays(values);. Meaning that the lib would apply default merging if the custom merger returns undefined. It would make the custom code shorter when not explicitly having to apply the default behavior.

Of course we would then also need a way to be able to tell the library explicitly that we want to use undefined as the result of the custom merge function. I guess we could do it with a sambol e.g. return utils.useUndefined; where useUndefined is a symbol coming from the deepmerge lib, so that the lib code could check if the return value of the call of the custom merger is this symbol and then use undefined as result of the custom merging.

Or the custom merger could receive a callback (used like utils.useUndefined()) in order to tell the lib that undefined should be used.

Originally posted by @NightProgramming in #23 (comment)

[RebeccaStevens]

@NightProgramming

I put quite a bit of thought into this when I first added support for custom merging. I decided to require that users must be explicit in what the value should be.

Having to hack in a way for users to be able to actually use undefined if that is what they want, didn't seem like the best way to do things. An alternative would be to not allow undefined at all but then the library would feel incomplete.

Another reason is that if a merger function explicitly returns undefined then it should be expected that undefined is the value at that point.
I'm also a big believer in requiring that functions do not contain both explicit and implicit return types (this behavior can be enforced with the ESLint rule consistent-return). So I wouldn't be a fan of allowing no return value at the end of the merger function.

[NightProgramming]

@RebeccaStevens Would it be possible to have an easy way to get the appropriate default merger? Something like return utils.defaultMerger(values). In context of mergeOthers utils.defaultMergeFunctions.mergeOthers would be used. In context of mergeArrays utils.defaultMergeFunctions.mergeArrays would be used when calling utils.defaultMerger.

[RebeccaStevens]

That is possible but it would add in a small amount of overhead as each merger would need its own version of utils. As of right now, all mergers are just passed the same object.

[RebeccaStevens]

@NightProgramming

I've put some more thought into this and realized that implicit default merging could quite easily be allowed for via an option. #59

[NightProgramming]

@RebeccaStevens Generally I like the idea of having options on the object, that is passed to deepmergeCustom and having those options affect all custom mergers (mergeOthers, mergeArrays,...). In this case though I find it a bit verbose because people who want the less code approach (having no return statement indicates using the default merger) probably also don't want to configure allowImplicitDefaultMerging. They might rather expect this to be the default behavior. But I understand that it's too much of a breaking change having an undefined return value (without any additional new options) as indicator to apply default merging.

Your approach with allowImplicitDefaultMerging and implicitDefaultMergingValue is flexible because it supports being lazy (not having to code a return stament) and also being precise (defining something like a symbol in order to indicate default merging). But maybe it's a bit of an overkill to support those two paradigms?

My suggestion would be not having a implicitDefaultMergingValue but a couple of predifined (= coming from the library) Symbols that can be used as return value in order to indicate some kind of merging.

return utils.defaultMergeIndicator; // this is simply a shorthand Symbol for 'return utils.defaultMergeFunctions.mergeArrays(values);' when we are inside the contect of mergeArrays.

return utils.priorityMergeIndicator; // Would indicate merging like in discussion #56, if ever supported out-of-the-box by deepmerge

So with the above approach there would be some flexibility (via different shorthands), and also there would be no configuration required. Because if deepmerge ever wants to support different kinds of default merging, then the available options would become more complicated (e.g. implicitPriorityMeringValue besides implicitDefaultMergingValue and so on).

And if it's still a requirement to have as little code as possible (meaning no return value indicates default merging behavior), then I would make this the default (no configuration required), but like I said, it's probably too much of a breaking change. And of course then there would need to be an option like undefinedIndicatesDefaultMerging which the client can set to false (default is true) in order to enable using undefined as the custom merge result (true result, no indicator), but in my opinion it would be clearer (fewer paradigm mixes) when just having predefined indicator Symbols, that can be returned and act as shorthands.

Also I don't quite see the benefit of being able to specify implicitDefaultMergingValue as client compared to having deepmerge offering such symbols.

[RebeccaStevens]

Thanks for the feedback.

I've reworked the PR, removing implicitDefaultMergingValue and instead defining the value internally. It's now available via utils.use.defaultMerging. I also broke out the breaking changes into their own PR (#61).
I don't want to change the default behavior to be undefined => default merging, so I've kept allowImplicitDefaultMerging around (this value doesn't have to be set to use utils.use.defaultMerging).