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

Cache EntityStore storeObjectReconciler @wry/equality erroring on Proxy values #10510

Open
Tmktahu opened this issue Feb 3, 2023 · 7 comments

Comments

@Tmktahu
Copy link

Tmktahu commented Feb 3, 2023

Issue Description

Inside EntityStore within the storeObjectReconciler defined at the bottom of the file, it uses the @wry/equality package to determine if the existingValue and incomingValue are equal or not.

In some cases, one or both of existingValue and incomingValue are wrapped in Proxies. If the wrapped value is an object with a nested array, the equal method from the @wry/equality package hard errors on this specific line with the following error:

TypeError: 'get' on proxy: property 'nestedArrayPropertyName' is a read-only and non-configurable data property on the proxy target but the proxy did not return its actual value (expected '[object Array]' but got '[object Array]')

I need to have this fixed ASAP in my project. My current plan is to try and override the @wry/equality package and fix it locally, but suggestions for possible solutions would be fantastic.

I have opened an issue on the wryware repository as well, but it hasn't been updated in 6 months. Given it may not receive a fix, I figured posting an issue here as well was warranted.

Link to Reproduction

N/A

Reproduction Steps

Due to upcoming deadlines, I do not currently have the time to create a reproduction project to showcase this problem. It's also entirely possible that it is an issue specific to my environment.

@bignimbus
Copy link
Contributor

Hi @Tmktahu 👋🏻

As an unblocker, you might try wrapping the body of isDefinedKey in wryware with a try/catch that returns false on exceptions.

I know you're on a time crunch. When you get a moment it would be helpful as a demonstration of the problem to have something runnable for the maintainers to play with so we can consider this case in the most concrete terms possible. For transparency, we definitely want to think through this a bit so I don't know exactly when we'll have something tangible to share. But thanks a bunch for opening this issue!

@bignimbus bignimbus added 🥀 needs-reproduction and removed 🏓 awaiting-team-response requires input from the apollo team labels Feb 9, 2023
@Tmktahu
Copy link
Author

Tmktahu commented Feb 11, 2023

I was able to stem the issue by overriding the wryware package and making a local copy that utilizes Vue's unref function to recursively remove Proxies from objects before they are compared for equality.

Though, this doesn't seem to be an issue specific to wryware. I encountered the same bug with immer... though they had a flag to prevent freezing objects that fixed it on their side. Then I encountered a similar bug with window.sendMessage where it doesn't like any Proxies in the payload.

The underlying issue seems to be Proxies getting mixed up inside the cache data. Sometimes results pulled from the cache are wrapped in Proxies and sometimes they aren't. And the influx in Proxies is causing these syntax errors.

To be honest I'm not even sure if this is an Apollo specific problem. This started happening as I was upgrading the whole app from Vue 2.7 to Vue 3. If Vue 3 is relying more on Proxies then it's possible they are bleeding into the cache somehow beyond my own code.

I'll attempt to get a repro working if possible, but most of these problems occur with regards to subscriptions and websockets... which I'm not sure how to reproduce in a condensed environment.

I could try to make a reproduction that showcases the Proxies getting into the cache, since that seems to be the underlying issue?

@bignimbus
Copy link
Contributor

That's all great info, thanks @Tmktahu. For the record, I was surprised to hear about Proxy objects getting into the cache. I'm not a Vue user, though, so my surprise at Proxies in userspace probably isn't surprising 😛 A reproduction would be interesting and helpful, though, if you're able to make one. I'm glad you're unblocked and am looking forward to learning more.

@kylekz
Copy link

kylekz commented Nov 16, 2023

@Tmktahu Were you able to find a definitive fix for this? Running into it now. Wrapping isDefinedKey in a try-catch fixes it in one instance but not another instance. Performing a deep unref results in max callstack size errors.

@phryneas
Copy link
Member

At this point we still don't know how you get Proxies into your store in the first place (Apollo Client doesn't use Proxies!), so if you could provide us with an example why this is happening for you in the first place, we could try to help you figure this out.

@Tmktahu
Copy link
Author

Tmktahu commented Nov 16, 2023

While I don't have a reproduction, I'm fairly certain that it's due to Vue 3 using Proxies for its reactivity. If anything gets touched by reactivity, it tends to be wrapped in a Proxy. Then that Proxy gets saved and ends up causing issues.

I still deal with this issue on a regular basis unfortunately. Most commonly it rears its head with structuredClone or postMessage.

@kylewardnz The most reliable method I have of dealing with it is prefacing such actions with a deepunref from https://github.com/DanHulton/vue-deepunref which hasn't broken for me yet.

Unfortunately making a reproduction is beyond me at the moment. I'm not familiar enough with the entire tech stack to make a reproduction from scratch in a reasonable amount of time.

@kylekz
Copy link

kylekz commented Nov 17, 2023

I tried making a repro as well as couldn't get it to trigger. The closest I've gotten is:

  1. Perform a mutation to create an entity
  2. In the onDone handler (using Vue Apollo here), route to the page for the new entity
  3. Immediately after that router.push statement, trigger a refetch of a query that's used on the old page and the new page, which loads in the newly created entity as well as old data.

Both mutation and query are in the same composable/hook. I've seemingly fixed the first instance by triggering the refetch before routing away, which leads me to believe it's an issue with the query pulling from the cache. The refetch is there as previously I was using onResult hooks to push to a store, but this model falls apart due to onResult hooks retriggering whenever the query is mounted again, resulting in:

  1. Load 3 entities via query
  2. Create an entity via mutation
  3. Push new entity to store (Pinia/Vuex)
  4. Route to new page
  5. Query remounts, triggers its onResult hook, resets the store to 3 entities from step 1
  6. Created entity is now missing

I was doing this to 1) work around this Proxy error, 2) have local optimistic updates, and 3) prevent unnecessary data fetching

The second instance is kind of the same but it's mutation to delete the entity, then route away. The error is sporadic/doesn't happen every time, but I spent about 10 minutes last night trying to redo it over and over and couldn't get it to trigger again.

The structuredClone issue I run into quite a bit when using the onResult model rather than just using the Apollo cache as the store. I'll give it all another crack this weekend but at this point I'm almost sure it's a Vue Apollo issue rather than apollo/client.

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

No branches or pull requests

5 participants