mapValuesDeep array issue

[GaborTorma]

Hi

I found some interesting things in mapValuesDeep funtcion (and maybe on the other map function...): If a change an array value to the object, the array values remaining in the object with index key.

You can check it on my pen: https://codepen.io/gabortorma/pen/zYvgXaG

Thanx! Gábor

[YuriGor]

Thank you, will check

[YuriGor]

Hi, could you please make sure you are not processing children of this array you changed to be an object?

As per docs, mapValuesDeep keeps the same structure, so when you are iterating over children - children values will be set to the old paths, and old path used to have 0 index, so Lodash's set method will create all the missing parents.

If you want to change structure - use mapKeysDeep, it should handle such complicated cases. (But probably i didn't test converting arrays to objects)

[YuriGor]

The more I think about this issue, the closer I am to adding support of expected by you behavior, because it really makes more sense..

[YuriGor]

Thank you for a very good example! After a bit of meditation on this, I see the easy(=fast) and a reasonable way to solve it is to do something like you tried to do:

• if the current deep value is a leaf - then no problem just use given by callback value
• if the current deep value is not a leaf, then:
  • if callback returned original value (new value === original value) - then set empty array or object to result object and go deeper for children
  • if callback returned something else (new value !== original value) - then use given value as is and skip children.
  • even worst-case should work: if callback will modify the current value and then return it, instead of creating a brand new one.

With the current implementation, we may read all the children's keys before the callback, but this will happen only if 'leavesOnly'=true.

• So if 'leavesOnly=true' then callback will never receive not-leaf value.
• And if 'leavesOnly=false' - then iteration will go over new value, no pre-cached keys in this case.
• And empty arrays/objects are considered as leaves, so callback can take some empty object, add put some nested data inside, but it's ok to not process added children, in this case, we will use returned value as is - because it was a leaf before processing.

I think all the edge cases are covered now.

[YuriGor]

Oh, we also have tree mode, when childrenPath is specified in options. So in this case we need to clone whole value from callback, except children. (Similar to filterDeep in tree mode)

[YuriGor]

No, relying on the fact of reference equality is a bad design. If the user needs immutable mapping - he has no control.

So I'll better check if returned not-leaf value is of the same type as original or not. By default if it is the same type - we should go deeper over children, otherwise - should not. Additionally, I will add 'skipChildren' method to the context. If user call this method with true or false - it will override the default behavior.

[YuriGor]

Ok, done as described in a recent comment. Your example to work as expected should look like this: https://github.com/YuriGor/deepdash/blob/ce9643332ed04b64f4b3114222554b5f998b4103/test/mapValuesDeep.test.js#L253-L261

Let me know if you will find something else or stuck with how to implement something. Currently, docs are not covering such details (I always have no time for good docs :disappointed: )