useQuery data invalidated by useMutation causing infinite loops (rerender)

[RemyMachado]

The data reference returned by useQuery changes when a useMutation returns the same type, even if it's the same resulting object.
This behavior causes infinite loops when the mutation call depends on the query data.

Some details:
Fetch policy: cache-first (default)
The useQuery doesn't refetch the data, it just invalidates the reference, making it impossible to correctly track changes through useEffect dependencies.
From what I understand, it's replaced by the updated cache from the mutation call.

Intended outcome:

1. Since this is the exact same data that is returned from the query.data and the mutation.data.nested.field the data reference should be kept the same. Now, since it's complex objects, I understand it may be impossible to compare the results without a custom policy.
2. A warning helping to understand what's happening (in development environment) would be great.
3. A parameter option for useQuery allowing to prevent this behavior. (I tried using lazyQuery or refetch without success).

Actual outcome:

1. data reference changing on every render
2. No idea of what's causing it
3. No idea on how to fix it

How to reproduce the issue:
given the following GQL schema

type User {
    # many complex fields
}

type House {
    owner: User
}

query FetchUser($id: ID!) {
    fetchUser(id: $id) {
        ...UserFields
    }
}

mutation CreateHouse($ownerId: ID!) {
    createHouse(ownerId: $ownerId) {
        owner {
            ...UserFields
        }
        ...OtherFields
    }
}

and the following react hook

