Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

refetch() does not update loading to true #321

Closed
PascalHelbig opened this issue Nov 11, 2016 · 62 comments · Fixed by apollographql/apollo-client#4992 or #3339
Closed

refetch() does not update loading to true #321

PascalHelbig opened this issue Nov 11, 2016 · 62 comments · Fixed by apollographql/apollo-client#4992 or #3339

Comments

@PascalHelbig
Copy link

PascalHelbig commented Nov 11, 2016

Steps to Reproduce

import React from 'react';
import { graphql } from 'react-apollo';
import gql from 'graphql-tag';

const App = ({ data }) => {
  console.log('loading', data.loading);
  return (
    <button
      onClick={() => {
        console.log('refetch');
        data.refetch();
      }}
    >refetch</button>
  );
};

const query = gql`
  query allTypes {
      __schema {
          types {
              name
          }
      }
  }
`;

export default graphql(query)(App);

Buggy Behavior

When I call props.data.refetch(), props.data.loading is still false.
image

Expected Behavior

I thought it would change loading to true.

From docs:

loading: This field is true if there is currently a query fetch in flight, including after calling refetch. false otherwise.

Version

@tmeasday
Copy link
Contributor

I think this is a docs issue, I think we changed this a little while back.

@SachaG
Copy link

SachaG commented Dec 6, 2016

Maybe there should be a way to differentiate between initial loading (where you just show a loading spinner) and incremental loading (where you also want to keep the current content in addition to the loading indicator)?

@helfer
Copy link
Contributor

helfer commented Dec 13, 2016

@SachaG I think networkStatus is what you're referring to?

@SachaG
Copy link

SachaG commented Dec 14, 2016

@helfer you're right, I was using an older version of Apollo that didn't have it.

@rafaelcorreiapoli
Copy link

Hi!
I'm not receiving any new props when I call refetch, only when the results arrive from the server (and only if there are new results)
This is bad because I can't show a loading indicator when the users request the refetch action
I tried to inspect networkStatus as @helfer pointed but it is always "7", and I have no idea what it means

Thanks!

@dallonf
Copy link

dallonf commented Dec 27, 2016

I just gave @PascalHelbig's repro a quick check, swapping console.log('loading', data.loading); for console.log('networkStatus', data.networkStatus); and it only logs once:

networkStatus 1
networkStatus 7
refetch
refetch
refetch
(and so on)

If I instead change query to query a dummy resolver I added to my server just now to return a random number, it looks more like this:

networkStatus 1
networkStatus 7
refetch
networkStatus 7
refetch
networkStatus 7
refetch
networkStatus 7
(and so on)

The component never receives any props to indicate that a query is in flight.

@dallonf
Copy link

dallonf commented Dec 28, 2016

Aha, I might have found a workaround / solution: there is a notifyOnNetworkStatusChange option on watchQuery, which is only documented in the API reference.

With that option set, I get this output:

loading true networkStatus 1
loading false networkStatus 7
refetch
loading false networkStatus 2
loading false networkStatus 7
refetch
loading false networkStatus 4
loading false networkStatus 7
refetch
loading false networkStatus 4
loading false networkStatus 7

This is workable, but definitely surprising behavior that definitely needs to be better documented and possibly revisited.

Some specific feedback of things that are surprising to me:

  • My original intuition of the loading value is that it would be true whenever a query was in flight.
    Instead, it seems to be true only on the initial load? (so it acts more like what I would call !loaded) Although I think I've seen it also be true when the query's variables were updated...?
  • As mentioned above, networkStatus only receives updates if you pass the notifyOnNetworkStatusChange flag.
  • networkStatus is a number which doesn't mean much of anything to me. The only place this is "documented" is, as far as I'm aware, the type definition in the source code.
  • The first refetch's networkStatus, according to that type definition, is setVariables, rather than refetch, which is what I would expect and what it is on subsequent refetches. Is this a bug?

@raphaelcosta
Copy link

+1

@helfer
Copy link
Contributor

helfer commented Feb 10, 2017

This is really just a docs issue, so we should fix it there.

@nlhuykhang
Copy link

I have this problem too. Whenever I make a call to .refetch, data.loading is always false and data.networkStatus is always 7.

