mergeObject, Object.keys error in IE11

[volosovich]

Hello guys, I found next bug. We have the next function:

function mergeObject(target, source, options) {
    var destination = {}
    if (options.isMergeableObject(target)) {
        Object.keys(target).forEach(function(key) {
            destination[key] = cloneUnlessOtherwiseSpecified(target[key], options)
        })
    }
    Object.keys(source).forEach(function(key) {
        if (!options.isMergeableObject(source[key]) || !target[key]) {
            destination[key] = cloneUnlessOtherwiseSpecified(source[key], options)
        } else {
            destination[key] = deepmerge(target[key], source[key], options)
        }
    })
    return destination
}

I can send any value in source argument. When I send number, string, boolean in Google Chrome and even Edge returns empty array. But IE11 - throw exception. I think need to check type of source before use Object.keys

[volosovich]

this problem is indirectly related to https://github.com/KyleAMathews/deepmerge/issues/54

[TehShrike]

deepmerge only supports actual objects as arguments, not strings or numbers or booleans.

It doesn't specifically check what you pass in, but it doesn't explicitly support other uses either.

54 is a discussion about whether or not undefined or null should be explicitly handled.

[volosovich]

I agree with you, but all major browsers correct resolve this problem, except IE11. So a lot of developers can send the wrong argument and don't catch any problem. We have different behavior for different browsers, it's not good. For my pov - if you think that argument should be only the Object, need to implement validator for that. Because a lot of people doesn't think about what they sent in source argument, and all work fine. Only if they catch an error in IE they start checking and spent some time for that. And my last pov. For my opinion, in this case, need to do as for undefined value. When you send not Object - throw an error. Thanks for lib, you do good job.

[volosovich]

I can prepare PR if you this that validator it's a good idea.

[TehShrike]

I'd rather not add type-checking assertions to this smallish codebase - beyond adding a few more lines of code, it would be a breaking change, since there could be codebases passing in strings or whatever as the first argument, and it could be fine for them as long as they're not shipping to IE11.

I would rather focus on updating the typescript definition to be more correct/restrictive.

[TehShrike]

TypeScript types are published with deepmerge as of 2.2.0. #119