-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add an option in ApolloClient constructor to return a deep copy of query results (or a way to globally transform results after the caching step) #372
Comments
Hi @M1CK431 👋🏻 thanks for filing this issue! Without knowing more about your specific use case, this seems like a problem that can be solved via field and type policies. Here are a few links that might be helpful:
Not having used Vue extensively I can't comment on the best design pattern here. Reactive variables may also be of use as a tool to help integrate UI state changes into the cache. Let us know if that helps! |
Hi @bignimbus and thank you to take time reading this issue. From the second link you provide (I read both):
My issue is not about "a particular field". I need a way to obtain a deep copy of all queries results and so I will be able to mutate them freely yet avoid mutating the cache. I guess reactive variables are already used internally by vue-apollo to provide reactive queries (when a local variable used to build query variables change, depending of the fetch policy, the query is automatically resent to the API if the result isn't already in the cache, else the cached version is used). In fact my use case is not specific at all. This issue will impact at least all developers which use vue-apollo. To simplify the need: how can I globally configure ApolloClient to return a deep copy (aka which will not have all object properties read only) of all queries results (regardless the requested fields)? |
Thanks for the quick reply @M1CK431. I'm struggling to find the words for why it seems "off" to want to deep clone the cache in this way, besides the potential performance tax. Does https://github.com/vuejs/apollo have any solutions for what you're trying to do? Thanks again 🙏🏻 hope we can help figure something out |
As you can imagine @bignimbus I have already read carefully vue-apollo documentation (of course, I could still have miss something). The goal of vue-apollo is to integrate AC in VueJs, make a bridge, not to add feature or alter behavior of AC. As previously said, I'm not the only one concerned and every VueJs developers which use Apollo would have a benefit with this feature request. Example here: vuejs/apollo#58 (comment) Notice that this comment is from Akryum, which is part of VueJs core team and maintainer of vue-apollo library. His advice is:
So exactly what I'm requesting here but at a global level. Also, please notice that in my project I'm often using AC directly in my To conclude, about the "potential performance tax", most of the time API results are not so giant that Again, thanks a lot for the time you spent to consider this feature request and provide advises 🙏🏼 |
Thanks @M1CK431 - I'll tag this as a feature request so others in the community can find and weigh in on it 🙏🏻 |
Hi @bignimbus hope you are well :) |
My team is also facing this issue, it would be a nice feature |
my team is also facing this issue |
Since I answered in that issue that was newly opened in the Apollo Client repo, I'll copy-paste my response here, too: Hi there, we've talked about this internally and this is something we're not going to introduce to the Reintroducing the option to turn off As for the general notion of returning a modifiable copy - that would end up being something that is actively discouraged in most frameworks, as most frameworks have their own methods of tracking state changes that would be distinct from that - in React for example it's strictly forbidden to have any kind of mutable object (never do that in your Next app!). Every state change has to go through React's There is also other behaviour that would be problematic or unclear here - in the case of any kind of cache update, you'd lose all changes you made to that object locally, because you would receive a new full copy of the cached value. That would probably not be very desirable in most cases. Generally, we expose methods that allow you to modify the cache, and those changes will always propagate to your components. But even then keep in mind that new data from the network will override your cache modifications. I believe that in the case of vue-apollo, that data being stored in Lastly, something like a |
Totally agree. Forbid direct cache mutation is not the issue.
Here is how reactivity works with Vue.js: https://vuejs.org/guide/essentials/reactivity-fundamentals
Not at all! Please let developers take that decision based on there own needs. When the "new full copy of the cached value" is received, I, as a developer, will decide what to do with the data. Simple, cristal clear.
This is out of scope since we don't want to modify the cache here.
If you read carefully the Vue documentation I provide, you will understand that you don't believe correctly, sorry 😝
Sorry again, I already explain why this is not desirable. After carefully reading your answer, it's clear that you don't understand Vue developers needs. This is not a problem in fact, you don't need to perfectly understand all developers needs, but please don't reject community feedback like this one just for that. This feature request is not about introducing any breaking change. All we need is a "after cache" data transformation hook. React developers will (probably) not use it, fine. Most (if not all) Vue developers will (probably) use it, fine too! Everyone will be happy! What's the problem to add it? |
Hi @phryneas, |
@steven-twerdochlib Please create a local copy of that data using e.g. @M1CK431 I am sorry, I am just seeing your answer. I still believe that your feature request is much better suited to be asked of the maintainers of They are in a much better place to make a call if this feature is good for their users, and of course they can add additional features on top of Apollo Client, especially something like this, which needs no deep core integration. |
@phryneas |
@steven-twerdochlib you would copy the data into a new object and modify that new object - that's perfectly fine. You could also track your changes separately and merge the values together to display them, if you would want to keep your changes up to date over time. What you get AC is an observable object that provides you with a new object every time something changes in the cache. |
Hey @phryneas
As you can see I'm changing the value/response from the change event and this works fine. |
@phryneas I will not waste more time trying to explain again and again why your suggestion...
... will simply NOT solve the issue. In addition, as I already explain, there is nothing "Vue specific" in a generic "after cache" data transformation hook. Anyway, many people request for it, you don't want with no real reason => feel free to close that issue. |
@steven-twerdochlib you are not modifying If you were to modify the event itself, in normal non-framework JavaScript, that would not have any consequences for the DOM and the next event you received would again be based on the actual DOM value and not reflect the changes you made to the previous event. |
Hello,
With the release of AC2.6 (and it's the same with AC3), all queries results are now read only.
However, there is use cases where it's handy and intuitive to mutate the query result (which is different than mutate the cache directly).
For example, when using VueJs, dev could write something like this:
With vue-apollo,
userMe
query result is stored directly indata.userMe
and, in VueJs,data
is the dedicated place for mutable data (aka mutate them will trigger a re-render). So dev will intuitively want to mutate the query result, like any other variables indata
.But, because the result object is now read only, for a real world example, when the dev will bind
userMe.name
to an<input>
, a console error will appears saying thatname
is read only.Currently, dev have to workaround that on every query this way:
To solve that while still preserving the cache from direct mutation, an option
deepCopyResults
(for example) could be added in ApolloClient constructor.Another way to solve that could be to add an option
transformResults
(for example) which accept a function to transform results after they are stored in the cache (a thing that a network link can't do if I read the documentation correctly). This solution may be more interesting because it allow for any global transformation after the caching step (covering more use cases).Wdyt? After all, there is already
assumeImmutableResults
so why notdeepCopyResults
ortransformResults
? 🙃The text was updated successfully, but these errors were encountered: