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

stopPolling doesn't work when using React StrictMode #7221

Closed
gius80 opened this issue Oct 23, 2020 · 20 comments
Closed

stopPolling doesn't work when using React StrictMode #7221

gius80 opened this issue Oct 23, 2020 · 20 comments

Comments

@gius80
Copy link

gius80 commented Oct 23, 2020

Intended outcome:

According to documentation, stopPolling function should stop the query polling.

Actual outcome:

After calling stopPolling(), polling continues.

How to reproduce the issue:

export default function MyComponent() {

  const {
    data: carsData,
    loading: carsLoading,
    error: carsError,
    fetchMore,
    stopPolling,
    startPolling
  } = useQuery<getCars, getCarsVariables>(GET_CARS, {
    fetchPolicy: 'network-only',
    pollInterval: 10000
  })

useEffect(() => {
    return () => {
      stopPolling() // Doesn't stop polling on unmount
    }
  }, [])

  return (
    <>
      <Button
        onClick={() => {
          stopPolling() // Doesn't stop polling on click
        }}
      >
        STOP
      </Button>
      
     {/* MyComponent Render Stuff  */}

    </>
  )
}

Versions

System:
OS: macOS 10.15.6
Binaries:
Node: 14.12.0 - /usr/local/bin/node
Yarn: 1.22.5 - /usr/local/bin/yarn
npm: 6.14.8 - /usr/local/bin/npm
Browsers:
Chrome: 86.0.4240.111
Firefox: 81.0.2
Safari: 14.0
npmPackages:
@apollo/client: ^3.2.5 => 3.2.5

@gius80
Copy link
Author

gius80 commented Oct 23, 2020

I noticed that stopPolling works only if the polling is started using startPolling (so without passing pollIntervall options in query declaration).

Is this the desired behaviour?

export default function MyComponent() {

  const {
    data: carsData,
    loading: carsLoading,
    error: carsError,
    fetchMore,
    stopPolling,
    startPolling
  } = useQuery<getCars, getCarsVariables>(GET_CARS, {
    fetchPolicy: 'network-only'
  })

   useEffect(() => {
    startPolling(10000)
    return () => {
      stopPolling() // Works
    }
  }, [startPolling, stopPolling])

  return (
    <>
      <Button
        onClick={() => {
          stopPolling() // Works
        }}
      >
        STOP
      </Button>
      
     {/* MyComponent Render Stuff  */}

    </>
  )
}

@benjamn
Copy link
Member

benjamn commented Oct 26, 2020

@gius80 If you set a breakpoint (or debugger statement) before the stopPolling call and step into it using the debugger, what happens inside that method? I'm guessing this.reobserver is somehow undefined, so nothing happens, but I'd like to confirm that suspicion before making any further guesses.

@gius80
Copy link
Author

gius80 commented Oct 28, 2020

@benjamn In both cases this.reobserver is not undefined, is a valid object:

concast: Concast {observers: Set(1), promise: Promise, _subscriber: ƒ, resolve: ƒ, reject: ƒ, …}
fetch: ƒ (currentOptions, newNetworkStatus)
observer: {next: ƒ, error: ƒ}
options: {fetchPolicy: "network-only", pollInterval: 0, variables: {…}, query: {…}, onCompleted: ƒ, …}
shouldFetch: ƒ ()
__proto__: Object

I see that pollInterval in this.reobserver.options in both cases is 0 (so the value seems updated correctly), but in the first case (initial pollInterval set in the query options) it seems that the setTimeout it's not cleared.

@benjamn benjamn self-assigned this Oct 28, 2020
@benjamn
Copy link
Member

benjamn commented Oct 29, 2020

@gius80 We do have a passing test of calling stopPolling manually here, and I also just wrote this test, which passes:

    itAsync('should stop polling on unmount using stopPolling', (resolve, reject) => {
      let renderCount = 0;
      const Component = () => {
        let { data, loading, stopPolling } = useQuery(CAR_QUERY, {
          pollInterval: 100,
        });

        useEffect(() => stopPolling, []);

        switch (++renderCount) {
          case 1:
            expect(loading).toBeTruthy();
            break;
          case 2:
            expect(loading).toBeFalsy();
            expect(data).toEqual(CAR_RESULT_DATA);
            unmount();
            break;
          default:
            reject(new Error('Uh oh - we should have stopped polling!'));
        }

        return null;
      };

      const { unmount } = render(
        <MockedProvider link={new MockLink(CAR_MOCKS)}>
          <Component />
        </MockedProvider>
      );

      return wait(() => {
        expect(renderCount).toBe(2);
      }).then(resolve, reject);
    });

Can you see anything I'm missing in this test, compared to what you're doing?

@gius80
Copy link
Author

gius80 commented Nov 1, 2020

@benjamn test code seems ok.

