Don't pass mutable keyPath arrays to callback functions while retaining them in our state

[Phanabani]

Callback functions like getStyle, readOnly, etc. pass a mutable array keyPath, which is also retained in REJT code. Users can mutate this array and disrupt REJT's internal state. We should clone these data structures before passing them to callbacks.

Options:

• Homemade clone function
• Lodash
• Immer

I think immer is overkill, and implementing a homemade clone isn't really worth it, so I think lodash is an easy and reliable solution.

[oxyno-zeta]

Hello @Phanabani ,

This was intentional when I've created the library. Making a deep copy can be heavy (depends on source object size) and I'm afraid that doing this will, under some circonstances, create a performance drop...
Do you think this is needed ? Don't you think this is an edge case ?

Regards,

Oxyno-zeta

[Phanabani]

@oxyno-zeta

Oh, I didn't expect that it would be a big performance issue. I suppose you're probably right though, cloning the same array for every node, multiple times (for each callback)...

It's definitely an edge case, but I made the issue because I had hit this edge case myself in my own code. 😅 Admittedly, I wrote the code when I was just beginning to learn programming, but it still confused me a lot when keyPath.push(key) somehow broke everything. I didn't realize mutating the array in place would be bad. Of course I ended up just using keyPath = keyPath.concat(key).

What about using Object.freeze(keyPath) instead to help prevent mutations and give more useful error output to users?

[oxyno-zeta]

That can be a good compromise yes. Seems to be a good idea !

Ahah no problem for the beginning. It is normal and you aren't the only one ahah.

Oxyno-zeta