Skip to content

fix refetch after error#141

Open
sijad wants to merge 2 commits into
trojanowski:masterfrom
sijad:fix-refetch
Open

fix refetch after error#141
sijad wants to merge 2 commits into
trojanowski:masterfrom
sijad:fix-refetch

Conversation

@sijad

@sijad sijad commented Apr 24, 2019

Copy link
Copy Markdown
Contributor

fixes #74

Comment thread src/useQuery.ts
const lastError = observableQuery.getLastError();
const lastResult = observableQuery.getLastResult();

if (!suspend) {

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

SimenB commented May 9, 2019

Copy link
Copy Markdown

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

@trojanowski

Copy link
Copy Markdown
Owner

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

@sijad

sijad commented May 13, 2019

Copy link
Copy Markdown
Contributor Author

@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

jack-sf commented Jun 3, 2019

Copy link
Copy Markdown

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

SimenB commented Jun 3, 2019

Copy link
Copy Markdown

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

jack-sf commented Jun 4, 2019

Copy link
Copy Markdown

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

amannn commented Aug 5, 2019

Copy link
Copy Markdown

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

@sijad

sijad commented Aug 5, 2019

Copy link
Copy Markdown
Contributor Author

you should probably use @apollo/react-hooks

@huidini

huidini commented Aug 5, 2019

Copy link
Copy Markdown

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

amannn commented Aug 6, 2019

Copy link
Copy Markdown

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

sijad commented Aug 6, 2019

Copy link
Copy Markdown
Contributor Author

@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

amannn commented Aug 6, 2019

Copy link
Copy Markdown

@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