apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.34k stars 2.66k forks source link

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

Closed shyamayadav154 closed 3 months ago

shyamayadav154 commented 4 months ago

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

https://github.com/apollographql/apollo-feature-requests/issues/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 commented 4 months ago

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 commented 4 months ago

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.

alessbell commented 3 months ago

Hi @shyamayadav154 - since this is a feature request and one that exists over at https://github.com/apollographql/apollo-feature-requests/issues/372 I will close this issue out, but please feel free to add any other thoughts over on that issue. Thanks!

github-actions[bot] commented 3 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

github-actions[bot] commented 2 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. For general questions, we recommend using StackOverflow or our discord server.