apollographql / react-apollo

:recycle: React integration for Apollo Client
https://www.apollographql.com/docs/react/
MIT License
6.85k stars 787 forks source link

onCompleted not called when useQuery refetch finished #3709

Open jthistle opened 4 years ago

jthistle commented 4 years ago

Intended outcome:

Here is a brief, minimal outline of the inside of a React functional component I have:

const { refetch } = useQuery(MY_QUERY, {
  variables: {
    myVar
  },
  onCompleted: data => {
    console.log('on completed');
    sendAnEvent(false);
    setSomeState(data.foo.bar);
  },
});

function callback(event) {
  console.log('callback');
  refetch();
}

return (<div>
  <button onClick={ () => callback() }>Click me</button>
</div>);

What I expect is that, when I click the button:

  1. The message callback is output to the console
  2. The query is executed
  3. The message on completed is output to the console
  4. All the other onComplete actions execute

Actual outcome:

  1. The message callback is output to the console
  2. The query is executed
  3. The message on completed is not output to the console and none of the other onComplete actions execute

How to reproduce the issue:

See this sandbox for a working minimal reproduction.

Version

@apollo/client: 3.0.0-beta4

boas19910115 commented 4 years ago

Same issue. For now, my solution is to use React-Hook useEffect for detecting if the response data changed. Update redux if it's changed.

jthistle commented 4 years ago

Nice to know that other people have this issue. Is this repo even maintained anymore? @hwillson?

dylanwulf commented 4 years ago

@jthistle I believe they're in the process of merging this repo into the main apollo-client repo

hwillson commented 4 years ago

@dylanwulf is right; we're in the process of merging this project directly into apollo-client, for the release of Apollo Client 3 (you can follow along here). We're still maintaining this repo in the meantime, but we've definitely fallen behind on certain issues due to time constraints (PR's are definitely welcome!). I know this transition is a bit painful, but the good news is that one of the main benefits we'll get from merging AC + RA together is a much faster turnaround time on bug fixes + PR's.

verekia commented 4 years ago

Looking forward to the v3 :)

Am I doing something wrong, or is loading also not updating on refetch?

Seems like only data works on refetch.

  const { data, loading, refetch } = useQuery(
    gql`
      {
        random
      }
    `,
    { fetchPolicy: 'network-only' }
  )

  console.log('data', data)
  console.log('loading', loading)

  // return JSX

Console as I trigger a refetch:

data: undefined
loading: true

data: {random: 5071}
loading: false

data: {random: 7393}
loading: false

data: {random: 2262}
loading: false
verekia commented 4 years ago

The workaround I'm doing in the meantime is using useLazyQuery instead.

const [foo] = useLazyQuery(fooQuery, { onCompleted: () => /* ... */ })

useEffect(() => {
  foo()
}, [])

<button onClick={() => foo()} />

Both foo trigger the full lifecycle.

D9001 commented 4 years ago

@verekia How do you get this to work? useLazyQuery does the same thing for me after I call it the first time..

bradyhigginbotham commented 4 years ago

@D9001 Try adding fetchPolicy: 'no-cache' to your query options. It's most likely actually triggering but using the cache from the previous attempt, so it doesn't look like it's doing anything.

skywinston commented 4 years ago

Setting notifyOnNetworkStatusChange: true on the options object caused the refetch returned from useQuery to trigger the onCompleted lifecycle event.

The docs don't make this obvious, as the description of notifyOnNetworkStatusChange read:

Whether updates to the network status or network error should re-render your component. Defaults to false.

Turns out, refetching does change your network status, which if you have this option set to true will cause onCompleted to run again.

oshalygin commented 4 years ago

So the general feeling is work with the workaround listed above, notifyOnNetworkStatusChange: true or useEffect. Can confirm that notifyOnNetworkStatusChange solves the problem.

jthistle commented 4 years ago

I think the larger suggestion here is that this library will soon be deprecated and replaced with @apollo/client.

This is, in fact, an issue with @apollo/client, so I'm not sure what you mean.

oshalygin commented 4 years ago

you're right @jthistle notifyOnNetworkStatusChange: true is still required. Apologies.

ericucsc commented 4 years ago

I am on @apollo/react-hooks v3.1.3 setting notifyOnNetworkStatusChange: true does not make onCompleted to run after refetch is finished.

I tried to use userLazyQuery, I get to pass different variables to it every time I manually call to query. It does trigger the onCompleted callback when query finishes. But onCompleted is called on every re-render of my component even though there's no new query finishes. It holds the data from previous fetch, so I have to use an if to compare the data with the one I got from previous fetch (which I saved to a state), to figure out if this is a "new" data. Kind of hacky, but works for me at the moment.

dominik-myszkowski commented 4 years ago

Guys, set the fetchPolicy: 'network-only', it should work then, I had the same problem. const { refetch } = useQuery(MY_QUERY, { variables: { myVar }, onCompleted: data => { console.log('on completed'); sendAnEvent(false); setSomeState(data.foo.bar); }, fetchPolicy: 'network-only' });

adrenalien commented 4 years ago

I didn't want to do this like this but I just found this hack (I use it in a mutation onCompleted):

async function callback(event) {
  const { data } = await refetch()
  setSomeState(data.foo.bar)
}
debegr92 commented 4 years ago

Guys, set the fetchPolicy: 'network-only', it should work then, I had the same problem. const { refetch } = useQuery(MY_QUERY, { variables: { myVar }, onCompleted: data => { console.log('on completed'); sendAnEvent(false); setSomeState(data.foo.bar); }, fetchPolicy: 'network-only' });

Not working for me...

martinschaer commented 4 years ago

It works for me only if I add both options: notifyOnNetworkStatusChange: true and fetchPolicy: 'network-only'

jeffreyabarrios commented 4 years ago

Im using Apollo 3.0.1 on a React Native project and I'm facing the same issues, useLazyQuery does not run onCompleted after I call it a second time. And useQuery as well, refetch doesn't trigger onCompleted and neither after I make for example a client.writeQuery with the same query. Data updates, but onCompleted never gets called again.

notifyOnNetworkStatusChange doesn't change anything, and funny thing happening on react native with this version of Apollo... if I set any kind of 'fetch-policy', the query runs and goes on forever pinging the server and never stops even if it gets a 200 response.