Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Effect don't work when use reactive to proxy a Map which has reactive object as member's key. #919

Closed
Gkxie opened this issue Apr 4, 2020 · 4 comments
Labels
🐞 bug Something isn't working

Comments

@Gkxie
Copy link

Gkxie commented Apr 4, 2020

Version

3.0.0-alpha.11

Reproduction link

https://jsfiddle.net/xieyu/psavz576/51/

Steps to reproduce

With devtools opened, it obvious that the 'trigger' should be printed twice, however it just printed once.

What is expected?

The function wrap by effcet should rerun.

What is actually happening?

It lose responsiveness.

@LinusBorg
Copy link
Member

LinusBorg commented Apr 4, 2020

We only track the raw key here (line 30):

https://github.com/vuejs/vue-next/blob/7402951d945b4e49474661594992a95f878de3f0/packages/reactivity/src/collectionHandlers.ts#L23-L37

and then in the deleteEntry handler, we check first forthe key as-is (which means here: being a proxy), and if that's not found, a possible raw version of the key.

Since in this instance, they key is a proxy, we call trigger() for that proxy key (see lines 84, 93), which isn't being tracked as explained above:

https://github.com/vuejs/vue-next/blob/7402951d945b4e49474661594992a95f878de3f0/packages/reactivity/src/collectionHandlers.ts#L81-L96

I'm not sure what the "correct" behaviour would be, though. should we always trigger() for the raw key?

@LinusBorg LinusBorg added the 🐞 bug Something isn't working label Apr 4, 2020
@yyx990803
Copy link
Member

yyx990803 commented Apr 4, 2020

Why would you even add both the original and the observed version of the same object as keys in the same map?

The basic rule of thumb when using v3's reactivity system is if you intend to make something reactive, create the reactive version directly and work only with the reactive version. Forget about the original.

  • You should not keep a reference to originObj, always work with observedObj.
  • You should not first mutate originMap and then observe it. Create the observed map directly and then work with it.

If you somehow ran into this situation when writing normal component code, then it would better to investigate how this happened in the original scenario.

@yyx990803
Copy link
Member

yyx990803 commented Apr 4, 2020

The expected behavior is fixed, but you should still avoid having both the raw and observed versions of the same object as keys in the same map. It can lead to rather ambiguous behavior and should definitely be avoided. (a warning is added for such cases)

@Gkxie
Copy link
Author

Gkxie commented Apr 4, 2020

Why would you even add both the original and the observed version of the same object as keys in the same map?

The basic rule of thumb when using v3's reactivity system is if you intend to make something reactive, create the reactive version directly and work only with the reactive version. Forget about the original.

  • You should not keep a reference to originObj, always work with observedObj.
  • You should not first mutate originMap and then observe it. Create the observed map directly and then work with it.

If you somehow ran into this situation when writing normal component code, then it would better to investigate how this happened in the original scenario.

Thanks for your reply, and I agree with you. It is a better way to add warning to avoid this.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants