From 17a88d00c328177eded8b2bb51a2048320abab8b Mon Sep 17 00:00:00 2001 From: Martin Schuhfuss Date: Tue, 7 Nov 2023 23:20:23 +0100 Subject: [PATCH 1/3] feat: update map viewport when props are changed Improves handling of view props (center, zoom, heading, tilt) and introduces a new `useCallbackRef` hook --- src/components/map.tsx | 87 ++++++++++++++++++------------ src/libraries/use-callback-ref.tsx | 8 +++ 2 files changed, 62 insertions(+), 33 deletions(-) create mode 100644 src/libraries/use-callback-ref.tsx diff --git a/src/components/map.tsx b/src/components/map.tsx index ba4539b7..22d2d694 100644 --- a/src/components/map.tsx +++ b/src/components/map.tsx @@ -2,6 +2,8 @@ import React, { CSSProperties, PropsWithChildren, + Ref, + RefCallback, useCallback, useContext, useEffect, @@ -14,6 +16,7 @@ import {APIProviderContext, APIProviderContextValue} from './api-provider'; import {useApiIsLoaded} from '../hooks/use-api-is-loaded'; import {logErrorOnce} from '../libraries/errors'; +import {useCallbackRef} from '../libraries/use-callback-ref'; // Google Maps context export interface GoogleMapsContextValue { @@ -75,6 +78,7 @@ export const Map = (props: PropsWithChildren) => { } const [map, mapRef] = useMapInstanceHandlerEffects(props, context); + useMapOptionsEffects(map, props); useDeckGLCameraUpdateEffect(map, viewState); const isViewportSet = useMemo(() => Boolean(viewport), [viewport]); @@ -117,13 +121,10 @@ Map.deckGLViewProps = true; function useMapInstanceHandlerEffects( props: MapProps, context: APIProviderContextValue -): readonly [ - map: google.maps.Map | null, - mapRef: (instance: HTMLDivElement | null) => void -] { +): readonly [map: google.maps.Map | null, containerRef: Ref] { const apiIsLoaded = useApiIsLoaded(); const [map, setMap] = useState(null); - const [container, setContainer] = useState(null); + const [container, containerRef] = useCallbackRef(); const { id, @@ -134,10 +135,6 @@ function useMapInstanceHandlerEffects( ...mapOptions } = props; - const mapRef = useCallback((node: HTMLDivElement | null) => { - setContainer(node || null); - }, []); - // create the map instance and register it in the context useEffect( () => { @@ -175,7 +172,7 @@ function useMapInstanceHandlerEffects( }, // Dependencies need to be inaccurately limited here. The cleanup function - // will remove the map-instance with all it's internal state, and we can't + // will remove the map-instance with all its internal state, and we can't // have that happening. This is only ok when the id or mapId is changed, // since this requires a new map to be created anyway. @@ -190,28 +187,6 @@ function useMapInstanceHandlerEffects( [id, container, apiIsLoaded, props.mapId] ); - // update the map options when mapOptions is changed - useEffect( - () => { - if (!map) { - return; - } - - // FIXME: for now, we have to filter all options describing the viewport - // here, since those are updated in google maps internally or using the - // viewState parameter externally. - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const {center, zoom, heading, tilt, mapId, ...otherOptions} = mapOptions; - - map.setOptions(otherOptions); - }, - // Not triggered when the map is changed, since in that case the - // options have already been passed to the map constructor. - - // eslint-disable-next-line react-hooks/exhaustive-deps - [mapOptions] - ); - // report an error if the same map-id is used multiple times useEffect(() => { if (!id) { @@ -229,7 +204,53 @@ function useMapInstanceHandlerEffects( } }, [id, context, map]); - return [map, mapRef] as const; + return [map, containerRef] as const; +} + +/** + * Internal hook to update the map-options and view-parameters when + * props are changed. + */ +function useMapOptionsEffects(map: google.maps.Map | null, mapProps: MapProps) { + const {center, zoom, heading, tilt, ...mapOptions} = mapProps; + + // All of these effects aren't triggered when the map is changed. + // In that case, the values have already been passed to the map constructor. + + // update the map options when mapOptions is changed + useEffect(() => { + if (!map) return; + + // NOTE: passing a mapId to setOptions triggers an error-message we don't need to see here + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {mapId, ...opts} = mapOptions; + + map.setOptions(opts); + }, [mapProps]); + + useEffect(() => { + if (!map || !center) return; + + map.setCenter(center); + }, [center]); + + useEffect(() => { + if (!map || !Number.isFinite(zoom)) return; + + map.setZoom(zoom as number); + }, [zoom]); + + useEffect(() => { + if (!map || !Number.isFinite(heading)) return; + + map.setHeading(heading as number); + }, [heading]); + + useEffect(() => { + if (!map || !Number.isFinite(tilt)) return; + + map.setTilt(tilt as number); + }, [tilt]); } /** diff --git a/src/libraries/use-callback-ref.tsx b/src/libraries/use-callback-ref.tsx new file mode 100644 index 00000000..2bfe46a7 --- /dev/null +++ b/src/libraries/use-callback-ref.tsx @@ -0,0 +1,8 @@ +import {Ref, useCallback, useState} from 'react'; + +export function useCallbackRef() { + const [el, setEl] = useState(null); + const ref = useCallback((value: T) => setEl(value), [setEl]); + + return [el, ref as Ref] as const; +} From 6221bb6dafcc1d87f40e4f46153b8f8196cc7710 Mon Sep 17 00:00:00 2001 From: Martin Schuhfuss Date: Tue, 7 Nov 2023 23:48:17 +0100 Subject: [PATCH 2/3] feat: cleanup map, remove onLoadMap prop BREAKING CHANGE: removed MapProps.onLoadMap --- src/components/map.tsx | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/src/components/map.tsx b/src/components/map.tsx index 22d2d694..139ac01c 100644 --- a/src/components/map.tsx +++ b/src/components/map.tsx @@ -3,8 +3,6 @@ import React, { CSSProperties, PropsWithChildren, Ref, - RefCallback, - useCallback, useContext, useEffect, useLayoutEffect, @@ -18,7 +16,6 @@ import {useApiIsLoaded} from '../hooks/use-api-is-loaded'; import {logErrorOnce} from '../libraries/errors'; import {useCallbackRef} from '../libraries/use-callback-ref'; -// Google Maps context export interface GoogleMapsContextValue { map: google.maps.Map | null; } @@ -45,10 +42,6 @@ export type MapProps = google.maps.MapOptions & { * This is also needed to reference the map inside the useMap hook. */ id?: string; - /** - * A callback function that is called, when the Google Map is loaded. - */ - onLoadMap?: (map: google.maps.Map) => void; /** * Viewport from deck.gl */ @@ -129,34 +122,20 @@ function useMapInstanceHandlerEffects( const { id, initialBounds, - onLoadMap, - // FIXME: this destructuring here leads to mapOptions being a new - // object for every render, and we'll be calling map.setOptions() a lot. + ...mapOptions } = props; // create the map instance and register it in the context useEffect( () => { - // this will be called a couple of times before the dependencies are - // actually ready to create the map if (!container || !apiIsLoaded) return; - // Since we can't know the map-ids used in sibling components during - // rendering, we can't check for existing maps with the same id here. - // We do have a seperate hook below that keeps an eye on mapIds and will - // write an error-message to the console if reused ids are detected. const {addMapInstance, removeMapInstance} = context; const newMap = new google.maps.Map(container, mapOptions); setMap(newMap); addMapInstance(newMap, id); - if (onLoadMap) { - google.maps.event.addListenerOnce(newMap, 'idle', () => { - onLoadMap(newMap); - }); - } - if (initialBounds) { newMap.fitBounds(initialBounds); } @@ -164,6 +143,7 @@ function useMapInstanceHandlerEffects( return () => { if (!container || !apiIsLoaded) return; + // remove all event-listeners to minimize memory-leaks google.maps.event.clearInstanceListeners(container); setMap(null); @@ -171,17 +151,11 @@ function useMapInstanceHandlerEffects( }; }, - // Dependencies need to be inaccurately limited here. The cleanup function - // will remove the map-instance with all its internal state, and we can't - // have that happening. This is only ok when the id or mapId is changed, - // since this requires a new map to be created anyway. - // FIXME: we should rethink if it could be possible to keep the state // around when a map gets re-initialized (id or mapId changed). This // should keep the viewport as it is (so also no initial viewport in - // this case) and any added features should of course stay as well (though - // that is for those components to figure out to always be attached to - // the correct map-instance from the context) . + // this case) and any added features should of course get re-added as + // well. // eslint-disable-next-line react-hooks/exhaustive-deps [id, container, apiIsLoaded, props.mapId] @@ -189,9 +163,7 @@ function useMapInstanceHandlerEffects( // report an error if the same map-id is used multiple times useEffect(() => { - if (!id) { - return; - } + if (!id) return; const {mapInstances} = context; From 283be709a5b7f59beff1b9787c8f2ae8eb72faf3 Mon Sep 17 00:00:00 2001 From: Martin Schuhfuss Date: Wed, 8 Nov 2023 13:17:14 +0100 Subject: [PATCH 3/3] feat: implement props for all map-events with custom MapEvent type --- src/components/map.tsx | 283 +++++++++++++++++++++++++++++++++++------ 1 file changed, 247 insertions(+), 36 deletions(-) diff --git a/src/components/map.tsx b/src/components/map.tsx index 139ac01c..b7ea0bb9 100644 --- a/src/components/map.tsx +++ b/src/components/map.tsx @@ -19,42 +19,108 @@ import {useCallbackRef} from '../libraries/use-callback-ref'; export interface GoogleMapsContextValue { map: google.maps.Map | null; } - export const GoogleMapsContext = React.createContext(null); +/** + * Handlers for all events that could be emitted by map-instances. + */ +type MapEventProps = Partial<{ + // map view state events + onBoundsChanged: (event: MapCameraChangedEvent) => void; + onCenterChanged: (event: MapCameraChangedEvent) => void; + onHeadingChanged: (event: MapCameraChangedEvent) => void; + onTiltChanged: (event: MapCameraChangedEvent) => void; + onZoomChanged: (event: MapCameraChangedEvent) => void; + onProjectionChanged: (event: MapCameraChangedEvent) => void; + + // mouse / touch / pointer events + onClick: (event: MapMouseEvent) => void; + onDblclick: (event: MapMouseEvent) => void; + onContextmenu: (event: MapMouseEvent) => void; + onMousemove: (event: MapMouseEvent) => void; + onMouseover: (event: MapMouseEvent) => void; + onMouseout: (event: MapMouseEvent) => void; + onDrag: (event: MapEvent) => void; + onDragend: (event: MapEvent) => void; + onDragstart: (event: MapEvent) => void; + + // loading events + onTilesLoaded: (event: MapEvent) => void; + onIdle: (event: MapEvent) => void; + + // configuration events + onIsFractionalZoomEnabledChanged: (event: MapEvent) => void; + onMapCapabilitiesChanged: (event: MapEvent) => void; + onMapTypeIdChanged: (event: MapEvent) => void; + onRenderingTypeChanged: (event: MapEvent) => void; +}>; + +/** + * Maps the camelCased names of event-props to the corresponding event-types + * used in the maps API. + */ +const propNameToEventType: {[prop in keyof Required]: string} = { + onBoundsChanged: 'bounds_changed', + onCenterChanged: 'center_changed', + onClick: 'click', + onContextmenu: 'contextmenu', + onDblclick: 'dblclick', + onDrag: 'drag', + onDragend: 'dragend', + onDragstart: 'dragstart', + onHeadingChanged: 'heading_changed', + onIdle: 'idle', + onIsFractionalZoomEnabledChanged: 'isfractionalzoomenabled_changed', + onMapCapabilitiesChanged: 'mapcapabilities_changed', + onMapTypeIdChanged: 'maptypeid_changed', + onMousemove: 'mousemove', + onMouseout: 'mouseout', + onMouseover: 'mouseover', + onProjectionChanged: 'projection_changed', + onRenderingTypeChanged: 'renderingtype_changed', + onTilesLoaded: 'tilesloaded', + onTiltChanged: 'tilt_changed', + onZoomChanged: 'zoom_changed' +} as const; + +type MapEventPropName = keyof MapEventProps; +const eventPropNames = Object.freeze( + Object.keys(propNameToEventType) as MapEventPropName[] +); /** * Props for the Google Maps Map Component */ -export type MapProps = google.maps.MapOptions & { - style?: CSSProperties; - /** - * Adds custom style to the map by passing a css class. - */ - className?: string; - /** - * Adds initial bounds to the map as an alternative to specifying the center/zoom of the map. - * Calls the fitBounds method internally https://developers.google.com/maps/documentation/javascript/reference/map?hl=en#Map-Methods - */ - initialBounds?: google.maps.LatLngBounds | google.maps.LatLngBoundsLiteral; - /** - * An id that is added to the map. Needed when using more than one Map component. - * This is also needed to reference the map inside the useMap hook. - */ - id?: string; - /** - * Viewport from deck.gl - */ - viewport?: unknown; - /** - * View state from deck.gl - */ - viewState?: Record; - /** - * Initial View State from deck.gl - */ - initialViewState?: Record; -}; +export type MapProps = google.maps.MapOptions & + MapEventProps & { + style?: CSSProperties; + /** + * Adds custom style to the map by passing a css class. + */ + className?: string; + /** + * Adds initial bounds to the map as an alternative to specifying the center/zoom of the map. + * Calls the fitBounds method internally https://developers.google.com/maps/documentation/javascript/reference/map?hl=en#Map-Methods + */ + initialBounds?: google.maps.LatLngBounds | google.maps.LatLngBoundsLiteral; + /** + * An id that is added to the map. Needed when using more than one Map component. + * This is also needed to reference the map inside the useMap hook. + */ + id?: string; + /** + * Viewport from deck.gl + */ + viewport?: unknown; + /** + * View state from deck.gl + */ + viewState?: Record; + /** + * Initial View State from deck.gl + */ + initialViewState?: Record; + }; /** * Component to render a Google Maps map @@ -62,7 +128,7 @@ export type MapProps = google.maps.MapOptions & { export const Map = (props: PropsWithChildren) => { const {children, id, className, style, viewState, viewport} = props; - const context = useContext(APIProviderContext) as APIProviderContextValue; + const context = useContext(APIProviderContext); if (!context) { throw new Error( @@ -70,8 +136,9 @@ export const Map = (props: PropsWithChildren) => { ); } - const [map, mapRef] = useMapInstanceHandlerEffects(props, context); + const [map, mapRef] = useMapInstanceEffects(props, context); useMapOptionsEffects(map, props); + useMapEvents(map, props); useDeckGLCameraUpdateEffect(map, viewState); const isViewportSet = useMemo(() => Boolean(viewport), [viewport]); @@ -111,7 +178,7 @@ Map.deckGLViewProps = true; * ref that will be used to pass the map-container into this hook. * @internal */ -function useMapInstanceHandlerEffects( +function useMapInstanceEffects( props: MapProps, context: APIProviderContextValue ): readonly [map: google.maps.Map | null, containerRef: Ref] { @@ -144,7 +211,7 @@ function useMapInstanceHandlerEffects( if (!container || !apiIsLoaded) return; // remove all event-listeners to minimize memory-leaks - google.maps.event.clearInstanceListeners(container); + google.maps.event.clearInstanceListeners(newMap); setMap(null); removeMapInstance(id); @@ -182,12 +249,17 @@ function useMapInstanceHandlerEffects( /** * Internal hook to update the map-options and view-parameters when * props are changed. + * @internal */ function useMapOptionsEffects(map: google.maps.Map | null, mapProps: MapProps) { const {center, zoom, heading, tilt, ...mapOptions} = mapProps; - // All of these effects aren't triggered when the map is changed. - // In that case, the values have already been passed to the map constructor. + /* eslint-disable react-hooks/exhaustive-deps -- + * + * The following effects aren't triggered when the map is changed. + * In that case, the values will be or have been passed to the map + * constructor as mapOptions. + */ // update the map options when mapOptions is changed useEffect(() => { @@ -223,6 +295,40 @@ function useMapOptionsEffects(map: google.maps.Map | null, mapProps: MapProps) { map.setTilt(tilt as number); }, [tilt]); + /* eslint-enable react-hooks/exhaustive-deps */ +} + +/** + * Sets up effects to bind event-handlers for all event-props in MapEventProps. + * @internal + */ +function useMapEvents(map: google.maps.Map | null, props: MapEventProps) { + // note: calling a useEffect hook from within a loop is prohibited by the + // rules of hooks, but it's ok here since it's unconditional and the number + // and order of iterations is always strictly the same. + // (see https://legacy.reactjs.org/docs/hooks-rules.html) + + for (const propName of eventPropNames) { + // fixme: this cast is essentially a 'trust me, bro' for typescript, but + // a proper solution seems way too complicated right now + const handler = props[propName] as (ev: MapEvent) => void; + const eventType = propNameToEventType[propName]; + + // eslint-disable-next-line react-hooks/rules-of-hooks + useEffect(() => { + if (!map) return; + if (!handler) return; + + const listener = map.addListener( + eventType, + (ev?: google.maps.MapMouseEvent | google.maps.IconMouseEvent) => { + handler(createMapEvent(eventType, map, ev)); + } + ); + + return () => listener.remove(); + }, [map, eventType, handler]); + } } /** @@ -263,3 +369,108 @@ function useDeckGLCameraUpdateEffect( }); }, [map, viewState]); } + +export type MapEvent = { + type: string; + map: google.maps.Map; + detail: T; + + stoppable: boolean; + stop: () => void; + domEvent?: MouseEvent | TouchEvent | PointerEvent | KeyboardEvent | Event; +}; + +export type MapMouseEvent = MapEvent<{ + latLng: google.maps.LatLngLiteral | null; + placeId: string | null; +}>; + +export type MapCameraChangedEvent = MapEvent<{ + center: google.maps.LatLngLiteral; + bounds: google.maps.LatLngBoundsLiteral; + zoom: number; + heading: number; + tilt: number; +}>; + +const cameraEventTypes = [ + 'bounds_changed', + 'center_changed', + 'heading_changed', + 'projection_changed', + 'tilt_changed', + 'zoom_changed' +]; + +const mouseEventTypes = [ + 'click', + 'contextmenu', + 'dblclick', + 'mousemove', + 'mouseout', + 'mouseover' +]; + +export function createMapEvent( + type: string, + map: google.maps.Map, + srcEvent?: google.maps.MapMouseEvent | google.maps.IconMouseEvent +): MapEvent { + const ev: MapEvent = { + type, + map, + detail: {}, + stoppable: false, + stop: () => {} + }; + + if (cameraEventTypes.includes(type)) { + const camEvent = ev as MapCameraChangedEvent; + + const center = map.getCenter(); + const zoom = map.getZoom(); + const heading = map.getHeading() || 0; + const tilt = map.getTilt() || 0; + const bounds = map.getBounds(); + + if (!center || !bounds || !Number.isFinite(zoom)) { + console.warn( + '[createEvent] at least one of the values from the map ' + + 'returned undefined. This is not expected to happen. Please ' + + 'report an issue at https://github.com/visgl/react-google-maps/issues/new' + ); + } + + camEvent.detail = { + center: center?.toJSON() || {lat: 0, lng: 0}, + zoom: zoom as number, + heading: heading as number, + tilt: tilt as number, + bounds: bounds?.toJSON() || { + north: 90, + east: 180, + south: -90, + west: -180 + } + }; + + return camEvent; + } else if (mouseEventTypes.includes(type)) { + if (!srcEvent) + throw new Error('[createEvent] mouse events must provide a srcEvent'); + const mouseEvent = ev as MapMouseEvent; + + mouseEvent.domEvent = srcEvent.domEvent; + mouseEvent.stoppable = true; + mouseEvent.stop = () => srcEvent.stop(); + + mouseEvent.detail = { + latLng: srcEvent.latLng?.toJSON() || null, + placeId: (srcEvent as google.maps.IconMouseEvent).placeId + }; + + return mouseEvent; + } + + return ev; +}