TypePolicy Merge function not called before cache deepmerge preventing to solve race-conditions

[gastonmorixe]

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.

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

[gastonmorixe]

PR #9503

[jwarykowski]

Hey @gastonmorixe / @benjamn,

I think I'm seeing this same problem on the latest version (v3.6.6). Essentially I provide my update to a nested document via writeFragment. Within my merge function I do something similar to what @gastonmorixe is doing however we set local fields of cacheStatus and cacheTimestamp, we define the update we have done (e.g UPDATE) and a timestamp for how long it should persist for (in our case 15 seconds). When we recieve a backend response we check these properties to determine whether to merge the incoming record.

Just to give some background we use a CQRS backend so the response is essentially a boolean value as to whether the event to do our update has been fired meaning we have to perist our local state as its not immediate.

I've added my scenario in the following code sandbox: https://codesandbox.io/s/apollo-client-issues-9502-cpi493?file=/src/index.js.

In this case we have a parent record (Contact) and nested record (Label), everything is updated as expected, however when the contact query is written again with stale data it overwrites all of the state even though the label merge funtion returned our persisted (existing) local values.

Just to note I have read the editing existing data section within the cache reading and writing documentation, if I omit the labelText from the 2nd get contact request the data is persisted so this does work and the functionality is working as per your documentation (if the value is defined it will be overwritten).

However, what I am trying to understand is how does this all work when you're using the useQuery hook? Should the top level root queries be honouring the merge functions which are defined for the types?

Any help would be greatly appreciated.

[gastonmorixe]

@benjamn updates? My PR is working quite well for us in production.

[glennholland]

Would like to see a fix for this too. We have a couple of cases where we do manual cache injection and would like to rely on the existing TypePolicy merge functions we have defined for when data arrives by subscription or query response.

[glennholland]

Any news on this? @bignimbus