readField() in type policy merge function returns undefined only on first query

[miracle2k]

I was using @apollo/client@3.3.19 with a custom type policy to do pagination merging based on what is described here:

https://www.apollographql.com/docs/react/caching/cache-field-behavior/#merging-non-normalized-objects

My merge function looks as follows. It is intended to merge a connection-type entity with a nodes subkey, so something like:

query { 
  platform {
      events {
           nodes {
               id
           }
      }
  }
}

Here it is:

merge(existing: any, incoming: any, { args, readField }: any) {
      const { nodes: incomingNodes, ...rest } = incoming || {};

      let mergedNodes: Map<string, any>;
      if (existing?.nodes) {
        mergedNodes = new Map(existing.nodes);
      } else {
        mergedNodes = new Map();
      }

      incomingNodes?.forEach((item: any) => {
        mergedNodes.set(readField("id", item), item);
      });

      return {...rest, nodes: mergedNodes};
    },

This worked fine. After upgrading to @3.5.7 I see the following behaviour:

1. The first time useQuery() gets a result, readField("id", item) returns undefined. Because of this, only a single node is actually stored in the cache.
2. On subsequent requests (say to load the second page), readField works correctly.

I don't have a repo, but wonder if it is related to https://github.com/apollographql/apollo-client/pull/8508/files

Note: nodes is an interface type called Event, the nodes returned from the server are a concrete type implementing the interface (SaleEvent). I am defining possibleTypes in the cache (it implements both interfaces):

{
   Event: ['SaleEvent'],
   MarketplaceEvent: ['SaleEvent'],
}

[BogdanAdrian]

I'm experiencing the same issue on @apollo/client: 3.7.1 (latest release right now).
Every time I refresh the page, the existing argument from the merge function is coming undefined breaking my infinite scroll 😢

[ryancrunchi]

Same here, with any fetchPolicy involving cache : 'cache-first', 'cache-and-network', 'cache-only'...

[lprhodes]

Just adding that I'm experiencing the same issue, and is the same when setting 'network-only' as the fetchPolicy.

It's not all queries and so I'm digging into specifically what in the graphql output that's causing it to occur. I suspect it's when fetching a connection that contains a field with another connection.

[lprhodes]

It may be different for others but in my case I have the following graphQL structure:

User {
    id
    posts {
        edges [
            {
                node {
                    id
                    message
                    creationTimestamp
                    user {
                        id
                    }
                }

            }}
        ]
    }
}

If the top level User.id is the same as the User.post.edge.node.user.id then readField returns undefined during the first query. The second fetch of the query works as expected.

[lprhodes]

With this issue having been raised almost a year ago, has anyone found a work-around?

[lprhodes]

I've reverted to version 3.3.21 which resolves the issue. 3.4.0 onwards re-introduces it.

[m4riok]

I can confirm that what @lprhodes suggested is correct. This issue only occurs when an object id reappears within the nested objects. It also has another side effect, the read function gets executed twice and the ObservableQuery also fires twice.

[lprhodes]

Unfortunately version 3.3.21 that I downgraded to has an issue where it repeatedly performs queries created by the useQuery hook so I'll need to really dig into this issue.

[lprhodes]

In case anyone's also working on the issue...I'm getting closer to locating the specific code causing the problem. It's related to how the writeToStore function is using the applyMerges function when iterating through the context.incomingById variable.

[lprhodes]

Not a fix but I know specifically what's happening now.

In the case of the following graphql, where the root user.id and nested user.id (i.e. user.id.posts.edges.node.user.id) are the same, the merging of the data is being performed out of order. An attempt is made to merge user.posts` before the nodes are saved to the store .

User {
    id
    posts {
        edges [
            {
                node {
                    id
                    message
                    creationTimestamp
                    user {
                        id
                    }
                }

            }}
        ]
    }
}

I'm unsure yet as to whether this is intentional to prevent an infinite loop, a missed requirement or an actual bug.

I'll keep digging and see whether I can determine a fix for it but it'd be much appreciated if @benjamn could chime in please as the author of this piece of code.

[lprhodes]

I've now found the specific code that's causing the issue:

apollo-client/src/cache/inmemory/writeToStore.ts

Lines 420 to 434 in 9134aaf

	const previous = context.incomingById.get(dataId);
	if (previous) {
	previous.storeObject = context.merge(previous.storeObject, incoming);
	previous.mergeTree = mergeMergeTrees(previous.mergeTree, mergeTree);
	fieldNodeSet.forEach(field => previous.fieldNodeSet.add(field));
	} else {
	context.incomingById.set(dataId, {
	storeObject: incoming,
	// Save a reference to mergeTree only if it is not empty, because
	// empty MergeTrees may be recycled by maybeRecycleChildMergeTree and
	// reused for entirely different parts of the result tree.
	mergeTree: mergeTreeIsEmpty(mergeTree) ? void 0 : mergeTree,
	fieldNodeSet,
	});
	}

This code checks whether there's a previous entity and merges the two mergeTree's if so.

I believe this PR is the responsible: #8372

As a work-around, if you set previous to always equal false then it prevents the nested User object from merging with the root User object (and subsequently causing the User mergeTree to be called out of order).

I assume that by setting previous to always equal false, there will be an impact on performance but I'm yet to see any bugs.

[tapnair]

Having this issue too.
If this helps anyone I think I have a decent workaround.
mergedResults[item.__ref] = item;
Not ideal, but so far seems reliable.

[pz]

Is there any chance the team can respond to this? This continues to be an issue in 3.11.4. I've put together a repro here if that is helpful.

At the very least it would be nice if readField would loudly fail in these situations.

Our workaround has been to remove the id from the top level query object and then write the result back into the cache with the id to normalize the result. This has surfaced other issues which I won't detail here, but the fallout from this particular issues has taken up quite a bit of the team's time. It would be great to see this get fixed! Thanks