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

fix refetch after error #141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

sijad
Copy link
Contributor

@sijad sijad commented Apr 24, 2019

fixes #74

const lastError = observableQuery.getLastError();
const lastResult = observableQuery.getLastResult();

if (!suspend) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this suspend test would fails, I'm not sure if it will breaks something.

@SimenB
Copy link

SimenB commented May 9, 2019

@trojanowski any chance of merging and releasing this? 🙏 It fixes the error I'm seeing in our app 🙂

@trojanowski
Copy link
Owner

Could you add a unit test for it? To ensure that it won't be back after #134 is merged.

@sijad
Copy link
Contributor Author

sijad commented May 13, 2019

@trojanowski a test case has been added but I think it's a little ugly. the other approach that came to my mind was to expose refetch in Tasks via useImperativeHandle. please let me know how can I make it better

@jack-sf
Copy link

jack-sf commented Jun 3, 2019

We tried this PR in our project and it works (fixes the problem), but there's one minor issue that could be fixed probably:

Right now, loading is always false, even when that refetch is in progress. Ideally, we should be informed about the loading change when refetch is being started.

Buut we can leave it as a separate issue and merge it PR in the meantime anyways ;]

@SimenB
Copy link

SimenB commented Jun 3, 2019

loading should not be true when refetching, you should check if networkStatus is 4: https://www.apollographql.com/docs/react/api/react-apollo/#datanetworkstatus

@jack-sf
Copy link

jack-sf commented Jun 4, 2019

Oh, okay. Well, in that case, the networkStatus isn't being updated, when refetch() is done after an error occurs.

See the following log:

image

Current outcome of networkStatus in the above scenario:

  • loading
  • failed
  • ready

Expected outcome:

  • loading
  • failed
  • refetch
  • ready

@amannn
Copy link

amannn commented Aug 5, 2019

Any update on this PR? Would be really great to see this merged.

@sijad
Copy link
Contributor Author

sijad commented Aug 5, 2019

you should probably use @apollo/react-hooks

@huidini
Copy link

huidini commented Aug 5, 2019

you should probably use @apollo/react-hooks

I started using it as well, but I really really miss the elegance and ease of use of Suspense.

@amannn
Copy link

amannn commented Aug 6, 2019

I understand. However @apollo/react-hooks is in beta currently and I wouldn't want to move to that library before it's stable.

Is there something missing in this PR or could it be merged as-is? As @huidini mentioned, using Suspense might still be a feature only this library has.

@sijad
Copy link
Contributor Author

sijad commented Aug 6, 2019

@amannn @apollo/react-hooks 3.0.0 released about an hour ago

https://github.com/apollographql/react-apollo/releases/tag/%40apollo%2Freact-hooks%403.0.0

@amannn
Copy link

amannn commented Aug 6, 2019

@sijad Oh wow, what a coincidence 🙂. Thanks!

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

Successfully merging this pull request may close these issues.

useQuery does not return data on refetch after error.
6 participants