I published a very simple example to illustrate the issue (it's a default react app).

http://apollo-polling.s3-website-eu-west-1.amazonaws.com/

pollInterval is set during query declaration. Polling start as expected, but calling stopPolling doesn't do anything.

Here is the code:

index.tsx

import React from 'react'
import ReactDOM from 'react-dom'
import './index.css'
import App from './App'
import reportWebVitals from './reportWebVitals'
import { ApolloClient, ApolloProvider, InMemoryCache } from '@apollo/client'

const client = new ApolloClient({
  uri: 'https://graphqlzero.almansi.me/api',
  cache: new InMemoryCache(),
})

ReactDOM.render(
  <React.StrictMode>
    <ApolloProvider client={client}>
      <App />
    </ApolloProvider>
  </React.StrictMode>,
  document.getElementById('root')
)

// If you want to start measuring performance in your app, pass a function
// to log results (for example: reportWebVitals(console.log))
// or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals
reportWebVitals()

App.tsx

import React from 'react'
import './App.css'
import { useQuery, gql } from '@apollo/client'

const GET_PHOTOS = gql`
  query {
    photos(options: { paginate: { page: 0, limit: 10 } }) {
      data {
        id
      }
    }
  }
`

function App() {
  const { loading, data, error, stopPolling } = useQuery(GET_PHOTOS, {
    pollInterval: 30000,
  })
  if (error) return <div>Error</div>
  return (
    <div className="App">
      <button onClick={stopPolling}>STOP POLLING</button>
      <div>
        {!loading &&
          data &&
          data.photos.data.map((photo: any) => (
            <span key={photo.id}>{photo.id} </span>
          ))}
      </div>
    </div>
  )
}

export default App

@Wozniaxos
Copy link

Any update on this?, I have the exact same issue (3.3.8v) - weird thing is that I have setup locally apollo library (same version) in development mode with -> https://github.com/apollographql/apollo-client/blob/main/CONTRIBUTING.md this fullstack setup guide. And the problem haven't occured, just gave up cause with startPolling manually it works properly.

@fhern077
Copy link

Also running into this issue as well !

@lcruz09
Copy link

lcruz09 commented Mar 4, 2021

Same issue here. Any updates?

@jmtimko5
Copy link

Seeing the same issue, this makes polling difficult to achieve because it seems like the only way to use it is unmountng the component with the query

@emik
Copy link

emik commented May 26, 2021

Had this issue (and a similar issue where the polling continued on other pages despite being unmounted) after upgrading apollo and CRA on development but not production and I traced it to <React.StrictMode>. Polling obviously does not play well with strict mode.

@brainkim brainkim self-assigned this Jun 7, 2021
@brainkim brainkim changed the title stopPolling doesn't stop query polling stopPolling doesn't work when using React StrictMode Jun 7, 2021
@brainkim
Copy link
Contributor

brainkim commented Jun 7, 2021

I’m only able to reproduce this bug when using React.StrictMode, so I updated the title. If anyone can come up with a reproduction that doesn’t use strict mode let me know.

@edwardofclt
Copy link

I'm still seeing this in v3.4.10.

I'm using this as a workaround:

const { data, loading, client } = useQuery(GetData, {
   pollingInterval: 5000
})

useEffect(() => {
  return () => {
     client.stop()
  }
}, [client]);

@briosheje
Copy link

briosheje commented Oct 19, 2021

I'm still seeing this in v3.4.10.

I'm using this as a workaround:

const { data, loading, client } = useQuery(GetData, {
   pollingInterval: 5000
})

useEffect(() => {
  return () => {
     client.stop()
  }
}, [client]);

Stilll experiencing this in 2021 (late october), but the above doesn't work for me because using client stops all the polling, while I just want to stop one specifically.

I think there is an info missing in the documentation about the fact that pollInterval and start/stop cannot be used together, hence I've found this working solution:

const { data, loading, startPolling, stopPolling } = useQuery(GetData);

useEffect(() => {
   startPolling(desiredTimeoutInMilliseconds);
}, [startPolling]);

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

Summarized, when the component is mounted the polling will start.
Once the component is destroyed, the polling will be stopped.

This seems to consistently work, hence I'm keeping this solution, hope this helps someone.

@brainkim
Copy link
Contributor

This is probably fixed in the 3.5, if you want to try it (it’s currently in a release candidate stage).

I think there is an info missing in the documentation about the fact that pollInterval and start/stop cannot be used together, hence I've found this working solution:

Yeah I’ve found it weird that you essentially pass a new pollInterval via the function, whereas I expected the pollInterval() passed to the original hook to be the source of truth, and the functions to take no arguments.

@briosheje
Copy link

This is probably fixed in the 3.5, if you want to try it (it’s currently in a release candidate stage).

I think there is an info missing in the documentation about the fact that pollInterval and start/stop cannot be used together, hence I've found this working solution:

Yeah I’ve found it weird that you essentially pass a new pollInterval via the function, whereas I expected the pollInterval() passed to the original hook to be the source of truth, and the functions to take no arguments.

Either way, I think the cleverest would be to have the hook to automatically stop the autopolling when the component is destroyed, since the polling is probably intended to be linked to the mounting component.

I will give a try to the RC when I happen to use that specific scenario again, right now I'm pretty good with this solution :)

@branaust
Copy link

branaust commented Nov 1, 2021

Still experiencing this issue with a React Native project.

We are using start/stopPolling (not pollInterval) and while start works, stop appears to do nothing.

@briosheje
Copy link

Still experiencing this issue with a React Native project.

We are using start/stopPolling (not pollInterval) and while start works, stop appears to do nothing.

I've used the approach I've provided above in react native as well and didn't have any issue, are you sure you are correctly invoking the stop method?

@cvburgess
Copy link

cvburgess commented Nov 4, 2021

I work with @branaust - this is the code in question (simplified a bit)

  const { data, loading, startPolling, stopPolling } = useQuery(
    GQL_QUERY,
    {
      variables: { id },
    }
  );

  const isItemPending = data?.items.some(
    (item) => item.status !== "READY"
  );

  useEffect(() => {
    isItemPending ?  startPolling(60 * 1000) : stopPolling();
  }, [isVideoPending]);

  useEffect(() => stopPolling, []);

Start works as expected but stop never works either on unmount or in the event that there are no pending items.

@subhamsaha-manu
Copy link

Its almost 2023 and still the behaviour remains the same, stopPolling() either works when startPolling(interval) has been used or without React.StrictMode.

@jaimefps
Copy link

This apollo issue with leaked polling caused a major problem for us this week. Thanks @briosheje, your answer saved us a lot of trouble.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests