Skip to content

Commit

Permalink
fix: don't use potentially unreliable addListener functions (#158)
Browse files Browse the repository at this point in the history
As discovered in #137, there are apparently situations where the `.addListener()` function that components inherit from the MVCObject class will return undefined in certain edge cases, like when the Map is rendering the fallback-screen for an invalid API-key.

Therefore we will use the hopefully more stable `google.maps.event.addListener` method everywhere.

Co-authored-by: Malte Modrow <[email protected]>
  • Loading branch information
usefulthink and mrMetalWood authored Jan 17, 2024
1 parent eb3fb7d commit 7309efa
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 18 deletions.
6 changes: 5 additions & 1 deletion src/components/__tests__/marker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,9 @@ test('marker should have a click listener', async () => {
const markerMocks = mockInstances.get(google.maps.Marker);
const markerMock = markerMocks[0];

expect(markerMock.addListener).toHaveBeenCalledWith('click', handleClick);
expect(google.maps.event.addListener).toHaveBeenCalledWith(
markerMock,
'click',
handleClick
);
});
13 changes: 7 additions & 6 deletions src/components/advanced-marker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,22 @@ function useAdvancedMarker(props: AdvancedMarkerProps) {
useEffect(() => {
if (!marker) return;

const m = marker;
const gme = google.maps.event;

if (onClick) marker.addListener('click', onClick);
if (onDrag) marker.addListener('drag', onDrag);
if (onDragStart) marker.addListener('dragstart', onDragStart);
if (onDragEnd) marker.addListener('dragend', onDragEnd);
if (onClick) gme.addListener(marker, 'click', onClick);
if (onDrag) gme.addListener(marker, 'drag', onDrag);
if (onDragStart) gme.addListener(marker, 'dragstart', onDragStart);
if (onDragEnd) gme.addListener(marker, 'dragend', onDragEnd);

if ((onDrag || onDragStart || onDragEnd) && !draggable) {
console.warn(
'You need to set the marker to draggable to listen to drag-events.'
);
}

const m = marker;
return () => {
google.maps.event.clearInstanceListeners(m);
gme.clearInstanceListeners(m);
};
}, [marker, draggable, onClick, onDragStart, onDrag, onDragEnd]);

Expand Down
2 changes: 1 addition & 1 deletion src/components/info-window.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const InfoWindow = (props: PropsWithChildren<InfoWindowProps>) => {
infoWindow.open({map, anchor});

if (onCloseClick) {
infoWindow.addListener('closeclick', () => {
google.maps.event.addListener(infoWindow, 'closeclick', () => {
onCloseClick();
});
}
Expand Down
3 changes: 2 additions & 1 deletion src/components/map/use-map-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ export function useMapEvents(
if (!map) return;
if (!handler) return;

const listener = map.addListener(
const listener = google.maps.event.addListener(
map,
eventType,
(ev?: google.maps.MapMouseEvent | google.maps.IconMouseEvent) => {
const mapEvent = createMapEvent(eventType, map, ev);
Expand Down
16 changes: 9 additions & 7 deletions src/components/marker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,19 @@ function useMarker(props: MarkerProps) {
const m = marker;

// Add event listeners
if (onClick) m.addListener('click', onClick);
if (onDrag) m.addListener('drag', onDrag);
if (onDragStart) m.addListener('dragstart', onDragStart);
if (onDragEnd) m.addListener('dragend', onDragEnd);
if (onMouseOver) m.addListener('mouseover', onMouseOver);
if (onMouseOut) m.addListener('mouseout', onMouseOut);
const gme = google.maps.event;

if (onClick) gme.addListener(m, 'click', onClick);
if (onDrag) gme.addListener(m, 'drag', onDrag);
if (onDragStart) gme.addListener(m, 'dragstart', onDragStart);
if (onDragEnd) gme.addListener(m, 'dragend', onDragEnd);
if (onMouseOver) gme.addListener(m, 'mouseover', onMouseOver);
if (onMouseOut) gme.addListener(m, 'mouseout', onMouseOut);

marker.setDraggable(Boolean(draggable));

return () => {
google.maps.event.clearInstanceListeners(m);
gme.clearInstanceListeners(m);
};
}, [
marker,
Expand Down
8 changes: 6 additions & 2 deletions src/libraries/google-maps-api-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const MAPS_API_BASE_URL = 'https://maps.googleapis.com/maps/api/js';
export class GoogleMapsApiLoader {
/**
* Loads the Google Maps API with the specified parameters.
* Since the maps library can only be loaded once per page, this will
* Since the Maps library can only be loaded once per page, this will
* produce a warning when called multiple times with different
* parameters.
*
Expand All @@ -39,6 +39,10 @@ export class GoogleMapsApiLoader {
const libraries = params.libraries ? params.libraries.split(',') : [];
const serializedParams = this.serializeParams(params);

// note: if google.maps.importLibrary was defined externally, the params
// will be ignored. If it was defined by a previous call to this
// method, we will check that the key and other parameters have not been
// changed in between calls.
if (!window.google?.maps?.importLibrary) {
window.__googleMapsApiParams__ = serializedParams;
this.initImportLibrary(params);
Expand Down Expand Up @@ -115,7 +119,7 @@ export class GoogleMapsApiLoader {
return apiPromise;
};

// for the first load we declare an importLibrary function that will
// for the first load, we declare an importLibrary function that will
// be overwritten once the api is loaded.
google.maps.importLibrary = libraryName =>
loadApi(libraryName).then(() => google.maps.importLibrary(libraryName));
Expand Down

0 comments on commit 7309efa

Please sign in to comment.