Using useQuery with pollInterval triggers onCompleted only once

[kocur4d]

Intended outcome:

I would expect the onCompleted callback to be fired after every poll.

Actual outcome:

onCompleted is being fired only once. First time the query is made.

How to reproduce the issue:

const { data, stopPolling } = useQuery(QUERY, {
  variables: {...},
  fetchPolicy: 'network-only',
  pollInterval: 1000,
  onCompleted: () => console.log('called')
})

Versions

npmPackages:
@apollo/react-common: ^3.0.1 => 3.0.1
@apollo/react-hooks: ^3.0.1 => 3.0.1
apollo-cache-inmemory: ^1.3.5 => 1.3.11
apollo-client: ^2.6.4 => 2.6.4
apollo-link: ^1.2.3 => 1.2.4
apollo-link-context: ^1.0.10 => 1.0.10
apollo-link-error: ^1.1.1 => 1.1.2
apollo-link-http: ^1.5.5 => 1.5.7
apollo-link-logger: ^1.2.3 => 1.2.3
apollo-server-koa: ^2.1.0 => 2.2.4
apollo-utilities: ^1.3.2 => 1.3.2
react-apollo: ^2.2.4 => 2.3.2

[ajhool]

Have you checked the network traffic? I believe that it does not actually poll, as opposed to executing onComplete only on the first query.

[kocur4d]

@ajhool I have just tested it again and I can confirm that the query is being called multiple times. I can see it in the network traffic every second, but the onCompleted is being called only once.

[jaredmdobson]

Yeah i'm seeing the same thing.

        "apollo-cache": "^1.3.2",
        "apollo-cache-inmemory": "^1.6.3",
        "apollo-client": "^2.6.4",
        "apollo-link": "^1.0.6",
        "apollo-link-error": "^1.0.3",
        "apollo-link-http": "^1.3.1",
        "graphql-tag": "^2.4.2",
        "ts-invariant": "^0.4.0",
        "tslib": "^1.9.3"

[ScripterSugar]

+1 Having same problem here. onCompleted will be fired only once at the first fetch.

[maddog986]

+1 Im also seeing the same issue

[mamousavi]

Setting notifyOnNetworkStatusChange to true solved the issue in my case.

[bhishp]

This could be the culprit?

apollo-client/src/react/data/QueryData.ts

Line 433 in 6bc9fdc

// No changes, so we won't call onError/onCompleted.

It seems onCompleted only runs when data has changed, so if you are polling and no data changes occur then it will not fire.

Ideally there could be an extra prop to always call. Or maybe an alternative function prop, onCompletedAlways?

[bhishp]

A workaround for our case was to add fetchPolicy: 'no-cache', to the query options. Thus:

const { data, stopPolling } = useQuery(QUERY, {
  variables: {...},
  fetchPolicy: 'network-only',
  pollInterval: 1000,
  onCompleted: () => console.log('called'),
  fetchPolicy: 'no-cache',
})

Obviously this means you will bypass the client-side cache but it will ensure the completion hook is triggered every time.

[GollyJer]

We've tried the workarounds here but none work.

We don't see onCompleted firing at all with @apollo/client 3.0.0-beta.37.
Even the first call to the query doesn't print to the console.

  const messagesQuery = useQuery(GET_CHAT_MESSAGES_BY_GROUP_ID, {
    variables: { chatGroupId },
    pollInterval: 1000,
    onCompleted: () => console.log('If this worked no useEffect needed. 😕'),
  });

[jcelliott]

I'm running into this issue as well. Even with fetchPolicy: 'no-cache' the onCompleted handler is only being called once.

[marcdavi-es]

The same is true for onError. It's called for the first error but is not called if later poll attempts have errors. The workarounds do not help me.

[3nvi]

Can confirm as well

[andreasonny83]

Same behaviour here

[dominik-myszkowski]

Guys, set the fetchPolicy: 'network-only', it should work then, I had the same problem. And better switch to: useLazyQuery instead of polling if it is possible.

[3nvi]

Guys, set the fetchPolicy: 'network-only', it should work then, I had the same problem. And better switch to: useLazyQuery instead of polling if it is possible.

Erm sorry but that's subjective. I need to store it in the cache since it's very expensive for me to re-fetch it. I need polling since I'm using it for an async server operation that has the results ready in between 30secs and 2 minutes, thus I need to continuously "check" if they are ready for serving

[andreasonny83]