const useHouseCreator = (userId: string) => {
  const {
    data: dataFetchUser // changes on every render occuring after the mutation
  } = useQuery(FETCH_USER, {
    variables: { id: userId },
  })
  
  const [
    createHouse,
    { data: dataCreateHouse },
  ] = useMutation(CREATE_HOUSE)

  useEffect(() => {
      if (dataFetchUser) {
        createHouse({ variables: { ownerId: dataFetchUser.fetchUser.id }) // invalidates dataFetchUser
      }
    },
    [dataFetchUser], // dependency is always different
  )
  
  return null
}

Additional note:
Removing the User from the mutation result...

mutation CreateHouse($ownerId: ID!) {
    createHouse(ownerId: $ownerId) {
        #    owner {
        #            ...UserFields
        #    }
        ...OtherFields
    }
}

...prevents the useQuery data to lose its reference. That's the reason that makes me think that apollo is replacing dataFetchUser.fetchUser with dataCreateHouse.createHouse.owner, thus losing the reference.

Versions

System:
OS: Linux 5.15 Ubuntu 20.04.5 LTS (Focal Fossa)
Binaries:
Node: 16.14.2 - /usr/local/bin/node
Yarn: 1.22.18 - /usr/local/bin/yarn
Browsers:
Chrome: 106.0.5249.91
npmPackages:
react: 18.2.0 => 18.2.0
react-dom: 18.2.0 => 18.2.0
typescript: 4.5.5 => 4.5.5
@apollo/client: 3.7.1 => 3.7.1

---

I don't think it's a duplicate of #6760 since they are discussing about the behavior of the fetch policy cache-and-network.
Also I struggled to find the problem and I first encountered this now closed issue #9204

[RemyMachado]

In my case, using a more precise dependency is enough to fix the problem.

 useEffect(() => {
      if (dataFetchUser?.fetchUser.id) {
        createHouse({ variables: { ownerId: dataFetchUser.fetchUser.id })
      }
    },
    [dataFetchUser?.fetchUser.id], // here
    // instead of `[dataFetchUser]`
  )

However, I still think it's really hard to understand/spot the cause of the error and that the documentation or behavior could be improved.

[FritsvanCampen]

Additional note:
Removing the User from the mutation result...
...prevents the useQuery data to lose its reference.

This to me is an indication that perhaps apollo is updating the cache, which triggers an update on your query. I'm not sure if this is default behavior or that you have a specific cache implementation that does this. Either way, this seems reasonable to me, and also that the query updates.

The real problem here in my opinion is the use of useEffect. There is some good information about when not to use useEffect. I'm working on removing spurious useEffects from my code base. I think I followed the principles outlined here: https://beta.reactjs.org/learn/you-might-not-need-an-effect

The 'proper' way to do this is with a createHouse() function that is called after a user action, and you should tie it in that way - specifically don't tie it to state. Since you need to fetch some data for the mutation it could look like:

const createHouse = async (): Promise<void> => {
    const data = await apolloClient.query(FETCH_USER, ...);
    await apolloClient.mutate(CREATE_HOUSE, ...);
}

[RemyMachado]

Hey @FritsvanCampen, thanks for your comment.

Indeed, it's a reasonnable behavior, but the mutation is returning the exact same object as the query, thus the incomprehension of losing the reference.

---

The 'proper' way to do this is with a createHouse() function that is called after a user action, and you should tie it in that way - specifically don't tie it to state

The createHouse process, in my case, is a silent one which has no button to trigger the function.
I need the query to render other components in the mean time. I can't await for it nor set the result afterward. (Note that the refetch cost would be insignicant with proper caching, but I wouldn't like to take this path.)

I found that the onCompleted callback option can work around this issue, so there is no need for an additional options field as I suggested.

Since, I'm entirely relaying on the state for the mutation, I think it makes sense to tie them together.
Now, onCompleted perfectly replaces the useEffect, but otherwise I think I would need one.

[FritsvanCampen]

Indeed, it's a reasonnable behavior, but the mutation is returning the exact same object as the query, thus the incomprehension of losing the reference.

We live and learn 😄

The createHouse process, in my case, is a silent one which has no button to trigger the function.

The mutation is likely triggered by some user input somewhere, hopefully not too far away. I get the impression that rendering a component triggers the mutation, but something has to trigger the rendering of the component, right? See if you can trigger the mutation there.

Using onCompleted is similar to doing:

const useHouseCreator = (userId: string) => {
  React.useMemo(() => {
    const data = await apolloClient.query(FETCH_USER, ...);
    await apolloClient.mutate(CREATE_HOUSE, ...);
  }, [ userId ]);
  
  return null;
}

This makes it very obvious were triggering a mutation during the render phase; the consensus is that this is not a good idea.

'Fixing' this might have a ripple effect on your code but I do think it's the right way to do it. In any case, I don't think this is an issue for Apollo.

[RemyMachado]

The mutation is likely triggered by some user input somewhere, hopefully not too far away.

The component is rendered when the user visits/lands on the page. There's no user input up the stream.

Using onCompleted is similar to doing: [...useMemo example...]

I'm not sure I understand what you mean there. Is it an example of what you should not do? Because useMemo() shouldn't be used for side-effects.
I think there's nothing wrong wih triggering side-effects during the render phase as it's the initial intent of useEffect().

---

In any case, I don't think this is an issue for Apollo.

We derived a bit from the initial issue, by discussing the use of useEffect() in my example, but my concern is about the fact that I lose the reference on a data that is updated (apparently with the cache) with the exact same object. Thus, a cache object that don't need an update.

[FritsvanCampen]

The mutation is likely triggered by some user input somewhere, hopefully not too far away.

The component is rendered when the user visits/lands on the page. There's no user input up the stream.

You've got your user input right there 😄. Trigger the mutation on page load outside of the component.

my concern is about the fact that I lose the reference on a data that is updated (apparently with the cache) with the exact same object. Thus, a cache object that don't need an update.

I understand. That's a much narrower issue. I think that pleading for your expected behavior to be the default is not likely to be successful. The obvious way to get your behavior is to compare the data before deciding to emit an update. A compare is not cheap, and in most cases the data will have changed, why else did you refetch? I think you can implement a custom caching strategy that does what you want, so the option is available to you; but having this as a default is not likely to be accepted.

[RemyMachado]

Trigger the mutation on page load outside of the component.

Where should I put it? It's the first component that renders and needs the information. I'm not using SSR on this page.

why else did you refetch?

I didn't refetch. It's the result of the mutation that is updating the query result reference

your expected behavior to be the default is not likely to be successful

Yes, and I thank you for challenging it. Afterward, I still think it may profit the apollo client to detect an infinite loop caused by a lost data reference due to cache updates. I imagine the ApolloProvider looking out for this issue like the react StrictMode does.
It would help for a quick debug. Also, I struggled, because the apollo debug chrome extension is not working on my side, I can't lookup the cache with ease and it took me time to realize the issue could be coming from here.

2. A warning helping to understand what's happening (in development environment) would be great.

[FritsvanCampen]

I didn't refetch. It's the result of the mutation that is updating the query result reference

You refetch it as part of your mutation result. You don't have fetch the whole result of a mutation. You can just leave out parts of the result you're not interested in.

If you still want to pursue this in Apollo I would suggest you make a new ticket with the narrower revised case, something like: useQuery emits an update after refetch even if data doesn't change.