apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.36k stars 2.66k forks source link

Infinite loop caused by Type Policy with read function #7028

Open timothyarmes opened 4 years ago

timothyarmes commented 4 years ago

My app has a User type for which I've defined a type policy:

    User: {
      fields: {
        createdAt:  { read: (date) => date ? parseISO(date) : null },
      },
    },

I also have a query to fetch the currently logged in user, and a HOC is used to fetch that user and render sub components.

const GET_SELF = gql`
  query me {
    me {
      ...userFragment
      prefs { ...userPrefsFragment }
    }
  }

  ${Users.userFragment}
  ${Users.userPrefsFragment}
`;

const UserProvider = ({ render }) => {
  const { error, loading, data: { me } = {}, refetch, subscribeToMore } = useQuery(GET_SELF, { fetchPolicy: 'cache-first' });

  <snip>

  if (loading) return <Loading />;
  return render(me);
}

Now, in a subcomponent I'm running a query which fetches a set of users:

export const GET_MISSION = gql`
  query mission($id: ID!) {
    mission(_id: $id) {
      users { ...userDisplayFragment }
    }
  }

 ${userDisplayFragment}
`;

When that query returns the currently logged-in user as part of the users list, the code enters into an infinite loop.

The fields returned for the currently logged in user are merged into the user object in the cache. Despite the fact that these fields have not changed the value of that user object, this causes the 'me' query to rerun, which reruns the 'mission' query above, which leads to the infinite loop.

However, if I remove the createdAt field from the Type Policy for the User object then everything works.

I thought that I be able to work around the issue be returning the exact same parsed date object when the date doesn't change:


class DatePolicy {
  constructor() {
    this.prevDate = null;
    this.prevParsedDate = null;
  }

  read = (date) => {
    if (date !== this.prevDate) {
      this.prevDate = date;
      this.prevParsedDate = date ? parseISO(date) : null;
    }

    return this.prevParsedDate;
  }
}

And using this class in the Type Policy:

User: {
  fields: {
    createdAt: new DatePolicy(),
  },
},

however that doesn't solve the problem.

If however the read function returns the object unchanged then the loop is avoided:

read = (date) => date

So, it seems that when a read function that returns a value other than that passed in will force the query to re-run.

Versions

System: OS: macOS 10.15 Binaries: Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node Yarn: 1.19.2 - /usr/local/bin/yarn npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm Browsers: Chrome: 85.0.4183.102 Firefox: 77.0.1 Safari: 13.0.2 npmPackages: @apollo/client: ^3.2.0 => 3.2.0 apollo-link-logger: ^1.2.3 => 1.2.3 apollo-server-express: ^2.17.0 => 2.17.0

benjamn commented 4 years ago

@timothyarmes Can you put this setup into a reproduction? That will help us give you the right advice without guessing (though I can keep guessing if a repro is not feasible).

As an initial note, that DatePolicy pattern seems risky because you'll be using the same DatePolicy instance (with only one this.prevDate) for all User.createdAt fields, even for different users. I don't think that's the key to solving the original problem, but it seemed worth mentioning.

timothyarmes commented 4 years ago

Hi Ben. I'll try to find time to create a repo.

You're right, the DatePolicy class instance won't work.

ziggy6792 commented 3 years ago

Hey @timothyarmes

Thanks for sharing this I was having the exact same issue. I came across this issue and @theodorDiaconu has done a brilliant job creating a small package that does what we want here. I followed his example and it works beautifully.

Good luck :)

brainkim commented 3 years ago

Gentle reminder that we’re looking for reproductions. I greatly prioritize any issue which I can run, and I got a lot of issues on my plate at the moment. If not, I will get to this soon no worries 😇

rcbevans commented 3 years ago

I'm hitting the same issue, just spent 3 hours trying to debug.

The trigger seems to be a mutation using an update function to merge a field onto a type with a read function specified on the type policy.

garrettg123 commented 2 years ago

