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

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) #11841

Open
shyamayadav154 opened this issue May 12, 2024 · 2 comments

Comments

@shyamayadav154
Copy link

shyamayadav154 commented May 12, 2024

This issue is to bring the attention toward issue that is open in apollographql github repo

apollographql/apollo-feature-requests#372

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:

export default {
  data: { userMe: null },
  apollo: {
    userMe: { query: userMeQuery }
  }
};

With vue-apollo, userMe query result is stored directly in data.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 in data.

But, because the result object is now read only, for a real world example, when the dev will bind userMe.name to an , a console error will appears saying that name is read only.

Currently, dev have to workaround that on every query this way:

export default {
  data: { userMe: null },
  apollo: {
    userMe: { query: userMeQuery },
    update: ({ userMe }) => JSON.parse(JSON.stringify(userMe)) // <= ugly deep copy
  }
};

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 not deepCopyResults or transformResults? 🙃

@shyamayadav154
Copy link
Author

shyamayadav154 commented May 12, 2024

I am facing the same issue in my nextjs application, and I think it is solvable by reintroducing freezeResult option which was present in apollo 2.6 and got removed in 3.0

@phryneas
Copy link
Member

phryneas commented May 15, 2024

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.

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

No branches or pull requests

2 participants