@ligaz
Copy link
Contributor

ligaz commented Mar 29, 2017

We hit the same problem this time with error property, which does not get updated correctly on refetch (with spotty connection: stop internet connection → refetch, you got error → start internet connect → refetch, your component does not re-rendered, thus still has the same error as property).

The only workaround is to use notifyOnNetworkStatusChange but shouldn't this be the default if properties like networkStatus, error and loading are exposed to the child component?

I have submitted a PR with a small documentation change in the docs that addresses this.

@helfer
Copy link
Contributor

helfer commented Mar 29, 2017

Which version of Apollo Client are folks running?

@ligaz
Copy link
Contributor

ligaz commented Mar 29, 2017

I'm on [email protected]. The version of react-apollo is the current master because the latest published is a couple of weeks old and the current master has fixes.

@nlhuykhang
Copy link

Mine is:

"apollo-client": "^0.10.0",
"react-apollo": "^0.13.0",

@itshallrun
Copy link

any solution to this?

@helfer
Copy link
Contributor

helfer commented Apr 7, 2017

@aikonplus the "solution" is to pass notifyOnNetworkStatusChange: true as a query option. This is really a docs issue and should be fixed there. If someone could make a PR to the React Apollo docs it would be very much appreciated!

Loading actually changes to true whenever a refetch happens, but unless notifyOnNetworkStatusChange is set, the component doesn't get passed the update.

@helfer helfer closed this as completed Apr 7, 2017
@merges
Copy link

merges commented Apr 24, 2017

Sorry to bug you on, well, a closed bug, but how does one pass query options to refetch()?

@helfer
Copy link
Contributor

helfer commented Apr 24, 2017

You can't. It wouldn't make sense to pass most options anyway, because refetch returns a promise.

@advance512
Copy link

advance512 commented May 12, 2017

Wouldn't a more sane behavior be to set the loading flag to true, or at least add refetching flag?

I mean, imagine you have two buttons: Start/Stop, and a "loading..." indicator.

On first view, you'll see "loading..." and then the Start button.
When you click Start, you'll still see the Start button, which means (in slow networks) you can spam it with multiple clicks. Yes, you can lock it, but wouldn't it be nicer to see the "loading..." indicator instead?
After a while of remaining with the Start button, you'll suddenly see the Stop button.

Adding a refecthing flag, or even better - setting the loading flag (with notifyOnNetworkStatusChange: true by default for all queries), would make a lot of sense.

@SevenOutman
Copy link

SevenOutman commented Jul 22, 2017

@helfer The notifyOnNetworkStatusChange approach doesn't seem to work with version 1.4.4, neither networkStatus nor loading was captured updating when refetch.
My code goes:

componentWillMount() {
    if (this.props.data.networkStatus >= 7) {
        console.log("refetch");
        this.props.data.refetch();
    }
}
componentWillReceiveProps(nextProps) {
    console.log(this.props.data.networkStatus, nextProps.data.networkStatus);
    console.log(this.props.data.loading, nextProps.data.loading);
}

Output:

1 7
true false

After refetchwas called, a new request appeared in network tab in Chrome devtools, but the output went:

1 7
true false
refetch
7 7
false false

And there's no more output after the request was completed.
Expected:

1 7
true false
refetch
7 4
false true
4 7
true false

@windgaucho
Copy link

Any solution for this? I have the same problem

@eskimoblood
Copy link

Shouldn't this ticket be reopened as the feature does not work as expected, aka. no changes innetworkStatus after refecthing the query.

@n1ru4l
Copy link
Contributor

n1ru4l commented Oct 16, 2017

@eskimoblood I am having the same issue and I think that apollographql/apollo-client#1263 apollographql/apollo-client#1622 is also related to this problem. We should probably create a new issue for this with a 2.0 prefix. What is your opinion?

Edit:

Seems like the promise returned by refetch is resolved after the refetch is finished, which means this can be used as a temporary workaround for determining if a query is being fetched or not.

Edit 2:

When using refetch while loading is true, the promise of refetch will never resolve nor reject.

I will also try to take a look at the code on the weekend.

@advance512
Copy link

Any response from the maintainers?

@marksauter
Copy link

marksauter commented Feb 17, 2018

