Open M1CK431 opened 1 year ago
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):
Field policies enable you to define what happens when you query a particular field
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: https://github.com/vuejs/apollo/issues/58#issuecomment-297988517
Notice that this comment is from Akryum, which is part of VueJs core team and maintainer of vue-apollo library. His advice is:
you need to clone the array
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 .js
file and I guess I'm not the only one with this mixed usage. Of course, I would like to have a consistent behavior across all my queries regardless if I made them from within a vue component (through vue-apollo) or a regular js file (using AC client directly) and this is possible only if this feature request is made in AC directly.
To conclude, about the "potential performance tax", most of the time API results are not so giant that JSON.parse(JSON.stringify(data))
have a noticeable impact. In addition, we could imagine a per query optout mecanism, like for fetch policy: a global option + a per query override option and so it become so easy to handle big API results exceptions (if any). Anyway, since it's an option, people who worry about that could simply not use it π
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 :) Any news regarding this feature request?
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 @apollo/client
core package.
Reintroducing the option to turn off freezeResult
option would worse case result in situations where you'd accidentally modify cache-internal data, but without the cache actually being aware of it. That means that the cache would change, but not all consumers would be modified about the change - your UI would start to become inconsistent.
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 useState
hook and new state needs to be immutably constructed.
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. If you want to just update values that you also update on the server in parallel (so some form of optimistic/pessimistic update), that's probably fine, but if you want to really do persistent local modifications of data that started out as cache data, I'd really recommend you to use the appropriate means of your Framework to copy the cache value into some form of local data and modify it there the intended way.
I believe that in the case of vue-apollo, that data being stored in data
is more of an implementation detail and not an invitation to also modify that data - but I'll readily admit that I am not an author of the library, so I can't say that for sure. You'd have to ask the libraries maintainers for more clarity on that.
Lastly, something like a transformResults
option would need to be implemented on a framework-specific basis, based on what the framework/rendering library itself encourages.
For React, we won't do that, as that's what useMemo
is meant for, but other implementations might have different answers here.
@phryneas
Reintroducing the option to turn off freezeResult option would worse case result in situations where you'd accidentally modify cache-internal data, but without the cache actually being aware of it. That means that the cache would change, but not all consumers would be modified about the change - your UI would start to become inconsistent.
Totally agree. Forbid direct cache mutation is not the issue.
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 useState hook and new state needs to be immutably constructed.
Here is how reactivity works with Vue.js: https://vuejs.org/guide/essentials/reactivity-fundamentals To resume, Vue reactivity is based on data mutation, so it's not "discouraged" at all.
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.
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.
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. If you want to just update values that you also update on the server in parallel (so some form of optimistic/pessimistic update), that's probably fine, but if you want to really do persistent local modifications of data that started out as cache data, I'd really recommend you to use the appropriate means of your Framework to copy the cache value into some form of local data and modify it there the intended way.
This is out of scope since we don't want to modify the cache here.
I believe that in the case of vue-apollo, that data being stored in data is more of an implementation detail and not an invitation to also modify that data - but I'll readily admit that I am not an author of the library, so I can't say that for sure. You'd have to ask the libraries maintainers for more clarity on that.
If you read carefully the Vue documentation I provide, you will understand that you don't believe correctly, sorry π
And the vue-apollo
library author already answer this question years ago (see this comment please).
Lastly, something like a transformResults option would need to be implemented on a framework-specific basis, based on what the framework/rendering library itself encourages.
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, I didn't know where else to vote for this feature request so I'll do it here. It is common in my app to use apolloQuery to get data from backend, edit that data and then do an apolloMutation when and only when the user clicks save which would edit the values in backend. With the response from apolloQuery now being immutable I won't be able to edit the responses to show front end changes without manually deepCloning or shallowCloning the response in ever scenario this happens. I hope my thoughts help your decision about including this feature or something similar. All the best :P
@steven-twerdochlib Please create a local copy of that data using e.g. structuredClone
.
If we provided a writable copy of that data, it opens the door for a lot of foot guns, since for a lot of developers that would also create the assumption that we would keep updating that writable copy with future updates, which is not the case.
It is much clearer that you have a non-updating copy of that value if you do a structuredClone
in your codebase.
@M1CK431 I am sorry, I am just seeing your answer. We make a clear distinction between Apollo Client Core and Apollo Client React - while they share the code base, most of Apollo Client is not React-specific. We add features that are only useful for React users and might be confusing for users of other frameworks to Apollo Client React, not to Core. What you are asking here is adding a feature that will only be useful for users of Vue and might be confusing for users of other frameworks to Apollo Client Core. We do not want to do that - there is something as "too many options", and frankly, we already have too many options to the point where it gets confusing to new users, and find ourselves in the position that we cannot drop them because it would break someone's workflow. If we could drop a bunch of options from core without impacting anyone, we would do so. So you can imagine how careful we are about adding new features that we don't see bringing value to the broad userbase. This feature would confuse both React and Angular users, probably also a lot of people using Apollo Client in plain JS or with other integrations.
I still believe that your feature request is much better suited to be asked of the maintainers of vue-apollo
. The library already adds various Vue-only concepts on top of Apollo Client, and this kind of observed mutability is just in line with that.
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 Thanks for the quick response, one thing I don't understand though is how is apollo client meant to be used to modify application data then? Cause this immutable change clearly shows the way I thought it should be done is wrong.
@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.
You wouldn't directly modify a form ChangeEvent
directly either, but just read data out of it, right?
Hey @phryneas You're example about change event is confusing to me. In the scenario where you'd want to change the value a user enters to uppercase you would do something like this (correct me if I'm wrong):
<input :value="value" @input="onChange" />`
onChange(evt) {
this.value = evt.target.value.toUpperCase();
}
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...
I still believe that your feature request is much better suited to be asked of the maintainers of vue-apollo. The library already adds various Vue-only concepts on top of Apollo Client, and this kind of observed mutability is just in line with that.
... 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. Also, no one will be "confused" since it's an option, disabled by default, non breaking change.
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 evt
there (which would be the value you get from Apollo Client in this analogy), but this
, which seems to be something "special" provided by your framework.
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
? π