Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useQuery loading state stays true after fetch finishes #6334

Closed
raduflp opened this issue May 22, 2020 · 27 comments · Fixed by #6417
Closed

useQuery loading state stays true after fetch finishes #6334

raduflp opened this issue May 22, 2020 · 27 comments · Fixed by #6417

Comments

@raduflp
Copy link

raduflp commented May 22, 2020

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

@hwillson hwillson self-assigned this May 26, 2020
@hwillson hwillson added this to the Release 3.0 milestone May 26, 2020
@hueter
Copy link

hueter commented Jun 2, 2020

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
Copy link

julian-sf commented Jun 2, 2020

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
Copy link

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
Copy link

hboylan commented Jun 3, 2020

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
Copy link

hueter commented Jun 3, 2020

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
Copy link
Contributor

tpict commented Jun 3, 2020

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
Copy link

jeremymcclelland commented Jun 3, 2020

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
Copy link

eDubrovsky commented Jun 4, 2020

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
Copy link

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: c76804e

@eDubrovsky
Copy link

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
Copy link
Member

hwillson commented Jun 4, 2020

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:

if (this.lastError || this.isDifferentFromLastResult(result)) {

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

@joni7777
Copy link

joni7777 commented Jun 9, 2020

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

@redmundas
Copy link

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
Copy link

hboylan commented Jun 9, 2020

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 added a commit that referenced this issue Jun 9, 2020
Issue #6334 exposed a problem where the `lastResult` mechanism
we use to prevent duplicate subscription notifications (when
data hasn't changed) can unintentionally block certain results
from propagating through Apollo Client. This leads to issues
like loading states not being updated properly, due to new
partial results looking similar to last results.

This commit ensures that last results are properly cleared out
when new partial results come in.

Fixes #6334.
hwillson added a commit that referenced this issue Jun 9, 2020
Issue #6334 exposed a problem where the `lastResult` mechanism
we use to prevent duplicate subscription notifications (when
data hasn't changed) can unintentionally block certain results
from propagating through Apollo Client. This leads to issues
like loading states not being updated properly, due to new
partial results looking similar to last results.

This commit ensures that last results are properly cleared out
when new partial results come in.

Fixes #6334.
hwillson added a commit that referenced this issue Jun 9, 2020
Issue #6334 exposed a problem where the `lastResult` mechanism
we use to prevent duplicate subscription notifications (when
data hasn't changed) can unintentionally block certain results
from propagating through Apollo Client. This leads to issues
like loading states not being updated properly, due to new
partial results looking similar to last results.

This commit ensures that last results are properly cleared out
when new partial results come in.

Fixes #6334.
hwillson added a commit that referenced this issue Jun 9, 2020
Issue #6334 exposed a problem where the `lastResult` mechanism
we use to prevent duplicate subscription notifications (when
data hasn't changed) can unintentionally block certain results
from propagating through Apollo Client. This leads to issues
like loading states not being updated properly, due to new
partial results looking similar to last results.

This commit ensures that last results are properly cleared out
when new partial results come in.

Fixes #6334.
@hwillson
Copy link
Member

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

@riccoski
Copy link

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
Copy link
Member

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

@riccoski
Copy link

@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
Copy link
Member

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
Copy link

riccoski commented Jun 11, 2020 via email

@ThingCoDanW
Copy link

ThingCoDanW commented Jun 15, 2020

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
Copy link

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
Copy link

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

@MAK-Denis
Copy link

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
Copy link

@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
Copy link

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
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.