useQuery loading state stays true after fetch finishes

raduflp commented 4 years ago

There is an issue with useQuery loading state remaining true if a cached query result is displayed and the subsequent variable changes return the same result as the cached one.
See codesandbox for the steps to reproduce.

changing the fetchPolicy to network-only or cache-and-network fixes the issues This issues appeared after beta-46 refactor, works as expected in beta-45

Intended outcome: the loading reverts to false once the fetch is finished

Actual outcome: the loading stays true until the you change the query variable so that the query returns results that are different from the cached one being displayed .

How to reproduce the issue: https://codesandbox.io/s/nifty-ptolemy-1308j?file=/src/App.js

Versions System: OS: Windows 10 10.0.19041 Binaries: Node: 13.8.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.4 - C:\Program Files\nodejs\yarn.CMD npm: 6.13.6 - C:\Program Files\nodejs\npm.CMD Browsers: Edge: 44.19041.1.0 npmPackages: @apollo/client: ^3.0.0-beta.49 => 3.0.0-beta.50 @apollo/link-error: ^2.0.0-beta.3 => 2.0.0-beta.3 @apollo/link-retry: ^2.0.0-beta.3 => 2.0.0-beta.3

hueter commented 4 years ago

This issue is a blocker for my team upgrading -- we're currently stuck on beta-44. I reproduced this on beta-54 just now; however changing the fetchPolicy did not help in my case.

This issue is especially notable on our app's "dashboard" view which features about a dozen queries firing simultaneously. On the upgraded version branch, some of the queries work fine but several of them just spin forever because loading is never set to false despite the new data coming back, as @raduflp reports

julian-sf commented 4 years ago

This seems to be a recurring issue. The following issues seem to be related:

I know these are all links from the deprecated repo, but my point is that this issue is CONSTANTLY happening. What gives? How is this such a problem?

Let me explain our scenario so you can see why this issue is so bonkers. We make three requests for paginated data. One for the previous page, current page, and next page. When you deep link into the application and it does it's initial render/load everything works out fine and the pages load.

Example of requests:

request	result
/api/data?page=0	`[A, B, C]`
/api/data?page=1	`[D, E, F]`
/api/data?page=2	`[G, H, I]`

If you add another query parameter, of course, the hooks fire off new network requests. Like so:

request	result
/api/data?page=0&foo=true	`[A, B, C]`
/api/data?page=1&foo=true	`[U, V, W]`
/api/data?page=2&foo=true	`[X, Y, Z]`

Note that the first request returned the same data both times: [A, B, C]. This is where this bug is such a problem. Since the result was the same, even though the query wasn't, the loading state never cycles. So because of this bug, our app would be stuck in an infinite loading state because the data from TWO DIFFERENT REQUESTS happened to have the same result. How the hell can this happen, like from a code perspective? What gnarly-ass cache keying is going on?

Riddle me this: why is the cache not using a different key when we change the query parameters? Why does the result of one request with one set of query params affect the loading state of a request with another set of query params? How is that even a thing?!?!

Sorry for the 🧂. This bug has been a real PITA lately. I'm using this venting as therapy. Don't take this personally. I love you all.

julian-sf commented 4 years ago

Note on the above: I'm more than happy to help out with updating the cache key logic, but a quick tip on where to start looking would be appreciated.

hboylan commented 4 years ago

We're encountering this issue on RC v3.0.0-rc.0. In our case, the variables change on every request, but the 4th or 5th query starts continually returning loading=true.

If it helps, this state appears to trigger when results are identical to that of previous queries as @julian-sf mentioned.

Unfortunately having to use fechPolicy: 'no-cache' to resolve this for now.

const { data, loading } = useQuery(GET_DELIVERIES, {
  fetchPolicy: 'no-cache',
  variables: {
    params: {
      ...baseParams,
      ...sortParams,
      searchQuery: query,
      limit: pageSize,
      offset: (page - 1) * pageSize,
    },
  },
})

hueter commented 4 years ago

Updates for me:

fetchPolicy: 'no-cache' is working now too on 3.0.0-rc.0
fetchPolicy: 'network-only' fails
the affected queries are susceptible to returning the same data with different variables

This corroborates what others in this thread have said.

I actually reproduced the issue closest to my use case in this sandbox:

https://codesandbox.io/s/autumn-darkness-8b69l

(check the console for more info in there)

tpict commented 4 years ago

I think there's a related but possibly separate issue in rc.0. For a very simple query (just swapped out field names, structure is the same):

export interface QueryType {
  result: string[];
}

const { data, loading } = useQuery<QueryType>(THE_QUERY);
console.log(data, loading);

we get the following console messages:

    undefined true                // expected
    { result: [ 'data' ] } false  // expected
    { result: [ 'data' ] } true   // now it's loading again?
    { result: [ 'data' ] } true   // test stalls and fails at this point because loading is never unset

The query is never refetched or anything, and there's no changing query variables.

Changing the fetch policy to no-cache works on rc.0. This behavior is not present in beta.54

jeremymcclelland commented 4 years ago

Same issue here. Using Next.js app. When initially spinning up app > loading TRUE > loading FALSE > DATA. Then when navigating to another page and back again > Loading TRUE, Loading TRUE, Loading TRUE, etc. Even when its a cached query with data ready to go. Got around it this way:

if (loading && !data) return <p>Loading...</p>;

But, still....

eDubrovsky commented 4 years ago

Have the same problem in 3.0.0-rc.0. with a simple query:

