apollographql / apollographql/apollo-client
TypePolicy Merge function not called before cache deepmerge preventing to solve race-conditions
- Dominant language
- TypeScript
- Stars
- 19.8k
- Forks
- 2.9k
- Avg merge
- 10h 17m
- Merged PRs (30d)
- 19
Description
Dear Apollo team, hope you are having a good day.
We just found that `typePolicies` **_merge function_** of a root type is not being called at all **before** the cache automatically deep merges the existing and incoming payloads.
We have a simple use case: we experience some race conditions between HTTP and WebSockets layer (queries/mutations and subscriptions), where we get outdated model data **after** newer data has already been correctly stored in the cache.
We just thought about a simple solution: let's just write a simple merge function for that type (ie Person) checking if the `updatedAt` field is **greater** in the cache (existing) than the incoming one, then disregard the incoming payload.
Well, spent a few days on debugging this slowly and understanding the internals, we just couldn't find a way to make the merge function work correctly. It was always being called too late.
Hopefully I am wrong and there's another way around it. I am attaching a PR that allows this functionally to work. I am open to improve it if you find this makes sense and helps the community.
**Intended outcome:**
After writing a custom merge function for a type (ie Person) we can compare the cached (existing) with the incoming and decide what to do.
In this test case as you can see the Person model is not updated in the second write since the `updatedAt` field is older. This prevents network race-conditions. We don't believe network order should be assumed to be correctly.
```ts
const cache = new InMemoryCache({
typePolicies: {
Person: {
merge(existing, incoming, tools) {
if (tools.isReference(existing) && !tools.isReference(incoming)) {
const cachedData = tools.cache.data.lookup(existing.__ref);
const existingUpdatedAt = cachedData?.["updatedAt"];
const incomingUpdatedAt = incoming?.["updatedAt"];
if (
typeof existingUpdatedAt === "number" &&
typeof incomingUpdatedAt === "number" &&
existingUpdatedAt > incomingUpdatedAt
) {
return existing;
}
}
return tools.mergeObjects(existing, incoming);
},
},
},
});
cache.writeQuery({
query,
data: {
person: {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "ACTIVE",
updatedAt: 100000,
},
},
variables: {},
});
cache.writeQuery({
query,
data: {
person: {
__typename: "Person",
id: 123,
status: "DISABLED",
updatedAt: 50,
},
},
variables: {},
});
expect(cache.extract()).toEqual({
ROOT_QUERY: {
__typename: "Query",
person: {
__ref: "Person:123",
},
},
"Person:123": {
__typename: "Person",
id: 123,
name: "Gaston",
age: 28,
status: "ACTIVE",
updatedAt: 100000,
},
});
```
**Actual outcome:**
Merge function is not being called for Types but for each operation (Query, Mutation, Subscription) root type **after** the cache already merged the incoming data. We think it should be called **before** the cache deep merges it so we have a way to decide what to do.
**How to reproduce the issue:**
See test-case above and attached PR.
**Versions**
apollo-client 3.5.10
System:
OS: macOS 12.2.1
Binaries:
Node: 17.1.0
Yarn: 1.22.17
npm: 8.1.3
Browsers:
Chrome: 99.0.4844.51
Firefox: 96.0.3
Safari: 15.3
Contributor guide
Research direction
Start with the supplied two-write InMemoryCache reproduction and trace how typePolicies, cache.writeQuery, and root operation merges process the Person payload. Confirm the current ordering and compare it with the attached PR; done means the custom Person merge function can reject the older update while the expected cache.extract() result remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100