Apollo-client: Query Functionality with `onCompleted` is broken in v3.0.0 release (CodeSandbox repro)

[3nvi]

Some core lazy querying functionality through useLazyQuery seems a bit off.

Specifically, when a lazy query (through a useLazyQuery hook) returns an error, then Apollo seems to start endlessly querying the server. The same thing happens if you give it a fetchPolicy of network-only (or anything that doesn't search the cache first).

Related: https://github.com/apollographql/react-apollo/issues/4044

At the same time, if the lazy query function (returned by useLazyQuery) gets called more than 1 times, then no callbacks are fired. The onCompleted doesn't fire again. The onCompleted fires on the 1st successful run of the "query" function and then consecutive invocations of the lazy query function, don't trigger onCompleted callback (they should, since you can't have conditional business logic based on whether the data is cached or not!)

Related: https://github.com/apollographql/react-apollo/issues/4000

Funnily enough, using client.query() works fine. All this used to work perfectly during the beta (I think I've verified that something occurred after @apollo/[email protected]), but now it doesn't.

UPDATE: I've added a codesandbox that reproduces it

Intended outcome:

• Lazy query errors shouldn't trigger endless API querying
• A network-only fetch policy shouldn't trigger endless API querying on a lazy query
• onCompleted should fire every time a lazy query gets successfully executed

Actual outcome:

• Lazy query errors & network-only fetch policy start an endless loop of requests
• onCompeted gets fired at most once regardless of the number of invocations

How to reproduce the issue:

• Take a lazy query and change the fetchPolicy to network-only or force an API error on a lazy query
• Click a button that triggers a lazy query multiple time, while observing the number of invocations of the onCompleted callback.

Versions
System:
OS: macOS 10.15.2
Binaries:
Node: 12.18.0 - /usr/local/bin/node
Yarn: 1.19.0 - /usr/local/bin/yarn
npm: 6.14.4 - /usr/local/bin/npm
Browsers:
Chrome: 83.0.4103.116
Firefox: 72.0.2
Safari: 13.0.4
npmPackages:
@apollo/client: ^3.0.2 => 3.0.2
apollo-link-error: ^1.1.12 => 1.1.13

[3nvi]

@benjamn Take a look here and observe the console

https://codesandbox.io/s/apollo-reference-equality-1exes?file=/src/App.js

While fetchPolicy is network-only React re-renders endlessly. If you make it cache-first things are ok again. Same thing would happen if the query errored.

Also observe how the ON COMPLETED text gets only logged once regardless of how many times the query finishes successfully

[3nvi]

I noticed that even if I change the variables after one query execution, the query automatically executes and fires its onCompleted (although it's lazy, so it shouldn't).

Related to https://github.com/apollographql/react-apollo/issues/3729

In general, this seems like a bug that may be unrelated to lazy querying, but I feel it's something on the "core" functionality of querying since it manifests in many different ways

[kainanaina]

I'm using 3.0.0-beta.43 since February and everything worked there just fine. Tried an upgrade to stable v3 and encountered this issue.

[orrybaram]

I've noticed that this is also happening with a regular useQuery hook when an onCompleted function is specified (even if its a noop) and the fetchPolicy is set to anything except cache-first or cache-only

[3nvi]

I've noticed that this is also happening with a regular useQuery hook when an onCompleted function is specified (even if its a noop) and the fetchPolicy is set to anything except cache-first or cache-only

Yeah I also mentioned that I feel that this is not necessarily tied only to lazy queries. Will update description

[Marcelo-Figueiredo]

Same problem. Endless request loop when using onCompleted in one of my applications after upgrading. Might go back to apollo-boost for now.

[hwillson]

This issue was fixed in https://github.com/apollographql/apollo-client/pull/6588, and will be coming in @apollo/[email protected] very soon.

[benjamn]

Heads up: we're not quite ready to publish @apollo/[email protected] yet, but I've published an @apollo/[email protected] release that includes #6588, if you want to try that in the meantime. 🙏

Update: thanks to @3nvi's reproduction, I can confirm the fix: https://codesandbox.io/s/apollo-reference-equality-t0ttx

[3nvi]

Heads up: we're not quite ready to publish @apollo/[email protected] yet, but I've published an @apollo/[email protected] release that includes #6588, if you want to try that in the meantime. 🙏

Update: thanks to @3nvi's reproduction, I can confirm the fix: https://codesandbox.io/s/apollo-reference-equality-t0ttx

Perfect. Thanks a lot @benjamn !

[emjaksa]

@benjamn I just tried the latest version @apollo/[email protected] and @apollo/[email protected]. When I try execute the search below the query gets stuck in an infinite loop, making multiple network requests and never calling onComplete. If I comment out either the fetchPolicy or onCompleted it stops but the combination of both fetch fetchPolicy: 'no-cache' and onCompleted: data => {} cause the query to go into an infinite loop.

I tried replicating it in your sandbox but I can only replicate it with @apollo/[email protected].

const [search] = useLazyQuery(SearchQuery, {
    fetchPolicy: 'no-cache',
    onCompleted: data => {},
  })

const handleClick = () => { 
  search({
    variables: {
       query: 'search',
    }
  }) 
}

[Tautorn]

@apollo/[email protected]

Thank you so much @benjamn ! It's working for me, great!

[cyberwombat]

@benjamn I just tried @apollo/[email protected] and it's still a no go. This issue might be slightly different than the 2 year old one (auto archived without fix and reopened by me https://github.com/apollographql/react-apollo/issues/3968 then repo archived). But the gist for me is that onCompleted doesn't fire again rendering the concept of a cache useless. I am a bit confused how anyone is able to use a cache as we never have been able to. @Tautorn you say its working for you - is the cache enabled? how are you adding business logic when refetching from cache?

[emjaksa]

@apollo/[email protected] worked for me. I had to delete the package-lock.json and run npm install again. Very odd

[Tautorn]

@benjamn I just tried the latest version @apollo/[email protected] and @apollo/[email protected]. When I try execute the search below the query gets stuck in an infinite loop, making multiple network requests and never calling onComplete. If I comment out either the fetchPolicy or onCompleted it stops but the combination of both fetch fetchPolicy: 'no-cache' and onCompleted: data => {} cause the query to go into an infinite loop.

I tried replicating it in your sandbox but I can only replicate it with @apollo/[email protected].

const [search] = useLazyQuery(SearchQuery, {
    fetchPolicy: 'no-cache',
    onCompleted: data => {},
  })

const handleClick = () => { 
  search({
    variables: {
       query: 'search',
    }
  }) 
}

Hi there.

The cache is enabled, but I don't use fetchPolicy to get any result.

I'm using onCompleted after query. Before I updated to @apollo/[email protected] was necessary wrapper useQuery and useLazyQuery with useCallback.

But now working normally (after update version).

[zeeshanaligold]

const { loading, error } = useQuery(GET_USER, {
    onCompleted({ user }) {
      setAuth({ user, isAuthenticated: true })
    },
})

Still getting stuck with infinite loop of requests to api. it's happening only when i call setAuth to save state with context api.

[orrybaram]

@zeeshanaligold try adding a flag to check for the user in the onCompleted method. You may be accidentally retriggering the infinite loop yourself by setting state on each render (which cause it to re-render again)

onCompleted({ user }) {
  if (!authedUser) {
    setAuth({ user, isAuthenticated: true })
   }    
}

[Tautorn]

const { loading, error } = useQuery(GET_USER, {
    onCompleted({ user }) {
      setAuth({ user, isAuthenticated: true })
    },
})

Still getting stuck with infinite loop of requests to api. it's happening only when i call setAuth to save state with context api.

@zeeshanaligold try with useCallback:

import { useCallback } from 'react'
...

const handleOnCompleted = useCallback(({ user }) => {
    setAuth({ user, isAuthenticated: true })
}, [])

const { loading, error } = useQuery(GET_USER, {
    onCompleted: handleOnCompleted
})

[zeeshanaligold]

@zeeshanaligold try adding a flag to check for the user in the onCompleted method. You may be accidentally retriggering the infinite loop yourself by setting state on each render (which cause it to re-render again)

onCompleted({ user }) {
  if (!authedUser) {
    setAuth({ user, isAuthenticated: true })
   }    
}

@orrybaram it's fix the infinite loop issue but still sending 2 requests.

One more issue when logout trigger & authedUser will set to false then again new request will go to graphql

[zeeshanaligold]

import { useCallback } from 'react'
...

const handleOnCompleted = useCallback(({ user }) => {
    setAuth({ user, isAuthenticated: true })
}, [])

const { loading, error } = useQuery(GET_USER, {
    onCompleted: handleOnCompleted
})

@Tautorn i did try it but still same issue.

[Tautorn]

@zeeshanaligold Could you reproduction this issue in some tool like codesandbox? If possible I hope.

[FezVrasta]

Does 3.1.1 include this fix?

[GeorgeMarcus]

Not fixed on 3.1.1. It still only works by wrapping onCompleted and onError in useCallback.

[emjaksa]

3.1.1 is working for me

[3nvi]

Not fixed on 3.1.1. It still only works by wrapping onCompleted and onError in useCallback.

Ι haven't tested it yet, but make sure the you reset this particular node_module and that package-lock.json correctly references the installed version

[3nvi]

@benjamn and @hwillson Thanks for tackling this.

Current State of things

As of the moment of writing, in 3.1.1:

• [x] An errorred query no longer re-renders endlessly 🎉
• [x] A cache-first query with onCompleted and/or onError no longer re-renders endlessly 🎉
• [x] A network-only query no longer pings the server endlessly 🎉

Pending bugs

• [ ] The onCompleted callback should always fire whenever a query is resolved (either from network or cache). https://github.com/apollographql/react-apollo/issues/4000

Currently only network-only requests have their onCompleted fired after every query execution. For cache-first ones, the onCompleted fires only once. Unless this is documented somewhere, I would flag this as a bug since our onCompleted logic shouldn't have to account for the origin of this data, i.e. It should be cache agnostic

• [ ] Changing a query variable on a lazy query should not automatically fire it. https://github.com/apollographql/react-apollo/issues/3729

This should definitely happen for useQuery hooks, but not for useLazyQuery hooks (since they wouldn't be lazy anymore). Again if this is the expected functionality it should ideally be documented

Reproduction of pending bugs

You can observe both these bugs in the following codesandbox --> https://codesandbox.io/s/apollo-reference-equality-op12z?file=/src/App.js

• To reproduce the onCompleted error: Change fetchPolicy to cache-first, click "Run Query" multiple times and see how onCompleted only fires once

• To reproduce the variables with useLazyQuery error: Click "Run Query" once and then start clicking "Change foo". Clicking "change foo" should not cause a query to run

Additional Context

Out of the two bugs listed above (in the order that they are mentioned):

• The first is within the scope of this ticket and a requirement in order to close it
• The second isn't necessarily related to this ticket (although mentioned) and I understand that it may require a more complex solution through a modification of your query observables. Thus, I'm willing to port (2) to another issue in order to help close the existing one

Thanks again!

[uncvrd]

I don't think I've seen this mentioned yet as it probably comes from the same underlying behavior...but a useLazyQuery with a onCompleted does not get called again if a user specifies a refetchQueries array in a useMutation method call.

Such as

const [updateAlbumReadStatus] = useMutation({
        // this triggers a network refetch but does not call the onComplete() of my UnreadAlbums useLazyQuery
        refetchQueries: ["UnreadAlbums"]
    });

[lenarmazitov]

@benjamn

Problem still exists in 3.1.1
for fetchPolicy: "cache-and-network"

(with fetchPolicy: "network-only" all works fine)

[warent]

I'm on 3.1.1 and onCompleted never calls if a useLazyQuery has its result cached.

[benjamn]

Those of you having problems with cache-and-network or network-only making unwanted network requests (especially if cache-first seems to fix things), please read this comment about the new options.nextFetchPolicy option introduced in v3.1.0: https://github.com/apollographql/apollo-client/issues/6760#issuecomment-668188727

[gtb104]

I believe I'm still seeing this on 3.1.3

  const client = new ApolloClient({
    cache: new InMemoryCache(),
    defaultOptions: {
      watchQuery: {
        fetchPolicy: 'cache-and-network',
        nextFetchPolicy: 'cache-only' // This didn't fix anything
      }
    },
    headers: {
      Authorization: authAccessToken(),
      Accept: 'application/json, text/plain'
    },
    uri: url
  });

I use that client like so

  const { data, error, loading } = useQuery(gql(queries.listDevices), {
    client,
    variables: {
      tenantId: tenantId()
    }
  });

I can see that it's constantly making network requests, all of which are returning data. Just seems like it can't get out of the loading state. When I log out data, error, and loading, what I see repeated is:

data undefined
error undefined
loading true

[finchen]

I had the same issue with "@apollo/client": "^3.1.3" (loading always false and no callback).

I found out that the variables must use react state.
Following code keeps loading always true and never goes to onCompleted callback:

const { loading, error, data } = useQuery(LOGS, {
  variables: {
      dateFrom: new Date().toISOString()
    }
}

Following code works but still have to add notifyOnNetworkStatusChange: true for the onCompleted callback:

const [dateFrom, setDateFrom] = useState(new Date().toISOString());
const { loading, error, data } = useQuery(LOGS, {
  notifyOnNetworkStatusChange: true,
  variables: {
      dateFrom
    }
}

[gtb104]

I solved my specific issue. I was creating the client within the component, so it was recreating the client each time the results were returned. I changed it to:

  const client = useRef(getClient());

  const { data, error, loading } = useQuery(gql(queries.listDevices), {
    client: client.current,
    variables: {
      tenantId: tenantId()
    }
  });

The getClient function does the new ApolloClient stuff from above.

[benjamn]

@gtb104 There's a context hook for that! See useApolloClient, which works if you're using ApolloProvider somewhere further up your component tree.

[nodabladam]

I just discovered that after upgrading from apollo 2 to 3.1.5 that my useLazyQuery are not always firing the onCompleted. Current workaround for me seems to be changing usage of useLazyQuery to:

client.query({ query: <QUERY> }).then(res =>{ ... } );

[adrienharnay]

Those of you having problems with cache-and-network or network-only making unwanted network requests (especially if cache-first seems to fix things), please read this comment about the new options.nextFetchPolicy option introduced in v3.1.0: #6760 (comment)

For this bug: Changing a query variable on a lazy query should not automatically fire it., I don't think options.nextFetchPolicy can be of any help, and I believe there's no workaround.

[benjamn]

@3nvi I believe both of the problems demonstrated in your reproduction are related to the fact that useLazyQuery uses client.watchQuery behind the scenes, when it should just be using client.query:

• Refetching a network-only query when a variable changes is expected if the query is being watched, but would not happen if the query only executes when explicitly requested.
• The problem with onCompleted not firing is more loosely related, but I'm sure that's something we could fix in the process of rewriting useLazyQuery to avoid watching, since we should be able to hook into the Promise returned by client.query.

Although we're about to release v3.2.0 with the changes that are currently on the release-3.2 branch, we haven't forgotten about this issue. We hope to include a fix in a 3.2.x (or a 3.3 beta) version soon.

[mschipperheyn]

@benjamn I have a problem with useLazyQuery that I feel is similar and might be related to the cache. I execute a useLazyQuery based on an id and fragment that I know are already in the cache. The query never shoots off a network query (presumably bc the element is in the cache), loading never becomes true, neither the error key, but neither does it update with data key. The data key remains undefined. onCompleted is also never called. Setting the fetchPolicy to network-only or no-cache or setting notifyOnNetworkStatusChange: true has no effect. (tried from 3.1.5 to 3.2.4 + 3.3.0-beta.12)

[vsylvestre]

I also have an issue with useLazyQuery that I think might relate to this?

I have an ApolloClient instance that I use in my ApolloProvider that I need to reinitialize once in a while because a couple of settings change during the lifetime of the app (mostly HTTP headers and connection params). Therefore, instead of having the client be created outside the scope of my React tree, it's generated within a custom hook. However, it seems like every time there's a new value passed to ApolloProvider for the client prop, all queries using useLazyQuery are relaunched automatically -- and they seem to be launched with the latest variables they were called with. I would expect that they wouldn't be launched until there's another call to the functions provided by the useLazyQuery hooks, right?

Maybe there's something wrong with the way I'm reloading the client though, and that it shouldn't go through React updates. I couldn't find any documentation on it. Let me know!

[hwharrison]

The bug calling onCompleted looks like a long-running holdover from 2.x.
https://github.com/apollographql/react-apollo/issues/2177

[3nvi]

Although we're about to release v3.2.0 with the changes that are currently on the release-3.2 branch, we haven't forgotten about this issue. We hope to include a fix in a 3.2.x (or a 3.3 beta) version soon.

@benjamn Given the fact that 3.3.0 launched without it, is there a chance we can get an update on the state of things? Is it something that's within your priorities or should we try and work around this for the time being.

As always, I appreciate you' re juggling a lot of things, so thanks in advance

[adrienharnay]

As always, I appreciate you' re juggling a lot of things, so thanks in advance

:+1: on this, thanks for the work you're putting into this project!

[l222p]

Any update on this? I'm getting this error on version

"@apollo/client": "^3.2.5",
"apollo": "^2.31.1",

I found an _orthodox_ solution, implemented useEffect which acts as a _completed_ validator:

const { loading, data, error } = useQuery( MY_QUERY, {
      variables: { id },
    },
  );

Then my useEffect:

useEffect(() => {
    if (!data) return;

    // your onCompleted logic here

}, [data]);