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

Example useEffect cleanup function? #573

Closed
stevenharderjr opened this issue Nov 2, 2022 · 4 comments · Fixed by #854
Closed

Example useEffect cleanup function? #573

stevenharderjr opened this issue Nov 2, 2022 · 4 comments · Fixed by #854
Assignees
Labels
triage me I really want to be triaged. type: question Request for information or clarification. Not an issue.

Comments

@stevenharderjr
Copy link

stevenharderjr commented Nov 2, 2022

The map is initialized via useEffect, but there is no cleanup function in the example code. Am I missing something?

@stevenharderjr stevenharderjr added triage me I really want to be triaged. type: question Request for information or clarification. Not an issue. labels Nov 2, 2022
@MALDRU
Copy link

MALDRU commented Feb 8, 2023

export const Map = ({ center, zoom, onLoad }) => {
  const ref = useRef()

  const callback = status => {
    if (status === Status.SUCCESS) {
      const map = new window.google.maps.Map(ref.current, {
        center,
        zoom,
      })
      onLoad && onLoad(map)
    }
  }

  return (
    <>
      <MapContainer ref={ref} />
      <Wrapper apiKey={googleMapsKey} callback={callback}></Wrapper>
    </>
  )
}

@deAtog
Copy link

deAtog commented Feb 5, 2025

This example is poorly written. The component creates a new instance of the google.maps.Map object on every render of the component. Google charges for every instance created.

function MyMapComponent({
  center,
  zoom,
}: {
  center: google.maps.LatLngLiteral;
  zoom: number;
}) {
  const ref = useRef();

  useEffect(() => {
    new window.google.maps.Map(ref.current, {
      center,
      zoom,
    });
  });

  return <div ref={ref} id="map" />;
}

Here is an improved version that only creates one instance of the google.maps.Map object:

function MyMapComponent({
  center,
  zoom,
}: {
  center: google.maps.LatLngLiteral;
  zoom: number;
}) {
  const ref = useRef();
  const [map, setMap] = useState();

  // Note the explicit empty array for dependencies here.
  // This ensures that this effect will only run ONCE after the first render.
  useEffect(() => {
    // MapOptions here aren't strictly necessary here, but may reduce initialization
    const map = new window.google.maps.Map(ref.current, {
      center,
      zoom,
    });

    setMap(map);
  }, []);

  useEffect(() => {
    if (!map) return;

    map.setZoom(zoom);
  }, [map, zoom]);

  useEffect(() => {
    if (!map) return;

    map.setCenter(center);
  }, [map, center]);

  return <div ref={ref} id="map" />;
}

@usefulthink
Copy link
Contributor

@deAtog thanks! Would you be able to quickly put that into the README and create a PR for it?

@usefulthink
Copy link
Contributor

I updated the example in #854

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage me I really want to be triaged. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants