Passthrough non-object parameters

[cinderblock]

If this function is called with a non Object parameter, return it right away instead of letting WeakMap fail

[cinderblock]

Thinking about this more...

Maybe the value should be passed to the cb function? This however does not match the mapArray function which does not call the cb on non object elements. So think this makes behavior consistent.

We could even simplify mapArray now...

[sindresorhus]

It should not just silently pass through non-Object values. It should throw a TypeError. You also need to use a different object check as the one already there is focused on a different use-case. typeof x === 'object' && x !== null is all you need for this type-checking.

[cinderblock]

Hey @sindresorhus. Thanks for taking a look! 😄 I happen to disagree with your reasoning:

With regard to throwing a TypeError, I can understand that logic. However with the recursive nature of this package, I think "recusing" "0" levels is a valid usage. If you want to keep the API the way you have it, that's your prerogative. I just think it's not very useful to throw when an invalid type is passed in if there is a sensible thing to do (just return it).

With regard to using your isObject function, your package has already had the decision to not mangle Regex, Date, or Error objects when doing recursion. Why is the top level special?

[sindresorhus]

I just think it's not very useful to throw when an invalid type is passed in if there is a sensible thing to do (just return it).

I prefer strict typing. Imagine if this package was using TypeScript, it would be typed to accept an object not any. Instead, we do the check at runtime. This helps catch mistakes in your code.

With regard to using your isObject function, your package has already had the decision to not mangle Regex, Date, or Error objects when doing recursion. Why is the top level special?

I honestly don't remember why I chose to exclude RegExp, Date, Error. There are many more built-ins that could be added to that list. In hindsight, it would probably have been better to have made the check like this:

const isObject = value => value !== null && (typeof type === 'object' || typeof type === 'function');

[cinderblock]

Except map-obj also support Array arguments explicitly. Those are clearly not objects...

My main point is that the recursion logic is inconsistent. If the argument is an array, it recurses. If any of those arguments are not objects, it doesn't convert them or thow. Why not throw a TypeError when an array is passed in? Why not throw a TypeError when an array of non-objects is passed in? Recursing a depth of "0" is just as valid as any other depth.

Throwing makes sense when there is no logical way to move forward. For instance, if the function is expecting an argument with a specific shape and is supposed to convert it to some other value. Maybe a function call like length({x:3, y: 4}) == 5... If x or y are missing, clearly that's worth throwing an error for... Of course this logic could apply to obj-map as it also expects a certain "shape"...

Maybe a better analogy is a function like makePositive(x). Should that throw if x is already positive?

---

You bring up a good point about functions. They do act a lot like objects. However changing that now would be a more significant breaking change as currently functions are not recursed.

[cinderblock]

Would you consider passing through non-object parameters if options.deep === true? Or is that still too defensive?