I don't know if anyone is still having issues with this, but I solved it by changing my fetchPolicy option to cache-and-network in the options passed to the graphql hoc function.

@BrunoLerner
Copy link

Im still having this issue, and I set the notifyOnNetworkStatusChange flag like @dallonf said, but I still get networkStatus=7 everytime.

@stearm
Copy link

stearm commented Apr 16, 2019

Why is this issue closed? :/ I still have this problem!

@vojto
Copy link

vojto commented Apr 26, 2019

@stearm Did you try solution I mentioned few comments above?

That one is working for me with vanilla <Query>, however I was just trying it in react-apollo-hooks, and it doesn't work there! Don't have time for deeper look– but has anyone made this work with hooks?

@helfer helfer reopened this Apr 29, 2019
@derblitz
Copy link

derblitz commented May 2, 2019

I'm using the HOC approach and it also doesn't work (refetch is triggered by another component).

export default compose(
    graphql(QUERY_GET_OWN_COLLECTIONS, { name: "QGetOwnCollections", options: { notifyOnNetworkStatusChange: true, fetchPolicy: "cache-and-network" } })
)(SidebarContainer)

@kaikun213
Copy link

Same issue, seems quite fundamental to me too.

@jasonpaulos
Copy link
Contributor

apollographql/apollo-client#4992 may help solve this. If anyone can provide an up-to-date reproduction, I can verify that. Thanks!

@kasvtv
Copy link

kasvtv commented Jul 26, 2019

I stumbled upon this issue within 5 minutes of trying out react-apollo.

It only fails to rerender the component with {loading: true} if the last fetch resulted in an error.

From a product/user perspective, this means the following:

If the user is given a retry button after fetching has failed (several times), and the user then clicks the button:

There is simply no way of informing the user that a request is actually being retried. If it fails again, it looked like nothing ever happened.

This would create pretty big UX issues in a lot of frontends.

fetchPolicy={'cache-first'} is the only workaround that works, but that doesn't work for hooks...

@jasonpaulos
Copy link
Contributor

@kasvtv sorry to hear that this is still an issue for you. Could you provide a codesandbox.io or github repo that shows this problem? I really want to fix this bug!

@kasvtv
Copy link

kasvtv commented Jul 28, 2019

@jasonpaulos
This problem happens very consistently any time the last result was an error.

@jasonpaulos
Copy link
Contributor

jasonpaulos commented Aug 2, 2019

I am trying to get apollographql/apollo-client#4992 merged into apollo-client, which should solve this issue @kasvtv

@akornato
Copy link

akornato commented Aug 8, 2019

Wow this is quite similar to #2177 i.e. issue closed/ignored even though it persists, and also related to network status changes.

@hwillson
Copy link
Member

Thank you very much for apollographql/apollo-client#4992 @jasonpaulos! We'll prioritize a release of Apollo Client with those changes included, so this issue will be fixed in RA very shortly.

hwillson added a commit that referenced this issue Aug 10, 2019
1. It prevents the `onError` callback from unnecessarily being
called multiple times in a row.

2. It migrates all peer and dev deps to use `apollo-client`
2.6.4. Apollo Client 2.6.4 provides a fix for last result
error tracking
(apollographql/apollo-client@c44e821),
which fixes the long standing React Apollo issue of `refetch`
not setting `loading` state properly. Calling `refetch` will
now first set `loading` to `true`.

These fixes are bundled together, as verifying that `onError`
is working properly requires being able to make sure results
after a refetch that don't have an error, are handled properly.

Fixes #3331.
Fixes #3287.
Fixes #2559.
Fixes #321.

(and probably fixes others!)
hwillson added a commit that referenced this issue Aug 10, 2019
)

* This commit helps address 2 main issues:

1. It prevents the `onError` callback from unnecessarily being
called multiple times in a row.

2. It migrates all peer and dev deps to use `apollo-client`
2.6.4. Apollo Client 2.6.4 provides a fix for last result
error tracking
(apollographql/apollo-client@c44e821),
which fixes the long standing React Apollo issue of `refetch`
not setting `loading` state properly. Calling `refetch` will
now first set `loading` to `true`.

These fixes are bundled together, as verifying that `onError`
is working properly requires being able to make sure results
after a refetch that don't have an error, are handled properly.

Fixes #3331.
Fixes #3287.
Fixes #2559.
Fixes #321.

(and probably fixes others!)

* Changelog update

* Silence extra CI warnings to troubleshoot test issues
@mhuconcern
Copy link

This still occurs on v2.6.8 for me. I think i have spent an honest 4 hours trying to fix this bug where

<Query 
variables={{this changes}}
fetchPolicy='network-only'/>
// Check network status, it's always 7
// Data is fulfilled twice, once with older param second with newer param, causing intermittent ui flashes of data
</Query>

@mhuconcern
Copy link

mhuconcern commented Feb 27, 2020

I was able to resolve this by setting notifyOnNetworkStatusChange to true and fetchPolicy to cache-and-network. Kinda strange that I had to set fetch policy too. Isn't cache-and-network the default?

Anyway, this is working for me in version 2.3.0:

<Query
  query={MY_QUERY}
  variables={{...}}
  notifyOnNetworkStatusChange={true}
  fetchPolicy={'cache-and-network'}
>
  ...
</Query>

And I'm triggering refetch from another query by using refetchQueries prop.

ApolloClient v2.6.3
Savior Comment by @vojto , Just don't think just copy this word to word and add networkStatus checks.

@Dajust
Copy link

Dajust commented Feb 27, 2020

If you're using React, the new apollo hooks useQuery have fixed these issues. I've actually removed my above workaround from my codebase.

@EdmundMai
Copy link

@Dajust do you mean for @apollo/react-hooks v. 3.1.3? loading still doesn't change for me when I call refetch on that version

@EdmundMai
Copy link

EdmundMai commented Mar 14, 2020

I fixed this issue by adding notifyOnNetworkStatusChange: true,, which triggers loading to change even when calling refetch

  const { loading, data, fetchMore, refetch } = useQuery(GET_POSTS, {
    notifyOnNetworkStatusChange: true,
    fetchPolicy: "network-only",
  });

However, it causes fetchMore to trigger loading as well

@mkhib
Copy link

mkhib commented May 24, 2020

I fixed this issue by adding notifyOnNetworkStatusChange: true,, which triggers loading to change even when calling refetch

  const { loading, data, fetchMore, refetch } = useQuery(GET_POSTS, {
    notifyOnNetworkStatusChange: true,
    fetchPolicy: "network-only",
  });

However, it causes fetchMore to trigger loading as well

This solution works but is it possible not to trigger loading on fetchMore ?

@EllaSharakanski
Copy link

EllaSharakanski commented Jun 7, 2020

@EdmundMai Doesn't work for me (@apollo/client 3.0.0-beta.50), the loading is false when I call refetch. Doesn't work on 3.0.0-rc.2 too. Also the networkStatus is always undefined. I use:

const {
    refetch: checkCredentials,
    data: isValidCredentialsData,
    loading: loadingIsValidCredentials,
    networkStatus,
  } = useQuery<isValidCredentialsQuery, isValidCredentialsQueryVariables>(
    IS_VALID_CREDENTIALS_QUERY,
    {
      skip: true,
      fetchPolicy: 'network-only',
      notifyOnNetworkStatusChange: true,
    },
  )

and then I call checkCredentials. Maybe it's because I use skip: true?
I need it to get a promise that I can await (useLazyQuery doesn't return a promise).

@airandfingers
Copy link

I'm seeing the same behavior as @EllaSharakanski, using [email protected].

If I remove skip: true then the refetching works as described by others above, but I added skip to prevent the query from running the query too early; I don't see why skip should interfere with loading, data, or networkStatus when refetching later.

@mkhib
Copy link

mkhib commented Jun 22, 2020

I fixed this issue by adding notifyOnNetworkStatusChange: true,, which triggers loading to change even when calling refetch

  const { loading, data, fetchMore, refetch } = useQuery(GET_POSTS, {
    notifyOnNetworkStatusChange: true,
    fetchPolicy: "network-only",
  });

However, it causes fetchMore to trigger loading as well

This solution works but is it possible not to trigger loading on fetchMore ?

For anyone who wants to have different loadings on fetchMore and refetch, watch the networkStatus as it has value of 3 on fetchMore and 4 on refetch.

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