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

fix: populate useState reactively in devtools #663

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hichem-dahi
Copy link

fixes #643

@hichem-dahi hichem-dahi changed the title fix: popluate useState reactively in devtools fix: populuate useState reactively in devtools May 16, 2024
@hichem-dahi hichem-dahi changed the title fix: populuate useState reactively in devtools fix: populate useState reactively in devtools May 16, 2024
@danielroe danielroe requested a review from antfu May 22, 2024 15:27
@kleinpetr
Copy link

any update here?

@antfu
Copy link
Member

antfu commented Jun 21, 2024

I don't think we should clone the data and update the clone eagerly - it would be very inefficient and could cost extra memory. The root cause is that we didn't properly trigger the data when they updates, but also because we are dealing with multiple Vue instances (one in host, one in devtools, where watching on a forgien reactive object won't triggers properly. I think this would need a fix with another approach

@hichem-dahi
Copy link
Author

hichem-dahi commented Jun 23, 2024

@antfu, I've made it possible to update only the necessary nested properties using the deepSync function. Can you check?

@@ -11,7 +13,7 @@ definePageMeta({
})

const client = useClient()
const payload = computed(() => client.value?.nuxt.payload)
const payload = computed<NuxtPayload>(() => JSON.parse(JSON.stringify(client.value?.nuxt.payload)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think still, I would avoid this cloning, as it

  1. duplicate the object, doubles the memory usage (could even crash when payload is too big)
  2. doesn't handle the circular references
  3. losing the details when serializing a class or special objects
  4. it not always reflecting to the real data, sometimes could cause confusing on debugging

@hichem-dahi
Copy link
Author

hichem-dahi commented Jun 26, 2024

@antfu I've removed cloning. I've used toRefs(payload.state) to reference the objects. It seems to be working.

@antfu
Copy link
Member

antfu commented Jun 30, 2024

Looks great; I will test it a bit. Thanks a lot for being patient on this!

@antfu
Copy link
Member

antfu commented Jul 2, 2024

I looked into it, but it seems that using toRef in template isn't a good practice. I end up with a different approach: 418a22e, which is released as v1.3.9. Please give it a try and let me know if it solves your cases. Thanks!

@hichem-dahi
Copy link
Author

I left a comment here.
It solves the initial issue. However, when modifying from the StateEditor, the change isn't synced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: reactivity of useState is not populated properly
3 participants