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

Bug: Renders indefinetly when using a callback function #58

Open
2Up1Down opened this issue Nov 12, 2022 · 2 comments · May be fixed by #60
Open

Bug: Renders indefinetly when using a callback function #58

2Up1Down opened this issue Nov 12, 2022 · 2 comments · May be fixed by #60
Assignees

Comments

@2Up1Down
Copy link

2Up1Down commented Nov 12, 2022

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?

@bence-toth
Copy link
Owner

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.

@2Up1Down
Copy link
Author

2Up1Down commented Nov 15, 2022

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?

@bence-toth bence-toth linked a pull request Nov 24, 2022 that will close this issue
@bence-toth bence-toth self-assigned this Nov 24, 2022
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 a pull request may close this issue.

2 participants