Also, @dominik-myszkowski , setting fetchPolicy: 'network-only' only triggers the onCompleted once. I can only get it working by setting notifyOnNetworkStatusChange: true

[bhishp]

This could be the culprit?

apollo-client/src/react/data/QueryData.ts

Line 433 in 6bc9fdc

// No changes, so we won't call onError/onCompleted.

It seems onCompleted only runs when data has changed, so if you are polling and no data changes occur then it will not fire.

Ideally there could be an extra prop to always call. Or maybe an alternative function prop, onCompletedAlways?

Are any of you falling in to this conditional? Is it not firing when no data has changed?

[andreasonny83]

@bhishp I would really love the onCompeted event only to run on data changed. however even when the data changes, I cannot see the onCompeted method triggered

[krishnaku]

Setting notifyOnNetworkStatusChange to true solved the issue in my case.

This worked for me. Setting 'network-only' did not.

[3nvi]

@hwillson any updates on that with the release of 3.0?

[martdavidson]

@andreasonny83 I'm seeing the same, the data changed but my onComplete didn't fire. notifyOnNetworkStatusChange fixed the issue for me. Still happening even in version 3.1.0-pre.1

[jure]

The issue with setting notifyOnNetworkStatusChange is that it will rerender for every poll interval (as documented). This is probably not what you want and you'd want to rerender only when the data changes.

[martdavidson]

@jure Yes, but as @andreasonny83 mentioned and I've also confirmed, it isn't firing when the data changes.

[jure]

Right, absolutely, it should! That's the bug. I've commented merely to point out that setting notifyOnNetworkStatusChange isn't a workable workaround in many situations and that folks should be aware of that drawback before applying it willy nilly, as it causes the whole tree below the hook to rerender on every interval.

[jure]

