react-hook-geolocation icon indicating copy to clipboard operation
react-hook-geolocation copied to clipboard

Bug: Renders indefinetly when using a callback function

Open 2Up1Down opened this issue 3 years ago • 2 comments

When using the useGeolocation hook it renders indefinitely when using the optional callback function. See the example below: https://codesandbox.io/s/nextjs-geolocation-b3defn?file=/pages/react-hook-geolocation.js

image

When the optional callback function is set to null its working fine. image

Is this an expected behaviour?

2Up1Down avatar Nov 12 '22 15:11 2Up1Down

Hi @2Up1Down 👋

Thank you for flagging this.

In the implementation there is a useCallback hook which has the callback function in its dependency array, which yields a function which is then later used in the dependency array of a useEffect. I believe that the reason you see this behaviour is that the referential safety of your original callback function is impaired. In other words, you are calling the hook with a referentially different callback function every time, which will also evaluate the useCallback hook and run the useEffect hook inside the useGeolocation hook every time.

I recommend you wrap your callback function within a useCallback to preserve its referential safety.

If you could kindly confirm that this resolves your issue, I would be very happy to update the documentation accordingly.

bence-toth avatar Nov 15 '22 19:11 bence-toth

Hi @bence-toth

I updated my code with a useCallback and this seems working. However I'm not the biggest fan of this approach. Maybe there is a more elegant solution to it. But then.. I consider myself still a beginner in React.,.

My question to you:

useEffect(() => {
    let watchId;

    if (isEnabled && navigator.geolocation) {
      navigator.geolocation.getCurrentPosition(updateCoordinates, setError);
      watchId = navigator.geolocation.watchPosition(
        updateCoordinates,
        setError,
        {
          enableHighAccuracy,
          maximumAge,
          timeout,
        }
      );
    }

    return () => {
      if (watchId) {
        navigator.geolocation.clearWatch(watchId);
      }
    };
  }, [
    isEnabled,
    callback,
    enableHighAccuracy,
    maximumAge,
    setError,
    timeout,
    updateCoordinates,
  ]);

Do you really need the callback in the dependency array?

2Up1Down avatar Nov 15 '22 21:11 2Up1Down