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

useQuery keeps polling #3272

Closed
MYKEU opened this issue Jul 24, 2019 · 6 comments · Fixed by #3273
Closed

useQuery keeps polling #3272

MYKEU opened this issue Jul 24, 2019 · 6 comments · Fixed by #3273
Assignees
Milestone

Comments

@MYKEU
Copy link

MYKEU commented Jul 24, 2019

Just noticed when using the useQuery hook with a poll interval, the polling continues even when navigating to another page / hiding the component with the query in.

Intended outcome:

The polling should stop when navigating away from the page with the query component / when the component is hidden (via state for example)

Actual outcome:

Polling continues

How to reproduce the issue:

https://codesandbox.io/s/apollo-react-query-component-f49si?fontsize=14

Click button to hide the component then check the network calls

Version

@apollo/react-hooks - 0.1.0-beta.11

@dqunbp
Copy link
Contributor

dqunbp commented Jul 24, 2019

I also reproduced this bug.
I have prepared PR to fix this #3273
My reproduce: https://codesandbox.io/s/apolloreact-hooksusequerybug-ympqk

@proutek
Copy link

proutek commented Jul 24, 2019

I'm using this workaround.

  const { loading, error, data, startPolling, stopPolling } = useQuery(GET_DELIVERIES_QUERY)

  useEffect(() => {
    startPolling(5000)
    return () => {
      stopPolling()
    }
  }, [startPolling, stopPolling])

@ivansky
Copy link

ivansky commented Jul 26, 2019

@proutek Why do you need update effect when links to functions startPolling and stopPolling are changing?

I am not sure is there a logic behind useQuery but for my vision these function should not be changed in any case, so this effect would be pretty enough.

useEffect(() => {
    startPolling(5000); // will be called only once
    return stopPolling; // just return cleanup function without making new one
  }, []);

Then effect will be

@proutek
Copy link

proutek commented Jul 26, 2019

Yes, you're right. I tried it before with empty array. I just wanted to get rid off the compiler warning.

Compiled with warnings.

./src/components/Shipping/ShippingActive.tsx
  Line 30:  React Hook useEffect has missing dependencies: 'startPolling' and 'stopPolling'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

Maybe I could try something like this facebook/create-react-app#6880

@ivansky
Copy link

ivansky commented Jul 26, 2019

@proutek the thing is, this lint rule is cool for check yourself with dependencies but it's not much smart to separate them as required and not required because there is no checking types and logic behind our code.
I am coding without this rule but I must be careful with dependencies.

@hwillson hwillson self-assigned this Jul 29, 2019
@hwillson hwillson added this to the Release 3.0 milestone Jul 29, 2019
@thomas81528262
Copy link

Here is the code work for me, but I am not sure it is the correct way or not.

const { data, error, loading, stopPolling } = useQuery(GET_INVENTORY_DATA, {
    pollInterval: 500
  });

  useEffect(() => {
    return () => {
      stopPolling();
    };
  });

hwillson pushed a commit that referenced this issue Aug 5, 2019
* Add test for polling
- update add link prop to the MockedProvider
- add test that fails when component unmounted

* fix useQuery polling unmounted bug

* Re-arrange things a bit to remove extra `setTimeout`

These changes will let us avoid the extra `setTimeout` call
in the cleanup `useEffect`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants