Skip to content

Commit 1dc1584

Browse files
authored
fix: add map camera state tracking (#84)
Camera state tracking allows the Map component to avoid a feedback-loop when camera values published by the map are fed back into the Map component props. I measured round-trip times (dispatching an event into the react-application and receiving them back as props) of around 4-5ms. However, there are cases where this roundtrip crosses the frame-boundary, and times more like 20ms can be observed. When this happens, stuttering, janky animations and suboptimal interactions will happen.
1 parent b87ba05 commit 1dc1584

File tree

6 files changed

+125
-12
lines changed

6 files changed

+125
-12
lines changed

src/components/__tests__/map.test.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,3 +150,16 @@ describe('creating and updating map instance', () => {
150150
expect(options).toMatchObject({mapId: 'othermapid'});
151151
});
152152
});
153+
154+
describe('map events and event-props', () => {
155+
test.todo('events dispatched by the map are received via event-props');
156+
});
157+
158+
describe('camera updates', () => {
159+
test.todo('initial camera state is passed via mapOptions, not moveCamera');
160+
test.todo('updated camera state is passed to moveCamera');
161+
test.todo("re-renders with unchanged camera state don't trigger moveCamera");
162+
test.todo(
163+
"re-renders with props received via events don't trigger moveCamera"
164+
);
165+
});

src/components/map/index.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {useCallbackRef} from '../../libraries/use-callback-ref';
1717
import {MapEventProps, useMapEvents} from './use-map-events';
1818
import {useMapOptions} from './use-map-options';
1919
import {useDeckGLCameraUpdate} from './use-deckgl-camera-update';
20+
import {useInternalCameraState} from './use-internal-camera-state';
2021

2122
export interface GoogleMapsContextValue {
2223
map: google.maps.Map | null;
@@ -73,8 +74,9 @@ export const Map = (props: PropsWithChildren<MapProps>) => {
7374
}
7475

7576
const [map, mapRef] = useMapInstance(props, context);
76-
useMapOptions(map, props);
77-
useMapEvents(map, props);
77+
const cameraStateRef = useInternalCameraState();
78+
useMapOptions(map, cameraStateRef, props);
79+
useMapEvents(map, cameraStateRef, props);
7880
useDeckGLCameraUpdate(map, viewState);
7981

8082
const isViewportSet = useMemo(() => Boolean(viewport), [viewport]);
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import {MutableRefObject, useRef} from 'react';
2+
import {MapCameraChangedEvent, MapEvent} from './use-map-events';
3+
4+
export type InternalCameraState = {
5+
center: google.maps.LatLngLiteral;
6+
heading: number;
7+
tilt: number;
8+
zoom: number;
9+
};
10+
11+
export type InternalCameraStateRef = MutableRefObject<InternalCameraState>;
12+
13+
/**
14+
* Creates a mutable ref object to track the last known state of the map camera.
15+
* This is updated by `trackDispatchedEvent` and used in `useMapOptions`.
16+
*/
17+
export function useInternalCameraState(): InternalCameraStateRef {
18+
return useRef<InternalCameraState>({
19+
center: {lat: 0, lng: 0},
20+
heading: 0,
21+
tilt: 0,
22+
zoom: 0
23+
});
24+
}
25+
26+
/**
27+
* Records camera data from the last event dispatched to the React application
28+
* in a mutable `IternalCameraStateRef`.
29+
* This data can then be used to prevent feeding these values back to the
30+
* map-instance when a typical "controlled component" setup (state variable is
31+
* fed into and updated by the map).
32+
*/
33+
export function trackDispatchedEvent(
34+
ev: MapEvent,
35+
cameraStateRef: InternalCameraStateRef
36+
) {
37+
const cameraEvent = ev as MapCameraChangedEvent;
38+
39+
// we're only interested in the camera-events here
40+
if (!cameraEvent.detail.center) return;
41+
const {center, zoom, heading, tilt} = cameraEvent.detail;
42+
43+
cameraStateRef.current.center = center;
44+
cameraStateRef.current.heading = heading;
45+
cameraStateRef.current.tilt = tilt;
46+
cameraStateRef.current.zoom = zoom;
47+
}

src/components/map/use-map-events.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import {useEffect} from 'react';
2+
import {
3+
InternalCameraStateRef,
4+
trackDispatchedEvent
5+
} from './use-internal-camera-state';
26

37
/**
48
* Handlers for all events that could be emitted by map-instances.
@@ -40,6 +44,7 @@ export type MapEventProps = Partial<{
4044
*/
4145
export function useMapEvents(
4246
map: google.maps.Map | null,
47+
cameraStateRef: InternalCameraStateRef,
4348
props: MapEventProps
4449
) {
4550
// note: calling a useEffect hook from within a loop is prohibited by the
@@ -61,12 +66,15 @@ export function useMapEvents(
6166
const listener = map.addListener(
6267
eventType,
6368
(ev?: google.maps.MapMouseEvent | google.maps.IconMouseEvent) => {
64-
handler(createMapEvent(eventType, map, ev));
69+
const mapEvent = createMapEvent(eventType, map, ev);
70+
71+
trackDispatchedEvent(mapEvent, cameraStateRef);
72+
handler(mapEvent);
6573
}
6674
);
6775

6876
return () => listener.remove();
69-
}, [map, eventType, handler]);
77+
}, [map, cameraStateRef, eventType, handler]);
7078
}
7179
}
7280

@@ -189,7 +197,7 @@ const mouseEventTypes = [
189197
type MapEventPropName = keyof MapEventProps;
190198
const eventPropNames = Object.keys(propNameToEventType) as MapEventPropName[];
191199

192-
type MapEvent<T = unknown> = {
200+
export type MapEvent<T = unknown> = {
193201
type: string;
194202
map: google.maps.Map;
195203
detail: T;

src/components/map/use-map-options.ts

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,32 @@
11
import {useEffect, useLayoutEffect} from 'react';
22
import {MapProps} from '@vis.gl/react-google-maps';
3+
import {InternalCameraStateRef} from './use-internal-camera-state';
4+
import {isLatLngLiteral} from '../../libraries/is-lat-lng-literal';
35

46
/**
5-
* Internal hook to update the map-options and view-parameters when
7+
* Internal hook to update the map-options and camera parameters when
68
* props are changed.
9+
*
10+
* @param map the map instance
11+
* @param cameraStateRef stores the last values seen during dispatch into the
12+
* react-application in useMapEvents(). We can safely assume that we
13+
* don't need to feed these values back into the map.
14+
* @param mapProps the props to update the map-instance with
715
* @internal
816
*/
9-
export function useMapOptions(map: google.maps.Map | null, mapProps: MapProps) {
10-
const {center, zoom, heading, tilt, ...mapOptions} = mapProps;
17+
export function useMapOptions(
18+
map: google.maps.Map | null,
19+
cameraStateRef: InternalCameraStateRef,
20+
mapProps: MapProps
21+
) {
22+
const {center: rawCenter, zoom, heading, tilt, ...mapOptions} = mapProps;
23+
const center = rawCenter
24+
? isLatLngLiteral(rawCenter)
25+
? rawCenter
26+
: rawCenter.toJSON()
27+
: null;
28+
const lat = center && center.lat;
29+
const lng = center && center.lng;
1130

1231
/* eslint-disable react-hooks/exhaustive-deps --
1332
*
@@ -17,32 +36,48 @@ export function useMapOptions(map: google.maps.Map | null, mapProps: MapProps) {
1736
*/
1837

1938
// update the map options when mapOptions is changed
39+
// Note: due to the destructuring above, mapOptions will be seen as changed
40+
// with every re-render, so we're boldly assuming the maps-api will properly
41+
// deal with unchanged option-values passed into setOptions.
2042
useEffect(() => {
2143
if (!map) return;
2244

23-
map.setOptions(mapOptions);
45+
// Changing the mapId via setOptions will trigger an error-message.
46+
// We will re-create the map-instance in that case anyway, so we
47+
// remove it here to avoid this error-message.
48+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
49+
const {mapId, ...opts} = mapOptions;
50+
map.setOptions(opts);
2451
}, [mapOptions]);
2552

2653
useLayoutEffect(() => {
27-
if (!map || !center) return;
54+
if (!map || !Number.isFinite(lat) || !Number.isFinite(lng)) return;
55+
if (
56+
cameraStateRef.current.center.lat === lat &&
57+
cameraStateRef.current.center.lng === lng
58+
)
59+
return;
2860

29-
map.moveCamera({center});
30-
}, [center]);
61+
map.moveCamera({center: {lat: lat as number, lng: lng as number}});
62+
}, [lat, lng]);
3163

3264
useLayoutEffect(() => {
3365
if (!map || !Number.isFinite(zoom)) return;
66+
if (cameraStateRef.current.zoom === zoom) return;
3467

3568
map.moveCamera({zoom: zoom as number});
3669
}, [zoom]);
3770

3871
useLayoutEffect(() => {
3972
if (!map || !Number.isFinite(heading)) return;
73+
if (cameraStateRef.current.heading === heading) return;
4074

4175
map.moveCamera({heading: heading as number});
4276
}, [heading]);
4377

4478
useLayoutEffect(() => {
4579
if (!map || !Number.isFinite(tilt)) return;
80+
if (cameraStateRef.current.tilt === tilt) return;
4681

4782
map.moveCamera({tilt: tilt as number});
4883
}, [tilt]);
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export function isLatLngLiteral(
2+
obj: unknown
3+
): obj is google.maps.LatLngLiteral {
4+
if (!obj || typeof obj !== 'object') return false;
5+
if (!('lat' in obj && 'lng' in obj)) return false;
6+
7+
return Number.isFinite(obj.lat) && Number.isFinite(obj.lng);
8+
}

0 commit comments

Comments
 (0)