query GetProjects {
    app_project(order_by: {created_at: desc}) {
        id
        title
    }
}

After first load loading=true then loading=false. Open another page and back, query not changed, but loading=true again.

In 3.0.0-beta.50 all work correctly.

eisnstein commented 4 years ago

I experience exactly the same behavior with 3.0.0-rc.0. I did a little investigation and could somehow solve it, but it is not a solution, just could show the origin of the misbehavior:

on line 227 in node_modules/@apollo/client/react/data/QueryData.js I commented out the if statement so that the onNewData() function always gets called
restart app
works like expected

see that commit: https://github.com/apollographql/apollo-client/commit/c76804eea2afcd3df4e01d5a8db3ae2c4c553aee

eDubrovsky commented 4 years ago

Some update: in my case loading=true again if query return empty data like:

app_project: []

and my query in cache empty and looks like:

app_project({"order_by":{"created_at":"desc"}}):

If my array is not empty - all work fine.

hwillson commented 4 years ago

Thanks for the great reproduction and details here. This issue is being caused by the way we snapshot and track differences between results, to help prevent unnecessary computations. For example, removing the isDifferentFromLastResult call from this line fixes the issue:

https://github.com/apollographql/apollo-client/blob/2d61e5c936c2bcb2aa62f4eb7c6f82b773b962ab/src/core/ObservableQuery.ts#L594

We'll tweak the snapshot approach a bit, and have a fix for this out shortly.

joni7777 commented 4 years ago

Any solution?? On 3.0.0-rc.2 still happening even with no-cache

redmundas commented 4 years ago

I have a similar issue. I'm using graphql hoc from @apollo/react-hoc@4.0.0-beta.1 and I get loading: true for my @client queries. reverting back to @apollo/client@3.0.0-beta.50 fixed the issue for me

hboylan commented 4 years ago

reverting back to @apollo/client@3.0.0-beta.50 fixed the issue for me

@edmundas-ramanauskas This version does not work for me.

This issues appeared after beta-46 refactor, works as expected in beta-45

I can confirm this version works as recommended in OP.

hwillson commented 4 years ago

Hi all - @apollo/client@3.0.0-rc.3 should address this. Let us know if not - thanks!

riccoski commented 4 years ago

Hi all - @apollo/client@3.0.0-rc.3 should address this. Let us know if not - thanks!

Thanks for the update. I have upgraded but I'm still getting the same issues with useLazyQuery. loading remains on true and onCompleted is never fired.

Thanks

hwillson commented 4 years ago

@riccoski would you mind providing a small runnable reproduction that shows the problem?

riccoski commented 4 years ago

@riccoski would you mind providing a small runnable reproduction that shows the problem?

I have created one here: https://codesandbox.io/s/quiet-sea-hj7es?file=/src/App.js

When you click 'Get data' the loading state persists and the onCompleted doesn't fire.

Thanks

hwillson commented 4 years ago

Thanks @riccoski - this appears to be a different issue. If you comment out the fetchPolicy line it works properly, which is different than the original issue here. Would you mind opening a new issue about this, with the same reproduction? Thanks again!

riccoski commented 4 years ago

Will do! Thanks

On Thu, 11 Jun 2020 at 11:55, Hugh Willson notifications@github.com wrote:

Thanks @riccoski https://github.com/riccoski - this appears to be a different issue. If you comment out the fetchPolicy line it works properly, which is different than the original issue here. Would you mind opening a new issue about this, with the same reproduction? Thanks again!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apollographql/apollo-client/issues/6334#issuecomment-642569196, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEA4EYGRGUTKAIZUGV53A3RWCZZVANCNFSM4NIC4F2Q .

-- Ricco

Frontend developer

ThingCoDanW commented 4 years ago

Updated to rc4 and noticed useQuery was not behaving correctly again - loading always on true when data coming from cache. Downgraded to rc3 and it appears to be working fine.

Edit - spoke too soon, rc3 works sometimes but hits the same issue after a few loads!

blackrock33 commented 3 years ago

Just FYI. (maybe not related) I use useQuery hook in React and loading always equals true on the console while fetch data finished. I found it because I pass the sub value of res to the child component. So I pass the whole res to the child component and loading turns to false. Everything works fine.

MilanBarande commented 2 years ago

I still experience this issue on 3.3.15, had to use the fetchPolicy: 'no-cache' trick for it to work 😕

MAK-Denis commented 2 years ago

Is this issue solved? I'm experiencing the same issue in version 3.6.0, and I don't see the solution here. Anyone please message here the solution.

senghuotlay commented 2 years ago

@MAK-Denis I am also experiencing the same issue on React version 17.0.1 and apollo/client version 3.6.1, any update please

senghuotlay commented 2 years ago

I somehow found the solution for it, I believe that versino 3.6.0 is very much catered for react 18, which in my case expo's sdk only support up to react 17, therefore using apollo/client: 3.5.4 worked, along with "graphql": "^16.3.0". I hope that helps for u too @MAK-Denis

davidarthurthomas commented 2 years ago

I somehow found the solution for it, I believe that versino 3.6.0 is very much catered for react 18, which in my case expo's sdk only support up to react 17, therefore using apollo/client: 3.5.4 worked, along with "graphql": "^16.3.0". I hope that helps for u too @MAK-Denis

@dalley8626 you dirty dog. just saved me days of confusion

apollographql / apollo-client

useQuery loading state stays true after fetch finishes #6334