For what it's worth, I've sort of resolved the issue caused by this workaround for the time being by chucking the polling useQuery into a dead end of the tree, so the rerendering isn't annoying. In there I then use makeVar which is then used in the typePolicies of the InMemoryCache, like this:

  {
    cache: new InMemoryCache({
      typePolicies: {
        Manuscript: {
          fields: {
            _currentRoles: {
              read(existing, { cache, args, readField }) {
                const currentRoles = currentRolesVar()
              },
            },
          },
        },
      },
    }

It's quite the detour, but it works, so hopefully it's useful for someone else too.

[DogAndHerDude]

Yup, can confirm. Still happening in 3.3.6. I resolved the issue with notifyOnNetworkStatusChange , though it's like using a baseball bat to clean the dishes.

The only other alternative I can come up with is using a useEffect, and refetch.

[doflo-dfa]

This has proven to be the most reliable combination for us. note the separate user queries and the manual starting and stoping of polling. this bypasses a second bug in which stop polling does not reliably work with pollinterval

  const [completed,setCompleted] = useState(false);

  const { data: versionData, loading: versionLoading, stopPolling, startPolling } = useQuery(QUERY, {
    variables: {
      id: id,
    },
  })

useEffect(() => {
    if (versionData) {
        ...
    }, [versionData])

useEffect(() => {
    // versionRefetch()
    if (!completed) {
      startPolling(4000)
    } else {
      stopPolling()
    }
    return () => {
      stopPolling()
    }
  }, [stopPolling, startPolling, completed])
  

[hwillson]

This should now be resolved in @apollo/client@3.5.0. Let us know if you notice any issues. Thanks!

[3nvi]

I don't think this is fixed @hwillson

Here's a codesandbox repro. Making notifyOnNetworkStatusChange: true fixes it (as it always did), but when it's false, the original issue still exists.

Should this be re-opened?

[d4v1d41]

I don't think this is fixed @hwillson

Here's a codesandbox repro. Making notifyOnNetworkStatusChange: true fixes it (as it always did), but when it's false, the original issue still exists.

Should this be re-opened?

Same, I just tried with version 3.5.6, still not working. notifyOnNetworkStatusChange: true does the trick though.

[adamszeptycki]

I have the same problem with latest version (for me it happens both on useMutation and useQuery - two different use cases)

[tonyjmnz]

This issue persists in 3.5.8.

It's baffling how issues like this can go multiple years without being addressed in such a widely used library... And this is not the first one I've seen.

I suggest reopening this issue.

[eduardogch]

I confirmed that the issue persists in the current latest version 3.5.8 even with the option notifyOnNetworkStatusChange: true

My work around to called onCompleted and solved this issue. It was to implement a timer or setInterval in a useEffect and removed the pollInterval option from the query. Example:

// From a useQuery
const { data, refetch } = useQuery(QUERY, {
  variables: {...},
  onCompleted: () => console.log('called'),
})

// From a useLazyQuery
const [getData, { error, loading }] = useLazyQuery(QUERY, {
  variables: {...},
  onCompleted: () => console.log('called'),
})

useEffect(() => {
    const interval = setInterval(() => {
      refetch() // Refetch useQuery
      getData(); // Called useLazyQuery
    }, 5000);
    return () => clearInterval(interval);
  }, []);

[andreasonny83]

@akikoskine, @brainkim, can we reopen this issue?

[squarewave24]

same issue, in addition to onCompleted, my react table is not reflecting changes even though i see the calls in network. notifyOnNetworkStatusChange is making everything re-render on each poll so that's not really a solution i can use.

[imorad87]

My work around:

const { loading, error, data, startPolling, stopPolling } = useQuery(GET_ALL_CAMPAIGNS)

useEffect(() => {
startPolling(1000);
return () => stopPolling();
}, []);

[alessbell]

Hi all - after taking a look at @3nvi's codesandbox (thanks for providing that!), onCompleted only fires once because the data never changes. So while the network requests are firing in the background, the cache remains in the same state as after the initial fetch. In this case, we do not fire onCompleted unless notifyOnNetworkStatusChange is set to true (but as others have noted, this also updates the loading state).

However, if the selection set observed by your query is being updated in your cache, onCompleted should be firing (even without notifyOnNetworkStatusChange: true), so any reproductions to the contrary would be helpful.

If anyone has any feedback on pollInterval + onCompleted, feel free to send them my way! I'm taking a look at this issue as part of #10229

[mogelbrod]

Glad to hear that you're looking into this @alessbell!

Not being able to reliably subscribe/listen for (refetches/polling) responses via onCompleted without also setting notifyOnNetworkStatusChange: true has lead to our codebase having to wrap the returned query.refetch() function (and any other functions that could trigger a new request) with something like this to attempt to intercept requests that do not update the cache:

function useQueryVariation(...) {
  const context = React.useRef()
  context.current = {
    onCompleted(data) {...}
  }

  const query = useQuery(..., {
    onCompleted: context.current.onCompleted,
  })

  return {
    ...query,
    refetch: React.useCallback(() => {
        return query.refetch().then(result => {
          context.current.onCompleted(result.data)
          return result
        })
      }, [query.refetch])
  }
}

To me this doesn't feel like a maintainable solution (the workaround doesn't capture refetches triggered in other ways) but rather like trying to reverse an implementation detail.

The name onCompleted (as well as the docs) suggest that it would be triggered whenever the hook completes (data is retrieved?), whereas the current behaviour would be a better fit for a onUpdated callback or similar. Perhaps there's an argument for having both, if the currently implemented behaviour is sometimes desired?

[tathagat2000]

@alessbell @hwillson @ajhool
Please consider this useCase.

fetchPolicy is cache-and-network.
Query goes and data is present in cache. onCompleted is called.

Simultaneously Network call goes (Because fetchPolicy is cache-and-network). In the network call, data has changed which causes cache to update BUT, onCompleted will never be called with this new data. Isn't this not a valid usecase where onCompleted should have been called?

Additional:
onCompleted won't be called the second time because of this check -> previousResult?.networkStatus !== result.networkStatus. networkStatus doesn't change in our above useCase when cache is revalidated with network request.

Please check

[ColonelThirtyTwo]

Bumping this because I ran into it. I was using pollInterval to check on a job, and set it to zero once the job is done (since there won't be any updates afterwards). I have fetchPolicy: "no-cache" and the data I fetch definitely changes, but onCompleted only runs once. As other have mentioned, notifyOnNetworkStatusChange: true causes onCompleted to rerun.

The fact that onCompleted runs only once is unintuitive and undesireable. The fact that its fixed by setting a seemingly unrelated option notifyOnNetworkStatusChange changes the behavior is very unintuitive.

[Haegin]

I'm on an old version of Apollo Client so some of this may not be relevant to the current code, but when running into this I also found that when using polling if I added a useEffect to monitor the returned data from the query that wasn't being run even if the data in the query was changing. I eventually got it working by manually implementing polling with setInterval and refetch, using a useEffect to compare the results. Posting here in case that helps anyone else and in case the issue is more generally related to polling, not just to the onCompleted callback.