apollographql / apollo-feature-requests

🧑‍🚀 Apollo Client Feature Requests | (no 🐛 please).
Other
130 stars 7 forks source link

Documentation and warning about cache and merging of array types is misleading #433

Open antonvialibri1 opened 7 months ago

antonvialibri1 commented 7 months ago

Hello 👋 ,

I've a question about the merging of array types in the Apollo cache mentioned in the docs:

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

It says:

Merging arrays

By default, the field's existing array is completely replaced by the incoming array.

In my case, that's fine. I have an entities: [Entity!]! field which is an array type containing Entity objects. I'm performing a mutation that updates entities on the EntityParent parent type, but I get the following warning each time a mutation on EntityParent completes and EntityParent is refetched:

  console.warn
    Cache data may be lost when replacing the entities field of a EntityParent object.

    This could cause additional (usually avoidable) network requests to fetch data that were otherwise cached.

    To address this problem (which is not a bug in Apollo Client), define a custom merge function for the EntityParent.entities field, so InMemoryCache can safely merge these objects:

      existing: [
      { __ref: 'Entity:1' },
      { __ref: 'Entity:2' },
      { __ref: 'Entity:3' },
      { __ref: 'Entity:4' },
      { __ref: 'Entity:5' },
      [length]: 5
    ]
      incoming: [
      { __ref: 'Entity:2' },
      { __ref: 'Entity:3' },
      { __ref: 'Entity:4' },
      [length]: 3
    ]

    For more information about these options, please refer to the documentation:

      * Ensuring entity objects have IDs: https://go.apollo.dev/c/generating-unique-identifiers
      * Defining custom merge functions: https://go.apollo.dev/c/merging-non-normalized-objects

In my case, it's absolutely fine for incoming to completely replace existing, just like the docs say:

Merging arrays

... By default, the field's existing array is completely replaced by the incoming array.

However, why do I get the warning then?

The warning goes away if I add a dummy custom merge function like this:

...
    EntityParent: {
        entities: {
          merge(existing, incoming) {
            return incoming; // Ignore existing and always return incoming.
          }
        },
...

But again, given the documentation, Apollo should already completely ignore existing and replace it with incoming by default, right?

The docs don't say that we'll get a warning if we don't specify a custom merge function and keep the default behaviour. I find this a bit confusing 🤔 .

Is the warning always logged as long as we have an array type without a custom merge function?

Is this the behaviour you're trying to explain here? 👉 https://github.com/apollographql/apollo-client/blob/285b2d9c7bc9e9a1893a8705fc8ac2971225ea8a/src/cache/inmemory/writeToStore.ts#L842

  const childTypenames: string[] = [];
  // Arrays do not have __typename fields, and always need a custom merge 👈👈👈👈👈
  // function, even if their elements are normalized entities.
  if (!isArray(existing) && !isArray(incoming)) {
    [existing, incoming].forEach((child) => {
      const typename = store.getFieldValue(child, "__typename");
      if (typeof typename === "string" && !childTypenames.includes(typename)) {
        childTypenames.push(typename);
      }
    });
  }

  invariant.warn(
    `Cache data may be lost when replacing the %s field of a %s object.

Then, if arrays always need a custom merge function, why isn't that written explicitly in the docs?

Thank you.