relay: Subsequent queries can cause previous data to appear "missing" due to normalization
I have a fairly common, very reasonable use case that demonstrates what I believe to be a fundamental flaw in the relay cache/store update strategy.
Let’s use these simplified types in the scenario:
type SomeObject implements Node {
id: ID!
name: String
description: String
}
type SomeWrapper implements Node {
id: ID!
object: SomeObject
}
type Query {
get(id: ID!): SomeWrapper
search(term: String!): [SomeWrapper]
}
The important part here is that SomeWrapper references SomeObject.
In my use case, I have two QueryRenderers: one to fetch search results, and one to fetch all the details of a selected object. They must be independent queries for 2 reasons:
- the details query fetches MUCH more data than would be practical to return on all search results
- search query should NOT be re-run each time the selected ID changes for the details query
Here’s another heavily simplified description in GraphQL (in reality the queries are large, use the Connection spec, and are composed of many fragments):
query SearchQuery ($term: String!) {
search (term: $term) {
id
object {
id
name
}
}
}
query DetailsQuery ($id: ID!) {
get (id: $id) {
id
object {
id
name
description
}
}
}
The important part here is that the description field is present in DetailsQuery but NOT in SearchQuery.
The problem comes when I have details pulled up for a Wrapper with ID aaa. It looks like this:
{
"id": "aaa",
"object": {
"id": "111",
"name": "Name 1",
"description": "This is a HUGE piece of data."
}
}
Now, let’s say that I do a search and receive results which contain Wrapper with ID aaa. It looks like this:
[
{
"id": "aaa",
"object": {
"id": "111",
"name": "Name 1"
}
}
]
Nothing has changed, and this is great. However, let’s say that aaa has changed to reference Object with ID 222:
[
{
"id": "aaa",
"object": {
"id": "222",
"name": "Name 2"
}
}
]
This becomes EXTREMELY problematic. Because 222 is new to relay, there is no cache for the description field, but the results for DetailsQuery are updated to reference it anyhow. While having a “description” go missing doesn’t sound like a big deal, my real-world case has dozens of fields with nested queries. Thus, receiving this kind of search result essentially makes the entire screen go blank.
The problem, then, is that relay seems to split the middle between two more predictable updating strategies, where it would either:
- NOT update the
DetailsQueryresults, since it knows that its cache is incomplete - Make a followup query to the required
node(id: ID!)query root for the fields it knows are missing
I believe I can acheive the former by using separate environments, but this feels quite dirty.
If I recall correctly, the latter was the behavior in relay classic. Is there any way to restore this in modern?
I would be very interested to learn the reasoning behind the current behavior, and am willing to helping implement a better one if the relay team is open to the idea.
About this issue
- Original URL
- State: open
- Created 7 years ago
- Reactions: 7
- Comments: 28 (22 by maintainers)
Commits related to this issue
- demo facebook/relay#2237 — committed to mike-marcacci/relay-demo-2237 by mike-marcacci 6 years ago
First, thanks for filing this issue. We informally call this the “missing data” scenario (an incredibly imaginative name, i know): some component fetches a field on a related object (say, a user.bestFriend.name), some update occurs where the relationship has changed (user.bestFriend is someone else now), but doesn’t fetch the name field, and the first component observes that user.bestFriend.name is now missing. This can definitely be surprising if you aren’t expecting it and seem like a flaw: however, this behavior is expected and by design.
The reason is consistency: the alternative (using the same example) is that when the update occurs and user.bestFriend has changed, the original component isn’t notified and continues to show the previous friend. In other words: the main alternative is for the UI to be inconsistent.
In client side caches there is a fundamental tradeoff between completeness (do we have all the data each component requested), consistency (do all components represent a consistent view of the world), and performance (do we achieve those properties in a reasonable amount of time). In general a system can achieve two out of three. For example, to prioritize completeness and performance you can simply fetch each view’s data in-full, up front in a single round trip, and never update that data. To prioritize completeness and consistency you can refetch anytime data goes missing - at the cost of possibly infinite refetches if data keeps changing on the server (and note that if you care about consistency, you can’t just apply the first update bc that wouldn’t be consistent - you have to wait to show the affects of an update until you know you’re not missing any more data). Relay chooses a third route: prioritize consistency and performance - in our experience, “missing data” due to relationship changes are relatively more rare and also relatively easier to work around when they occur (for example, fetch the same fragment in both places to ensure data stays in sync).
That said, we haven’t documented this tradeoff clearly anywhere so it can be surprising when it occurs. We should definitely document it somewhere - we’d appreciate feedback about where folks would intuitively expect to see this (in the architecture docs? but would enough people read those?). Also note that we’re working on new react-relay APIs that use Relay’s knowledge of which fields are missing (Snapshot.isMissingData is true whenever we encounter an unfetched field while reading a fragment). We’re also working on better debug logging for when this case occurs.
In summary: this is a fundamental aspect of Relay’s design that we do not plan to change. We’re very open to documentation changes to make this behavior more clear, and we’re working on some implementation changes that make it more obvious when this occurs (and when it does, why).
OK, I finally got around to building a full demo. Check out this repo and this video.
Again, I’d like to stress the seriousness of this, as I suspect it is crashing apps across the web under very hard-to-reproduce conditions (because it is triggered by cross-user concurrency). I also want to note that I’ve explored a few ideas for a fix, but that there are architectural consequences to this.
So I never finished writing my new
QueryRendererbut am going to have to pick it back up. To make sure I’m conveying the severity of this issue, fields guaranteed by the API to be non-null can come become undefined because of this, making the generated flow types completely unreliable. In my case, somebody can crash an entire app just by typing into a search field, if the result list contains an updatedNodethat’s used elsewhere on the screen.I’m going to put together a small app to demonstrate this issue, but I’m quite amazed that others don’t appear to be running into this like crazy.
Here are my current thoughts on how this might be fixed; I wish I could have a whiteboard session with somebody more knowledgable than me on relay’s internals, but since we’re just in an issue thread please take these as “first thoughts” rather than a well constructed proposal:
Detection
This issue can be created many different ways. While my example demonstrates it when a new
Nodeis introduced to the store, it’s possible for theNodeto already exist with insufficient data, and later get referenced where it doesn’t belong. This makes it clear that detecting insufficient data needs to occur when it is used.Idea 1
The most obvious way to detect missing data is to, whenever the data is provided to a
QueryRenderer, iterate through the query (with all its fragments) and check everyNodefor the existence of each requested field.Pros:
Cons:
Idea 2
Another option is to flag nodes with the query and position when its data is returned by a particular query, using a “private” client-side field. Then, whenever data is provided to a
QueryRenderer, iterate through every returnedNodeto check the presence of the corresponding flag.Pros:
Cons:
Resolution
While detecting missing data is reasonably straightforward, resolving it isn’t.
Idea 1
When a
QueryRendererdetects missing fields, it can re-run its query to the server. This would add the missing fields.Pros:
Cons:
Nodes just because 1 is missing data)Idea 2
When a
QueryRendererdetects missing fields, it can re-request content for the specificNodeusing thenode(id: X)API. This is similar to how relay classic worked, but the queries could be preassembled at compile time.Pros:
Cons:
After thinking about this for a bit, both Detection Idea 1 and Resolution Idea 1 seem like the simpler options, even though they aren’t perfect. I would be very interested in hearing thoughts by somebody on the relay team, and if anybody actually would like to hash this out live, let me know and I can Skype or whatever.
@mike-marcacci I’m glad that you found a solution that works for your use-case. As I mentioned in my earlier comment, there are different tradeoffs across completeness, consistency, and performance, and there isn’t one approach that is appropriate for every app. Relay focuses on cases where consistency matters: if you don’t need consistency then a simpler/lighter solution can be more appropriate.
I’m sure there are apps where this problem never crops up; however, for me this was a prevalent and serious issue that couldn’t be fixed without major changes to relay itself. We ended up moving everything (across multiple react apps) to Jayden Seric’s graphql-react plus the Apollo CLI’s code generation tools. This library avoids all these problems by not using a normalized cache. While this does mean there’s less that I “get for free,” this approach was a more-than-welcome tradeoff for predictability and consistency.
Do you mean “nullable” here?
@sibelius i have found that this workaround does not work for me. is relay suppose to completely replace the node in the store when using the refetch container? Why would it not just fetch the data and merge it into the existing store?
Since relay is replacing the entire node, many fields become undefined in the store after the refetch. The only workaround I’m seeing is including all the fields within the refetch fragment. However, as @mike-marcacci pointed out, it’s not practical to fetch all the details when there’s a lot of data in certain situations