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

RFC (graphcache): Cache resolve & invalidate with partial args #2713

Open
villesau opened this issue Oct 4, 2022 · 6 comments
Open

RFC (graphcache): Cache resolve & invalidate with partial args #2713

villesau opened this issue Oct 4, 2022 · 6 comments
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release

Comments

@villesau
Copy link
Contributor

villesau commented Oct 4, 2022

Summary

Let's say I have relay paginated savedArticles field that takes folder id into account. If I want to invalidate or resolve it, I need something like:

cache.resolve('Query', 'savedArticles', { "first": 20, "folderId": "187", "term": "" })

I'm removing an article from folder 187. I'm not too interested in the page it is at, all I want is to invalidate all the queries that matches folderId 187. Would make the code more readable and easier to write, if I could simply: cache.invalidate('Query', 'savedArticles', { "folderId": "187" }) without knowing anything about extra arguments. In my use case, the folderId is known by the mutation I'm using while the other variables are not interesting in that regard.Thus, I don't have easy access to them. I want to invalidate everything that matches the folder without caring about the rest of the arguments.

Proposed Solution

I'm proposing partial args matching for the cache resolve & invalidate, similar to pattern matching. This would greatly help with dealing with cache invalidation in resolvers.

Requirements

@villesau villesau added the future 🔮 An enhancement or feature proposal that will be addressed after the next release label Oct 4, 2022
@JoviDeCroock
Copy link
Collaborator

JoviDeCroock commented Oct 4, 2022

Have you tried doing something like

cache.inspectFields('Query')
  .filter(x => x.fieldName === 'savedArticles'' && x.arguments.folderId === '187')
  .forEach(x => cache.invalidate('Query', x.fieldName, x.arguments))

I think alternatively or commonly the query could look something like folder(id: x) { savedArticles {} } which might make this process a bit easier 😅

I guess this might have been the reason behind your discussion #2712

@villesau
Copy link
Contributor Author

villesau commented Oct 4, 2022

Indeed, that was partially the reason behind the discussion :) Unfortunately for the API design, we also support fetching saved articles without the folder filter at all ("all saved articles", folderId as null) which makes this design more usable otherwise. E: hmm maybe there could be a pseudo folder returned by the backend when the folder id is null or similar 🤔

Looks like the snippet does not work if I also have an optimistic update for the mutation. when I remove optimistic update for the same mutation affecting different field (the return type of the mutation instead of the list) the above snippet starts working.

@JoviDeCroock
Copy link
Collaborator

Ye, not really sure whether we can allow manipulation of optimistic entities. Would need to read up on crdt but that could lead to the mutation returning a deleted entity and you getting into impossible states

@villesau
Copy link
Contributor Author

villesau commented Oct 4, 2022

That is a bit problematic. In other parts of the UI we'd need to optimistically update status of the article (is it saved or not), while elsewhere we'd need to remove it from savedArticles which I think cannot be made optimistically in the same handler.

@JoviDeCroock
Copy link
Collaborator

Is it the invalidation itself that isn't working? Because I am having issues figuring out how an optimistic result on an entity would be wiping out the invalidation of a link 😅 you are basically setting Query.fields(x) to null my assumption was that the result for the optimistic mutation was restoring that but if it's only a status boolean it's most likely going to be entity-based and can't restore the invalidated field.

@villesau
Copy link
Contributor Author

Unfortunately I moved on from the logic that I couldn't get working so I don't have any proper repro for that anymore, if that was even a bug instead of user error in the first place.

However, back to the original topic: I think that partial arg matches would greatly improve the usability of the graph cache. Now the behaviour is pretty ruthless: You either match something or you don't. And if you don't, you have very little tools to understand why something is not matched. You might have a typo or misunderstanding, or then some entity might simply not exist in the cache. In all the cases you have no good tools to understand why. By having more relaxed methods to pull stuff out of the cache would make it easier to work with and to understand what you actually have there.

An example: I had a case where my query was something like: query MyQuery($id: ID!, searchTerm: String!) {someQuery(id: $id, term: $searchTerm) {...Stuff}}. I spend a great amount of time pulling the field with inspectFields, trying to update queries with arguments I received from the inspectFields and so on. All I got was null as a result. Something didn't match and I suspected my understanding of Urql was somewhat fundamentally flawed or something didn't exist in the cache. The real reason was that I got term out from the inspectFields and update query took searchTerm in. I had tried to pass term instead of searchTerm. Since so many things with the cache are so dynamic, I believe it is very hard to rely to typescript there. Being able to resolve and fetch stuff from cache would have helped me to narrow down the issue much faster to the term & searchTerm discrepancy.

It would also help with invalidation: We have case where we'd like to have "select all" feature for an "infinite" list of items, that you might or might not have loaded to the frontend. You could then perform an action or all of them (also the ones that are not loaded in the frontend) which should reflect everywhere in the ui. However since you cannot exactly know which ones are affected, we should be able to invalidate all such entities no matter what are their ids and how they were originally queried. Now, to my knowledge, the invalidation requires you to know the entitys id or fields parameters exactly.

@kitten kitten changed the title RFC: Cache resolve & invalidate with partial args RFC (graphcache): Cache resolve & invalidate with partial args Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

No branches or pull requests

2 participants