Infinite network requests with non-normalized objects

[oscar60310]

Issue Description

The issue is similar to https://github.com/apollographql/apollo-client/issues/9450, but it involves an additional non-normalized object.

I have two components which query the same object with different properties:

Compoment1

const QUERY_DATA_1 = gql`
  query foobar1 {
    foobar {
      id
      lastOnlineAt # changes on every query
      nested {
        value1
      }
    }
  }
`;

function Component1() {
  const { data } = useQuery(QUERY_DATA_1, {
    fetchPolicy: "cache-and-network",
    nextFetchPolicy: "cache-first",
  });

  return (
    <div>
      1: {data?.foobar.lastOnlineAt} {data?.foobar.nested.value1}
    </div>
  );
}

Component2

export const QUERY_DATA_2 = gql`
  query foobar2 {
    foobar {
      id
      lastOnlineAt # changes on every query
      nested {
        value2 
      }
    }
  }
`;

function Component2() {
  const { data } = useQuery(QUERY_DATA_2, {
    fetchPolicy: "cache-and-network",
    nextFetchPolicy: "cache-first",
  });

  return (
    <div>
      2: {data?.foobar.lastOnlineAt} {data?.foobar.nested.value2}
    </div>
  );
}

I realized that we should define a merge function for the non-normalized object foobar.nested:

new InMemoryCache({
    typePolicies: {
      Nested: {
        // merge: true,
      },
    },
  }),

Expected behaviour:

Before defining a merge function for it, I expected Apollo to log a warning and replace the value as described in the documentation.

Actual behaviour:

The two components send requests alternately and repeatedly.

https://github.com/apollographql/apollo-client/assets/9553914/8da3fb76-c20d-4017-93fd-d0dd50ca49e9

Link to Reproduction

https://github.com/oscar60310/apollo-issue-demo

Client code: https://github.com/oscar60310/apollo-issue-demo/blob/main/src/App.tsx Server code: https://github.com/oscar60310/apollo-issue-demo/blob/main/server.js

Reproduction Steps

No response

@apollo/client version

3.10.4

[alessbell]

Hi @oscar60310 👋

Thanks for opening this issue with a clear reproduction of what you're seeing!

First, when I ran your reproduction, I noted that the fetchPolicy and nextFetchPolicy had no bearing on the infinite loop behavior, so I commented them out.

Before defining a merge function for it, I expected Apollo to log a warning and replace the value as described in the documentation.

This is indeed what's happening, but for reasons I'll get into below, our two queries are "replacing the value" in an infinite loop, with no way to resolve the feud. First, here's the warning I saw:

Can you confirm you see a similar warning?

The warning notes that Cache data may be lost when replacing the nested field of a Foobar object and that This could cause additional (usually avoidable) network requests to fetch data that were otherwise cached.

As you noted, when the Nested entity can't be normalized, its fields are nested on our cached Foobar:foobar entity.

The problem arises when QUERY_DATA_1 and QUERY_DATA_2 select non-overlapping fields on this non-normalized entity. In practice, this means that as soon as e.g. QUERY_DATA_1 resolves with foobar.nested.value1, QUERY_DATA_2 sees that foobar.nested.value2 is missing in the cache, kicking off a network request whose response removes value1 from the cache and replaces it with value2. QUERY_DATA_1 then "notices" value1 is missing, and the cycle repeats forever.

I think a note in the docs advising against selecting non-overlapping fields on non-normalized objects would be helpful but would love any other ideas/suggestions you have here. Thanks!

[oscar60310]

Hi @alessbell , thanks for the explanation!

Can you confirm you see a similar warning?

The warning notes that Cache data may be lost when replacing the nested field of a Foobar object and that This could cause additional (usually avoidable) network requests to fetch data that were otherwise cached.

Yes, I see the same warning. It makes me think that we might lose the benefit of the cache. In the worst case, each hook would send a single query. However, the consequence is much more critical than simply not using the cache; it could result in infinite requests being sent to our server.

Is it possible to escape from the loop when we detect a cache issue?

I noticed that if we don't have a value that changes with each request, there won't be an infinite loop, and the cache is not replaced (component1 and component2 both get the correct value).

export const QUERY_DATA_1 = gql`
  query foobar1 {
    foobar {
      id
      # lastOnlineAt
      nested {
        value1
      }
    }
  }
`;

https://github.com/apollographql/apollo-client/assets/9553914/5d6651c5-7811-4670-beae-4c63a849fabb

[marco2216]

This sounds very similar to this issue, a fix was being worked on for that, but not sure what the status is for that and if it would fix this issue also. I'm not sure what the difference is with this repro and mine, I didn't look in detail, but in my repro it only occurred with the read function defined, but it sounds like you're having the same issue even without a read function.

[yoavbls]

We're experiencing the same thing at Wiz. When the key field is missing accidentally for some object and the object returns two times in a short time with different fields this unexpected behavior happens. We didn't find it in the docs, we wish we had the option to disable automatic refetching in such cases. Are there any intentions to solve it? Thanks

[esposito]

I'm not an expert on the Apollo cacheing system, but wouldn't it make sense to have an option to merge non normalized nested objects by default?

[sdemjanenko]

Ran into this as well. I wonder if it would be possible to catch this with a linter. Or perhaps some logic could check the schema against the cache's typePolicy and indicate missing merge logic. Might be something fun to whip together.

[jerelmiller]

wouldn't it make sense to have an option to merge non normalized nested objects by default?

@esposito We don't do this because its very easy to get into a situation where would-be distinct records could get merged together accidentally. The only sensible thing the cache can do by default is replace non-normalized records and require you to opt-into merging a non-normalized field together.

For example, let's say you have:

type User {
  id: ID!
  favoriteBook: Book
}

type Book {
  isbn: String
  name: String
}

One request might make:

query {
  currentUser {
    id
    favoriteBook {
      isbn
    }
  }
}

which returns favoriteBook as:

{
  __typename: 'Book',
  isbn: '9780679601395', // Invisible Man
}

Then let's say some time has passed, but your preferences have changed and you have a new favorite book, but another page in your site requests the following query:

query {
  currentUser {
    id
    favoriteBook {
      name
    }
  }
}

which returns favoriteBook as:

{
  __typename: 'Book',
  name: "The Lord of the Rings"
}

Now the cache contains data from 2 different books written to the same field:

{
  "User:1": {
    // ...
    favoriteBook: {
      __typename: "Book",
      isbn: "9780679601395",
      name: "The Lord of the Rings" // Oops the isbn doesn't match this book name
    }
  }
}

If we added an option to enable merging non-normalized records by default, its too easy for this situation to happen. These types of subtle bugs are very difficult to track down.

For this we require that you opt into merging non-normalized fields so that its obvious which fields are safe to merge:

const cache = new InMemoryCache({
  typePolicies: {
    User: {
      fields: {
        // This marks this field as safe to merge
        favoriteBook: {
          merge: true,
        },
      },
    },
  },
});

I wonder if it would be possible to catch this with a linter.

@sdemjanenko GraphQL ESLint does have support for a require-selections rule that enforces that you select id (or any other configured fields) to avoid this situation so this might help 🙂

Hope this helps!