proxyMap subscription ops don't include keys

[coracuity]

Bug Description

Operations passed to the subscribe handler for proxyMap don't include the key in the path, but instead include internal implementation details.

Setting a value shows up like this:

// px.bar.set("key1", "val1")
["set", ["bar", "index"], 1, 0]
["set",["bar", "data", "0"], "val1", null]

While I would expect it to show up like:

// px.bar.set("key1", "val1")
["set", ["bar", "key1"], "val1", undefined]

Am I using this wrong? Or is this working as intended? (I also snuck in undefined vs null in that example, but that might be more controversial)

Reproduction Link

https://codepen.io/coracuity/pen/rNXwMMz

[coracuity]

Oh, shoot, wait, this is working as intended? #514

[dai-shi]

Just to confirm, is this a new behavior changed in v2.1.0? Or, is it the same with v2.0.0?

[coracuity]

Given that #514 is from 2022, I suspect it's a long-standing issue

[dai-shi]

thanks for confirming.
unfortunately, as you noticed, it's working as intended and we have no plans to change.
#973 made it explicit that this capability is for internal use.

we are happy to discuss some workarounds. can you explain your overall goal?

[coracuity]

The goal was to use this for state management for a dataviz and to be able to subscribe to specific changes so that I can incrementally modify a visualization derived from that state. This data structure would be the medium to communicate state changes. I can still use subscribe to tell that something has changed but without ops I have to do the diffing of the data structures myself.

[dai-shi]

So, the use of Map is unavoidable because it comes from outside, or something like that?

I have to do the diffing of the data structures myself.

That sounds like a solid approach. Or, if you have a control, you could create a map to lookup a key with a value.

[overthemike]

To dai-shi's point, if you know the shape of the data and the keys being used are primitive values, using a plain object here would solve your issue (and would be a whole lot faster). If that's not the case, one possible solution would be to use .entries() on foo.bar to find the value and the corresponding key. This won't work, however, if your map has duplicate values.

[overthemike]

This is actually a side effect of how we get reactive behavior working with proxyMap, I believe. The main reason we have to create an emulation for Maps is that they store their data in internal slots and changes to the data are not visible outside of the map, which is necessary for valtio to be reactive. To overcome this, we store the data in a data property that valtio can view. I'm not sure if this is something that can be fixed easily due to the limitation of the design. I do have a few ideas but it would require changes to both subscribe and proxyMap.

[coracuity]

if there were a way to take that number key and convert it to the key in the map, that would probably be sufficient as an interim fix

[overthemike]

Propositional fix:
proxyMap (and proxySet) could use unstable_replaceInternalFunction to replace createHandlerDefault and conditionally override notifyUpdate to send the key instead of the data and it's index. Not sure what effect this would have on any other listeners, however. Should probably rename data to something like $$VALTIO_INTERNALS_data and index to $$VALTIO_INTERNALS_index to prevent any accidents. I'm not sure if there is a way to prevent the index itself being sent without possibly breaking .has() calls.

[overthemike]

I'll play with this a bit and report back any findings.

[dai-shi]

Should probably rename data to something

I have thought about it, and if the name collision is an issue, a symbol should work.

proxyMap (and proxySet) could use unstable_replaceInternalFunction to replace createHandlerDefault and conditionally override notifyUpdate to send the key instead of the data and it's index.

I'm not a big fan of it. unstable_ things are really unstable and we can't guarantee to provide the same capability in the future.

[overthemike]

Agreed. I haven't found a reliable way to make this work yet regardless.