Merge function never brings existing value for an identified fragment on cache

[Richacinas]

I have 2 queries which I use with the same variable/input, like this:

query GET_MY_DATA_FOO($id: String!) {
  foo(id: $id) {
    id
    ...Impagados
  }
}

query GET_MY_DATA_BAR($id: String!) {
  bar(id: $id) {
    id
    ...Impagados
  }
}

fragment Impagados on Impagados {
  impagados {
    id
    __typename
    ebe {
      error
      nota
    }
    icired {
      error
      nota
    }
    rai {
      error
      nota
    }
  }
}

I use it, let's say:

const { data, error, loading } = useQuery(GET_MY_DATA_FOO, {
  variables: { id: '1' },
});

const { data, error, loading } = useQuery(GET_MY_DATA_BAR, {
  variables: { id: '1' },
});

Intended outcome:

I would expect my merge function to bring existing data from previous queries. This sample merge function triggers ok:

merge(existing = {}, incoming = {}, { cache, toReference, readField }) {
  if (existing) return { ...existing };
  return { ...incoming };
}

Actual outcome:

After GET_MY_DATA_FOO query was launched, retrieved the data, and triggered the merge function (with existing value undefined, of course), I launch GET_MY_DATA_BAR which comes with existing value undefined as well.

I have tried removing the id field from the fragment, so it wouldn't become an entity, so to say, and I could maybe access the same fragment already cached on the other query. But I cannot find a way to do it.

If I keep the id field there, the merge function triggers, there is no existing value and trying to read the fragment from cache already brings the new value that was already written on cache:

const existing = cache.readFragment({
  id: incoming.__ref,
  fragment: gql`
    fragment Impagados on Impagados {
      id
      ebe {
        error
        nota
      }
      icired {
        error
        nota
      }
      rai {
        error
        nota
      }
    }
  `,
});

Versions

System:
OS: Linux 5.4 Ubuntu 20.04.3 LTS (Focal Fossa)
Binaries:
Node: 14.15.0 - ~/.nvm/versions/node/v14.15.0/bin/node
Yarn: 1.22.15 - ~/.nvm/versions/node/v14.15.0/bin/yarn
npm: 8.1.3 - ~/.nvm/versions/node/v14.15.0/bin/npm
Browsers:
Chrome: 94.0.4606.71
npmPackages:
@apollo/client: ~3.2.5 => 3.2.9

[benjamn]

@Richacinas I'm not sure if this is a typo or an explanation, but I think your first fragment has an unnecessary impagados root field:

fragment Impagados on Impagados {
  impagados { # Is this necessary?
    id
    __typename
    # ...
  }
}

The second fragment you showed (in the cache.readFragment example) looks right to me:

fragment Impagados on Impagados {
  id
  __typename
  # ...
}

The id and __typename fields are important for Impagados objects to be merged in the cache (which is permitted when the ids match). Given the same id, the Query.foo(id) and Query.bar(id) fields should both hold references to the normalized Impagados object with that id. If you don't provide the id, those fields will just hold separate objects, rather than references to shared/normalized data.

[Richacinas]

Thanks for your answer.

I'm not sure if this is a typo or an explanation...

Sorry, that was a typo.

What I'm trying to achieve is that, when the second query is launched, I can read the value that I got on the first query for this fragment. Once I have that, I would like to choose which one I keep.

Should I keep the __typename and id fields on this fragment in order to to that, or should I toss id so they stay separate? How could I get the value of the other fragment either way?

Thanks a lot

[Richacinas]

Any insight about this?

[benjamn]

@Richacinas Thanks for the elaboration! Sorry in advance for the wall of text…

First some background: A merge function always operates on an individual field (and the existing/incoming/merged values of that field), so even when you put the merge function in the type policy for Impagados (a convenience enabled by #7070), it still runs against either the Query.foo field or the Query.bar field (or whatever field holds the object/reference). Because the field names are different, the field values are stored separately, which is why you don't see any existing data from the Query.foo field when the merge function runs later for Query.bar.

This separation of field values by field name would still happen whether or not you request __typename and id (that just determines whether the field value is an unidentified object or a normalized Reference), though I generally recommend requesting those fields (__typename + any key fields) whenever possible. The cache is able to behave much more intelligently on your behalf when it knows the identities of your objects.

What I'm trying to achieve is that, when the second query is launched, I can read the value that I got on the first query for this fragment. Once I have that, I would like to choose which one I keep.

Based your explanation, I think you might be looking for something (very roughly) along these lines, where your merge function uses readField to read the other field's value?

const otherFieldNames = new Map()
  .set("foo", "bar")
  .set("bar", "foo");

new InMemoryCache({
  typePolicies: {
    Impagados: {
      // Note that this merge function might run for any field that holds an Impagados
      // object, such as Query.foo or Query.bar.
      merge(existing, incoming, { fieldName, readField, args, toReference }) {
        const otherName = otherFieldNames.get(fieldName);
        if (otherName) {
          // If you don't need to pass args to readField:
          // const otherValue = readField(otherName, toReference("ROOT_QUERY"));

          // If you need to pass args to readField, it has a more flexible options API:
          const otherValue = readField({
            fieldName: otherName,
            args,
            // Specifying options.from is unfortunately necessary when calling readField
            // from a merge function, since there's more than one possible parent object
            // (existing or incoming).
            from: toReference("ROOT_QUERY"),
          });

          if (typeof otherValue !== "undefined") {
            // TODO Decide whether to return otherValue here or continue processing incoming.
          }
        }

        if (existing) {
          // TODO Decide whether to return existing or incoming or some combination.
        }

        return incoming;
    },
  },
})

I have to admit I'm not thrilled with the boilerplate/technicality/amount of this code. In particular, I regret the hard-coding of toReference("ROOT_QUERY"), since it assumes the parent object will be the root Query object, which is only really true when you happen to be fetching your Impagados objects using Query.foo or Query.bar.

It would be convenient if readField(otherName) could default to reading from the current object in merge functions, like readField(siblingFieldName) does in field read functions. The difference with merge functions is that the default parent object is ambiguous, since it could be on the existing side or the incoming side. Your use case seems to favor defaulting to the existing parent object, but what if you requested query { foo bar } in the same query, and neither one already existed in the cache? Then you might want to examine sibling fields from the incoming parent object in your merge function, which in this case has both foo and bar. A system like this should ideally end up in the same state if the fields arrive in a different order within the same query or across multiple queries over time. I hope I'm conveying some of what's tricky about this problem space, even though I don't have a great solution for you.

With all of that said, I do see the value in your use case, and would love to keep talking about how to make it easier.

[Richacinas]

Let me have a proper read about this again.. I would like to keep talking about it as well.