This took a lot of trial/error, but for me it was printing out the options argument for the merge function:

              merge(existing = [], incoming, options) {
                const merged = _.uniqBy([...existing, ...incoming], '__ref')

                console.log('Merged audioPosts', {
                  existing,
                  incoming,
                  options,
                  variables, // ADDING `: options.variables,` FIXED THE ISSUE (along with a React Native printing error)
                  merged,
                })

EDIT: Nevermind, still getting 7 of the same queries. It only makes 1 request and the rest hit cache, but the merge is annoying/slow.

WallisonHenrique commented 1 year ago

Your UserType or fields within UserType (createdAt) probably don't have an ID for Apollo to deduplicate the data in the cache. https://www.apollographql.com/docs/kotlin/caching/troubleshooting

What is probably happening is that Apollo saves the data of the logged User in the cache and the other request that lists the users modifies the data of this logged user (with different data) forcing a cache update. A single missing field in the cache for the query querying the logged in user will cause a new request. The infinite loop happens because the UserType data in separate queries are different and each request updates the cache with the expected format.

This comment reinforces my point. https://github.com/apollographql/apollo-client/issues/6760#issuecomment-870904729

You can confirm that you are having a cache update on the user type using the apollo client devTools extension in the cache tab. https://chrome.google.com/webstore/detail/apollo-client-devtools/jdkknkkbebbapilgoeccciglkfbmbnfm

Complement

Apollo Client identifies similar data from different requests by associating the field type plus the object id. Example: User:2 => Type + Id user { '__ref': 'User:2' } It uses this reference to associate data from the cache and when requested by another request it reuses identifying the _ref.

It may be that a schema of a request has an id (or the field used to reference it) and the other request that has the same type does not have this id. The first request adds the id and the second modifies the cache without the id. When the cache data modifies it calls again the first request and modifying the cache again calls the second. Infinite loop.

alessbell commented 1 year ago

Hi everyone 👋 In case it's helpful, we have a CodeSandbox and forkable GitHub repo for creating reproductions, which would greatly aid in prioritizing/assessing this issue. Thanks!

marco2216 commented 12 months ago

I experienced the same issue today. Adding a read function to a field would cause two queries to be fetched in an infinite loop. I figured out that it was caused by one of those two queries that queried data in this structure:

 product {
        id
        details {
          id
          type
          title
        }
      }

The backend returned the same id for product and details, and our default dataIdFromObject function just returns the object id directly, so they would have the same cache id. Adding the typename to the dataIdFromObject function fixed the issue. Quite an odd issue, the read field was added to a completely different field, a field that was not even queried by the query that the snippet above is from. It was queried by the other query which was a part of the infinite loop though.

marco2216 commented 11 months ago

It started happening again with different queries, now I realised that the infinite loop also happens if there are some queries getting data from types that don't have ID where there is no merge function defined in the cache, if the queries override the data of each other. For some reason, this doesn't cause any issues usually, but as soon as the read function that returns new data is added, if one of the queries queries the field the read function is added to, it starts causing the infinite loop.

marco2216 commented 11 months ago

@alessbell Here is a reproduction of the issue with the cause being that different data in two queries is being fetched from the same type that doesn't have an ID/ or merge function in the cache combined with the custom read function on Person.name Upon opening the Sandbox, if you open the browser console, you should see query AllPeople being logged continuously, since the query is being repeated infinitely. If you remove line 121 in index.jsx name: () => "name_", and reload the page, you should see that the query only fires once. Sandbox link

marco2216 commented 10 months ago

@benjamn I would greatly appreciate it if you have time to look at this issue and my reproduction example above, this is blocking us from using custom cache read functions as I can't risk this happening in production.

phryneas commented 10 months ago

Hi @marco2216, I've taken a look at this and believe #11430 might get closer to solving this problem. The problem of potential feuds between queries is very complex, though, so this will likely need a lot more investigation before we can get it in - just as a fair warning.