Turning on/off setInterval locating - using React.js

Question

I work with the GoogleMaps API, using React.js.

Inside the GoogleMap component:

• I have a component called UserLocationTimer, which activates setInterval within useEffect and samples the user's location every 2 seconds, centering the map accordingly. In addition, returns the UserLocation component that displays the marker of the user's location on the map.

• In case the user drags the map, I catch the event isDragged, and return only the UserLocation component that displays the marker of the user's location on the map.

It works fine, however, how would you optimize this code? Is there a more efficient way to catch the dragging event and stopping the timer interval from the parent, instead using conditional rendering?

  <GoogleMap
    . . .
    onDrag={(map) => {
      setIsDragged(true);
    }}
    onDragEnd={(map) => {
      setIsDragged(true);
    }}

  >
    . . .

    {!isDragged && (
      <UserLocationTimer
        userLocation={userLocation}
        setUserLocation={setUserLocation}
        panTo={panTo}
      ></UserLocationTimer>
    )}

    {isDragged && (
      <UserLocation userLocation={userLocation}></UserLocation>
    )}

  </GoogleMap>

export const UserLocationTimer = (props) => {
  const [timer, setTimer] = useState(null);

  useEffect(() => {
    const interval = setInterval(() => {
      setTimer(new Date());
      locateMe();
    }, 2000);
    return () => clearInterval(interval);
  }, []);

  function locateMe() {
    // locating using geolocation...
  }

  return (
    <>
      {timer && <UserLocation userLocation={props.userLocation}></UserLocation>}
    </>
  );
};

export const UserLocation = (props) => {

  if (props.userLocation)
    return (
      <>
        <Marker
          position={{
            lat: props.userLocation.lat,
            lng: props.userLocation.lng,
          }}
          icon={{
            url: userLocationIcon,
            scaledSize: new window.google.maps.Size(30, 30),
            origin: new window.google.maps.Point(0, 0),
            anchor: new window.google.maps.Point(15, 15),
          }}
        />
      </>
    );
  else return null;
};

Answer 1

Typo? I think you meant to set isDragged to false when dragging finishes:

onDragEnd={() => {
  setIsDragged(false);
}}

Note that you don't need to declare parameter you don't use (and probably shouldn't, to make the code easier to read).

Concise props When you have variables that you want to pass as props of the same name, you can make things a bit concise by using shorthand property names and spreading the object into props. For example:

<UserLocationTimer
  userLocation={userLocation}
  setUserLocation={setUserLocation}
  panTo={panTo}
></UserLocationTimer>

can be

<UserLocationTimer {...{userLocation, setUserLocation, panTo}} />

(it has no children, so it can be made self-closing)

Listing fewer identifiers while writing code means less chance of typos, and helps enforce consistent names for things.

Marker position props Rather than listing out the lat and lng properties of userLocation, you can pass the plain object itself to position:

<Marker position={props.userLocation}

Return early instead of long if/else blocks - if one sees if followed by a long block of code, followed by an else, keeping track of what condition the else is corresponding to can be harder to recognize at a glance than it needs to be. Consider returning the short branch first instead. See below.

JSX fragments are only needed when enclosing 2 or more tags - if a fragment only has one child tag, it can be removed completely, like this:

export const UserLocation = ({ userLocation }) => (
  if (!userLocation) {
    return null;
  }
  return (
    <Marker
      // ...

The same thing applies to the JSX returned by UserLocationTimer, but there's a better approach:

Consolidate UserLocation Since UserLocation is being rendered regardless, I think conditionally putting it inside UserLocationTimer isn't the right way to go. I'd remove the timer component completely, and implement its logic in the parent component instead, while unconditionally rendering UserLocation.

const intervalRef = useRef();
useEffect(() => {
  if (isDragged) {
    const intervalId = setInterval(locateMe, 2000);
    // Cleanup will run on dismount and when isDragged goes to false
    return () => clearInterval(intervalId);
  }
}, [isDragged]);

// ...

<GoogleMap
  onDrag={() => {
    setIsDragged(true);
  }}
  onDragEnd={() => {
    setIsDragged(false);
  }}
>
  . . .
  <UserLocation {...{userLocation}} />
</GoogleMap>

Naming The name UserLocation is very similar to userLocation, but one is a component, while the other holds data. Maybe rename them so that they're more distinct - perhaps UserLocationMarker and userLocationCoords?