apollo-client: 3.0.0-beta.43 Missing cache result fields warning, cache not updated after mutation
I’m getting the following error after upgrading to 3.0 beta.43
Look like related to #5603 and #5146. Do i need to write keyFields manually for all type?
const link: any = errorLink.concat(authLink.concat(httpLink as any));
export const apollo = new ApolloClient({
link,
cache: new InMemoryCache(),
});
export default function ProductLayout() {
const query = useProductLayoutQuery({ variables: { productId } });
const product = query.data?.product;
return (
<Page>
<Product />
</Page>
);
}
// ProductLayout.gql
query productLayout($productId: Int!) {
product(id: $productId) {
id
name
description
}
}
export default function Product() {
const query = useProductQuery({ variables: { productId } });
const product = query.data?.product;
return <div />
}
// Product.gql
query product($productId: Int!) {
product(id: $productId) {
id
fullname
}
}
Intended outcome:
The cache should up to date Actual outcome:
The cache is not updated How to reproduce the issue:
Versions 3.0 beta.43
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 32
- Comments: 29 (7 by maintainers)
I don’t quite get this. The docs state clearly (https://www.apollographql.com/docs/react/caching/cache-field-behavior/#merging-non-normalized-objects) that as long as we fetch the id, the two objects can safely be merged in cache.
I am asking this, because i get tons of “Missing cache result fields” warnings, despite in my opinion, all these entities could have been merged safely. Do i need to customize
InMemoryCacheto fix this?edit: and just to clarify:
if i have a list and a detail view and in the list i fetch just the id and the name of a product, but in the detail page i fetch id, name, description and so on.
Should i get this warning in this case, yes or no? i tried to debug it on my project and it seems like this is indeed the case. I get the impression that this is a total normal pattern to fetch once less data and later more data from an entity and should not trigger a warning or am i wrong?
As mentioned in https://github.com/apollographql/apollo-client/issues/6136#issuecomment-615933193, this new warning is intentional. When query/cache mismatches like this happen in AC2, they are essentially swallowed. For example, when you have one query that’s relying on a certain set of data in the cache, but run another query that ends up removing some of the fields the first query was expecting from the cache, the inability of the first query to pull out what it needs is not exposed to developers. This cache underwriting/overwriting problem has been a longstanding issue in the past, so AC3 introduced this new warning to help developers know when this problem occurs.
Looking at the first
MissingFieldErrorfor example, we seeCan't find field 'name' on object .... This means at some point you ran a query to retrieve and store the data mentioned in that error message, and included anamefield in your query’s selection set. Then at some other point in your application, you ran a very similar query that ended up resolving to the same entity stored in the cache, but you didn’t use thenamefield in your selection set. This means thenamedata ended up being dropped from the cache, so the first query that was dependent on it can no longer find it. Depending on your fetch policy this might not be an issue at all, which is why this is a development time warning only (it won’t show in production). But it’s also quite possible you didn’t mean to do this. To resolve this cache mismatch issue (and again depending on your fetch policy), Apollo Client might decide to head to the network to fetch the data it needs, including the missingnamefield. But this means an extra network hit, which could have been avoided, if you just included the extranamefield in both queries (since again, they’re set to point to the same entity in the cache).If you’re migrating from AC2, you can definitely choose to ignore these errors if you want to, but we highly recommend looking into their cause. Addressing them will help keep your cache healthy and improve cache hits.
“Missing cache result fields” do not explain at all why this is happening. Also the solutions you provide seem not be very elegant.
This is against component design, so no-go. You don’t want to micro-manage which component should render first!
With this suggestion you need to manage
InMemoryCachefor every query you do. This is cumbersome and error prone. Why do i have to alter the behavior of InMemoryCache when i do different queries? Do i need to think: "oh, i remember that i used a different subset of this type somewhere else in my application, so i need to go to the initialization ofInMemoryCacheand try to invent some new id for this query? I don’t see that happening. I just tried to update to AC3 and myInMemoryCacheis already cluttered with cryptic merge functionssame problem: you need to clutter InMemoryCache and you probably don’t even know why. Even worse: if you remove these queries later, you probably keep all this custom logic in InMemoryCache and no one will ever know if this can be safely removed or not.
i also realize that the underlying issue is harder than we all know, but i had the impression AC3 would be clever enough to handle this hard problem for us. Instead it just gives one of the hardest problem in programming back to developers instead of solving it for us.
Its good that AC3 now shows us that these problem exists, alltough we probably did not even realize. But I am missing clean solutions for these problems. I did not spent much time dealing with
typePoliciesin InMemoryCache, so i don’t have a final conclusion, but it feels wrong to me. It centralizes what should not be centralized.Edit: i don’t want to sound harsh, I still love what you guys are doing (beeing a fan since meteor 0.2 something), i am just confused and I get stressed-out when i don’t understand something 😦.
@benjamn thank you for your clarification
Its this
You may be getting a warning in that case, but that doesn't mean anything bad is happening—unless you consider triggering a network request bad, which it can be, of course, when you don't expect it to happenthat triggered me when i tried to understand these warnings. I got the impression that I do something horribly wrong. That something is not supposed to happen like it did. That’s why I and probably many others got confused.
We were aware of what we fetched when, we used fragments and tried to have a good incremental loading and a good trade-off between overfetching and reducing network calls. Now all of sudden apollo client tells us that we doing something wrong, while in fact, everything is (probably?) fine.
Problem with warnings is, that they get ignored or turned off. When we have this warning because of some query-combination that is totally ok and on purpose, we can’t let it just be there. Because then, if later there is an actual problem, it will get ignored.
So personally I don’t believe that this kind of warning is useful at all. Maybe it would be better to toggle some diagnostic/benchmark mode that checks if many similar but different queries are run with a lot of smaller cache misses and give some summary about the “health” of the caching.
Maybe this could also be done through the apollo cli. Similar to the type generation it could even statically give some hints about what different queries using the same types are used. E.g. in a classic list - detail situation, you would expect to have at most two queries where one is a subset from the other ideally.
@anuragnagarkota This is definitely a challenging issue. These warnings are really just intended to help give some visibility into query/cache mismatching. We want to make sure developers have greater clarity into why data isn’t being matched from the cache, when it looks like it should be. Using a super set of all fields in a selection set can definitely be problematic/cumbersome. There are other options, like:
keyFieldsreadfunctions for the fields you know will differ between queries, to provide fallback valuesI realize the above suggestions might also be cumbersome, but again the new warning is just exposing behavior that has already been happening in AC2. Ignoring it is definitely still a valid option.
Judging by the 👍’s on this issue though, maybe we should consider providing a way to silence these warnings. Our full AC3 docs aren’t ready yet and we’re planning on having a section in them to help explain this warning, which will hopefully help as well.
An overly noisy warning is no warning at all, because people will become desensitized to it and start ignoring it. Moreover, the more important instances of the warning are being drowned out by all the less important instances in the console.
I’d much rather see some granularity and clarity about these warnings to differentiate between “hey just fyi you’re hitting the network” and “this might actually be an error in your cache”
@macrozone The way to think about the
InMemoryCacheconfiguration API is that it gives you a single place to define any non-default behavior of your graph that should apply uniformly across your entire application. When you think about it this way, you’re not cluttering anything. You’re saving yourself from having to think about concepts like object identity or field storage elsewhere in your application.You are right that some information, like the shape of individual queries, should not be centralized, and should not have surprising cross-component constraints, like needing to add fields to one query to satisfy other queries that will later consume the same cache data. Some of these problems can be eased by combining (or compiling) all your queries/fragments into one big query, but that approach has its own problems, like delaying the whole response until the slowest part finishes, and potentially over-fetching object fields. I think we can improve this situation dramatically though some combination of build-time validation/feedback, automatic query transformation, cache cleverness,
typePoliciesconfiguration, and better errors and warnings, but we’re not there yet.However, the
keyFieldsconfiguration definitely does make sense to centralize, because you’re going to have lots of problems if you apply different definitions of identity to objects of the same type in different parts of your application. As you probably know, the cache can automatically compute IDs for objects that have__typenameandidfields (and thus automatically merge their fields, when their identities are known to match), so you don’t usually need a customkeyFieldsconfiguration for types that follow this convention. These defaults are intended reduce the amount of custom configuration you need, but your mileage will vary. Another way to keep yourtypePoliciesshort and maintainable is to use helper functions to generate repetitive policies.I also want to stress that these warnings are definitely too noisy right now! They are surfacing useful information, in the sense that the kind of problems they’re warning about would be hard to diagnose without this kind of information, but that doesn’t mean the problem is always really a problem. In your example of a list/detail view, it should be fine to fetch just the
idfirst, and then the rest of the fields later. You may be getting a warning in that case, but that doesn’t mean anything bad is happening—unless you consider triggering a network request bad, which it can be, of course, when you don’t expect it to happen… and in those cases it’s convenient to have some sort of warnings that can help diagnose why the result from the cache wasn’t good enough.The honest reason these warnings aren’t perfect in the official 3.0.0 release is that we knew we could continue iterating on them after the release, because changing the content or frequency of a development-time warning is not a breaking change. I’m not completely happy with the current noise level, but I didn’t want this particular rough edge to hold up the (already long-overdue) AC3 release. Now that that’s behind us, we will definitely keep iterating on these warnings, and we welcome any ideas to make them more useful and precise.
Do you also fetch the id of your object, in order for
InMemoryCacheto be able to identify the object?@catherineluse In our project we use an eslint plugin which requires that
idis always included wherever it’s available. Only downside is that we have to keep a copy of the server schema with the client code so that the eslint plugin can work. Here are a couple projects which provide this functionality: https://github.com/apollographql/eslint-plugin-graphql https://github.com/dotansimha/graphql-eslint We use the first one, but the second one seems to be more actively maintainedThanks @hwillson. But what I understand is you are telling to use the super set of all fields at every place? which could be very problematic to do while coding a complex project?
@travigd You might find this part of docs useful https://www.apollographql.com/docs/react/v3.0-beta/caching/cache-field-behavior/#merging-non-normalized-objects In your case, perhaps something like this would work -
Thie strategy works for me, but I don’t have any root-level fields (aka your
viewer) so unsure if this policy works as well on the topQuerytype.For the life of me, I couldn’t understand why sometimes, I could successfully run a query against the cache, and sometimes the same query returned null. Same query, same variables, different results.
Finally I figured out with the help of this comment https://github.com/apollographql/apollo-client/issues/6136#issuecomment-635282012 that the problem was that one of my seemingly unrelated queries had fetched the same data without returning the ID of it, and depending on the order the queries were run in, the query that didn’t have the ID was overwriting the query that did have the ID.
The moral of the story for me was to always return the ID of every item from every query, even if it’s nested two levels deep. If I had always returned the ID, I wouldn’t have had this problem. I wish there was some way to configure the cache to just throw an error if I don’t always return the ID of everything in a query. That would have saved me so many headaches.
@jakubnavratil https://github.com/apollographql/apollo-client/pull/7055 was merged to help with this. Thanks!
Should not show this warning when
returnPartialData: trueI’m receiving the same warning for a mutation that replaces one array of strings with a different one:
That sounds a bit unwieldy, do I have to manually write merge strategy for all arrays I may have in the schema?
@hwillson I have an entity that has 5 fields, 3 fields are always present in every queryn and 2 others that can be added in some other query. The entity is a relation to a parent entity.
So pretty much is something like this
profiles inside users is an array and same is the companies inside profiles. Companies is not always present
I first start the app, I query the users without profiles. Then I insert a new profile to the user and I update the cache. The profile does not have any companies. And actually I haven’t even query the companies yet. Why am I still getting this warning when I update the cache?