Fragment matching broken when adding a new type that implements an existing interface #5750
Comments
|
Since this involves obtaining new information about Specifically, the server could send back any relevant I think it still makes sense to have a way of specifying |
|
Wow, that sounds pretty ideal. We are using Apollo Server (for our gateway), so that solution sounds like it would work. Should I file an issue in the apollo-server repo? |
|
Second that, that sounds fantastic |
|
Would this be the equivalent of the extension based strategy of ProgressiveFragmentMatcher? |
A small step towards implementing #5750 (comment).
|
@benjamn it may be a stupid question – but maybe we can get rid of How I understand all these things created for apollo-client/src/cache/inmemory/readFromStore.ts Lines 305 to 307 in 4cca16e apollo-client/src/cache/inmemory/writeToStore.ts Lines 258 to 260 in 4cca16e But what I really cannot understand – for what purpose exists this Anyway, the cache does not know anything about types and fragment types is some kind of duck-checking. I will be glad if you point me for test-case or issue where I can understand the real purpose of this |
|
No |
|
@benjamn any update on this issue? |
Easily the biggest breaking change for fragment matching in Apollo Client 3.0 was the removal of the FragmentMatcher abstraction, along with its subclasses HeuristicFragmentMatcher and IntrospectionFragmentmatcher, which were replaced by the declarative possibleTypes configuration. The HeuristicFragmentMatcher was so named because it attempted to perform fragment matching without any actual knowledge of supertype/subtype relationships in your schema, instead checking whether all the fields of a given fragment were present in the result object, which is indeed a relatively strong signal that the fragment probably matched. The HeuristicFragmentMatcher could be fooled by aliases and accidental sharing of field names between different fragments, but it was also relatively resilient to the possibility of adding new subtypes on the server, because heuristic matching doesn't care what the true subtypes of a supertype are, so what's one more? The other big drawback of the HeuristicFragmentMatcher was that it applied the same fuzzy logic to reading from the cache, where the heuristic makes a lot less sense. If an individual query result has all the keys you'd expect if a certain fragment matched, that's a pretty good sign that the fragment matched. But when you have a lot of data in your cache, from lots of different queries, it's not as meaningful to observe that all the fields required by the fragment are present, since they might be there because they've been written into the cache by other queries over time, not because the fragment actually matches the __typename of the object. In moving to the more exact possibleTypes system, we gave up both the benefits and the drawbacks of the HeuristicFragmentMatcher, replacing it with something functionally similar to the IntrospectionFragmentmatcher, but with a much simpler configuration API. As #5750 demonstrates, the exactness of the possibleTypes system makes it challenging for existing clients to adapt when a new subtype is added on the server. Is there any way to make this system more flexible, like the HeuristicFragmentMatcher, but without the drawbacks? This PR improves the possibleTypes API by allowing "fuzzy" subtype strings, which are interpreted as regular expressions for typenames, rather than as actual typename strings. If a fuzzy subtype matches the __typename of an object, fragments on supertypes of that fuzzy subtype are allowed to match the object, provided the result also has all the keys required by the fragment. The key advantages of this new system compared to the HeuristicFragmentMatcher are that (1) you can specify fuzzy subtypes for specific supertypes (rather than applying heuristic matching for all types by default), and (2) that it "learns" about fuzzy subtypes while writing (where the heuristic tends to work very well), and then merely uses those inferred supertype/subtype relationships when reading, so there is no need to perform heuristic matching while reading. This is the "twist" mentioned in the title of this PR. When one of these inferences happens, you'll see a warning in the console (in development). You do have to opt into fuzzy matching, but it can be a useful strategy for preparing your clients for upcoming server changes, or for relaxing the rules for a particular supertype, in cases when you know that its subtypes change frequently on the server.
Easily the biggest breaking change for fragment matching in Apollo Client 3.0 was the removal of the FragmentMatcher abstraction, along with its subclasses HeuristicFragmentMatcher and IntrospectionFragmentmatcher, which were replaced by the declarative possibleTypes configuration. The HeuristicFragmentMatcher was so named because it attempted to perform fragment matching without any actual knowledge of supertype/subtype relationships in your schema, instead checking whether all the fields of a given fragment were present in the result object, which is indeed a relatively strong signal that the fragment probably matched. The HeuristicFragmentMatcher could be fooled by aliases and accidental sharing of field names between different fragments, but it was also relatively resilient to the possibility of adding new subtypes on the server, because heuristic matching doesn't care what the true subtypes of a supertype are, so what's one more? The other big drawback of the HeuristicFragmentMatcher was that it applied the same fuzzy logic to reading from the cache, where the heuristic makes a lot less sense. If an individual query result has all the keys you'd expect if a certain fragment matched, that's a pretty good sign that the fragment matched. But when you have a lot of data in your cache, from lots of different queries, it's not as meaningful to observe that all the fields required by the fragment are present, since they might be there because they've been written into the cache by other queries over time, not because the fragment actually matches the __typename of the object. In moving to the more exact possibleTypes system, we gave up both the benefits and the drawbacks of the HeuristicFragmentMatcher, replacing it with something functionally similar to the IntrospectionFragmentmatcher, but with a much simpler configuration API. As #5750 demonstrates, the exactness of the possibleTypes system makes it challenging for existing clients to adapt when a new subtype is added on the server. Is there any way to make this system more flexible, like the HeuristicFragmentMatcher, but without the drawbacks? This PR improves the possibleTypes API by allowing "fuzzy" subtype strings, which are interpreted as regular expressions for typenames, rather than as actual typename strings. If a fuzzy subtype matches the __typename of an object, fragments on supertypes of that fuzzy subtype are allowed to match the object, provided the result also has all the keys required by the fragment. The key advantages of this new system compared to the HeuristicFragmentMatcher are that (1) you can specify fuzzy subtypes for specific supertypes (rather than applying heuristic matching for all types by default), and (2) that it "learns" about fuzzy subtypes while writing (where the heuristic tends to work very well), and then merely uses those inferred supertype/subtype relationships when reading, so there is no need to perform heuristic matching while reading. This is the "twist" mentioned in the title of this PR. When one of these inferences happens, you'll see a warning in the console (in development). You do have to opt into fuzzy matching, but it can be a useful strategy for preparing your clients for upcoming server changes, or for relaxing the rules for a particular supertype, in cases when you know that its subtypes change frequently on the server.
Easily the biggest breaking change for fragment matching in Apollo Client 3.0 was the removal of the FragmentMatcher abstraction, along with its subclasses HeuristicFragmentMatcher and IntrospectionFragmentmatcher, which were replaced by the declarative possibleTypes configuration. The HeuristicFragmentMatcher was so named because it attempted to perform fragment matching without any actual knowledge of supertype/subtype relationships in your schema, instead checking whether all the fields of a given fragment were present in the result object, which is indeed a relatively strong signal that the fragment probably matched. The HeuristicFragmentMatcher could be fooled by aliases and accidental sharing of field names between different fragments, but it was also relatively resilient to the possibility of adding new subtypes on the server, because heuristic matching doesn't care what the true subtypes of a supertype are, so what's one more? The other big drawback of the HeuristicFragmentMatcher was that it applied the same fuzzy logic to reading from the cache, where the heuristic makes a lot less sense. If an individual query result has all the keys you'd expect if a certain fragment matched, that's a pretty good sign that the fragment matched. But when you have a lot of data in your cache, from lots of different queries, it's not as meaningful to observe that all the fields required by the fragment are present, since they might be there because they've been written into the cache by other queries over time, not because the fragment actually matches the __typename of the object. In moving to the more exact possibleTypes system, we gave up both the benefits and the drawbacks of the HeuristicFragmentMatcher, replacing it with something functionally similar to the IntrospectionFragmentmatcher, but with a much simpler configuration API. As #5750 demonstrates, the exactness of the possibleTypes system makes it challenging for existing clients to adapt when a new subtype is added on the server. Is there any way to make this system more flexible, like the HeuristicFragmentMatcher, but without the drawbacks? This PR improves the possibleTypes API by allowing "fuzzy" subtype strings, which are interpreted as regular expressions for typenames, rather than as actual typename strings. If a fuzzy subtype matches the __typename of an object, fragments on supertypes of that fuzzy subtype are allowed to match the object, provided the result also has all the keys required by the fragment. The key advantages of this new system compared to the HeuristicFragmentMatcher are that (1) you can specify fuzzy subtypes for specific supertypes (rather than applying heuristic matching for all types by default), and (2) that it "learns" about fuzzy subtypes while writing (where the heuristic tends to work very well), and then merely uses those inferred supertype/subtype relationships when reading, so there is no need to perform heuristic matching while reading. This is the "twist" mentioned in the title of this PR. When one of these inferences happens, you'll see a warning in the console (in development). You do have to opt into fuzzy matching, but it can be a useful strategy for preparing your clients for upcoming server changes, or for relaxing the rules for a particular supertype, in cases when you know that its subtypes change frequently on the server.
|
#6901 was merged to help with this - thanks! |
Context:
Imagine a scenario where the client and server code are deployed separately.
Here's the server schema:
We have a client query that looks like this:
The client has been bundled with the latest fragment types, which include
RocketandUseras the possible types for theNodeinterface.Now, we add a third implementing type:
Since our client and server are deployed separately, the client's fragment types are stale.
Intended outcome:
Existing and deployed client code has access to all the fields defined in
TestFragmentfor all objects returned by theQuery.allTheStufffield.More generally: when fragment types are stale, we'd expect to see more cache-misses but not missing data.
Actual outcome:
Existing clients do not have access to the
idfields for any objects of theLaunchtype.Note that this problem does not exist when using the
no-cachepolicy, which makes the experience of using Apollo Client Cache inferior to that of not using the cache.More generally: When fragment types are stale, using the cache provides a broken experience with missing data.
How to reproduce the issue:
I created a fork of the fullstack tutorial that illustrates this issue. It uses
apollo-clientv3 (I tested and confirmed the issue in v2, as well) and configures thepossibleTypesforNodeto only includeRocketandUser. The "launches" page query does not receive any data for theLaunchobject returned fromQuery.allTheStuff.Versions
My company is moving towards using Apollo Federation with Graph Manager as the source of truth for our schema. We've been moving our schema-dependent tools and processes over to use the schema stored in Graph Manager. We're now looking at fetching the schema for the
IntrospectionFragmentMatcher. We've setup a script to download the schema from GM and generating the fragment types JSON from that. Since pieces of our schema are now being deployed separately and at different times, it will be difficult for us to keep the fragment types from being stale and potentially causing client bugs.The text was updated successfully, but these errors were encountered: