Cache key could refer previous value

[DevSDK]

Summary

The cache key is referred to the previous value when the shape of references in the cache key is the same.

Link to reproduction

https://jsfiddle.net/z41pyotn/

Minimum reproduction cases represent proxy in array proxy.

Here's my debugging context that may help:

It seems to be no cache invalidation when the 'remove' operation for the array.

In minimum repro. case, at line 40, the proxy and value are cached that key as a proxy of 'a'.

When handle() function is called, the array will empty. After that, the values are updated (not proxy reference.) and pushed again to a.

The result of the 'a' still refers to the same cache that has been made by 40 lines. Because it refers to the same shape of references that is used as the cache key.

Check List

Please do not ask questions in issues.

• I've already opened a discussion before opening this issue, or already discussed in other media.

Please include a minimal reproduction.

• I've added a link to a typescript playground or codesandbox with a minimal reproduction.

[dai-shi]

Thanks for reporting.
If I understand it correctly, this is a limitation with the current design choice.
It would be nice if we can unlimit it in v2: #703

(edit)
On second thought, I'm not sure if it's a limitation only in v1. Even in v2, this limitation may persist.

[dai-shi]

Can this be a workaround for now?

  window.handle = function handle() {
    const temp = a[0]
-   a.splice(0, 1)
    temp.nested.nested2.a = 'Banana'
+   a.splice(0, 1)
    a.push(temp)
    console.log(Object.is(temp, a[0]))
  }

[DevSDK]

Sure. It worked correctly.
That's the way we avoid this issue for now on our product.

[DevSDK]

It would be nice to resolve this issue as it can cause confusion and make debugging more challenging. Currently, the Redux DevTools are not notified when the state changes, which can lead to misunderstandings.

Additionally, relying on console.log for debugging further complicates the situation because proxies are updated correctly, but the snapshot is not.

[dai-shi]

I don't think it's resolvable without breaking something else. Anyone can give a try if interested.

Currently, the Redux DevTools are not notified when the state changes, which can lead to misunderstandings.

If you detach an object, there's no way to notify. So, people could know that they are doing something wrong. They should change the object before detaching.

I think we should describe this limitation in the docs: https://github.com/pmndrs/valtio/blob/main/docs/how-tos/some-gotchas.mdx
Would you like to open a PR?

[Juyeong-Byeon]

I think it is about a version of removed item.
Can't we update proxy version of item when item is removed from array as a simple solution?

[dai-shi]

@Juyeong-Byeon Don't we update proxy version when removing? Feel free to open a draft PR.

@DevSDK It would be nice if you can add a https://codesandbox.io/s/react-new reproduction in the description of this issue.

[DevSDK]

Sure. I've opened very draft PR #721 to add docs.
I'll fill the PR description soon

[dai-shi]

oops, my request was to just add a codesandbox link here from your jsfiddle one.

hoping @Juyeong-Byeon has an idea.

Otherwise, let's add it to docs #721.

[Juyeong-Byeon]

I need some time to look into it.
I think it is better to merge #721 first and edit later.

[Juyeong-Byeon]

@dai-shi FYI: I found change that caused this issue
#712 is an issue that has occurred since version 1.8.0 caused by ec44557 or #599

[dai-shi]

@Juyeong-Byeon
Thanks for your investigation!
ec44557 seems changing the cache key from receiver to target. Is it causing the issue?

[serdaryesilmurat]

This seems to be a relatively new issue, but since there is no update over a month, let me lit the fire again 😅
Will there be a fix anytime soon?

We also face the same issue in our project. We have a list of objects that are rendered in an antd Table component. Whenever we update the list in the store (replace an item in the array), Table component is not re-rendered.

// Not working
storeObj.nested.list.splice(index, 1, newObj); // Insert new item directly to the original array in store

// Workaround
storeObj.nested.list.splice(index, 1); // First delete
storeObj.nested.list.splice(index, 0, newObj); // Then insert the new item

[dai-shi]

// Not working
storeObj.nested.list.splice(index, 1, newObj); // Insert new item directly to the original array in store

I wonder if it's the same issue as this. Maybe @Juyeong-Byeon can confirm.

I thought and still think the original issue might not be fixable. But, splice(index, 1, newObj) should be fixed.

[Juyeong-Byeon]

I'll take a look on the weekend.
In a day or three.

[Juyeong-Byeon]

@serdaryesilmurat Can you describe more about your situation or share reproducible code? 🙂
I tried to reproduce the error, based on your description about the case, but I couldn't reproduce it.

I tried bellow:
https://codesandbox.io/s/valtio-examples-forked-tml257?file=/src/App.tsx

[serdaryesilmurat]

Sorry for the late response.
We are using the list in the store as the datasource of an antd table.
Not sure but the error seems to be in antd.

In the sample code below, you can reproduce the error by pressing "update first" after adding new rows. "update last" always updates the table, but "update first" does not.

https://codesandbox.io/s/valtio-examples-forked-hts2t9

[LiamMartens]

@serdaryesilmurat it looks like the problem you're having is specific to antd's Table component.
I did some digging and it works when rendering a raw table; it looks like this is because the way the ant table accesses the properties doesn't register their usage in the proxy so it doesn't end up tracking the keys and thus never sees a change.

I would have to dig into the Ant source code but it's likely they are making copies of the objects somehow.

One way you could resolve it is to manually access those keys beforehand ie:
https://codesandbox.io/p/sandbox/valtio-examples-forked-y2cmck?file=%2Fsrc%2FApp.tsx

I guess for any similar bugs the issue is likely to be key tracking especially when using it with third party libraries since there's no guarantee they will use the proxy objects as-is or whether they will be cloning them for their own usage which will always break the proxy logic.