Skip to content

Commit 31a5261

Browse files
authored
fix(map): don't crash when remounting with a broken cached map instance (#984)
When the initial map creation fails (e.g. the Maps JavaScript API didn't load), the cached map instance can return a non-Node from getDiv(). The reuse path then called container.appendChild() with that value and threw. Validate the cached div before reusing the instance, and fall back to creating a fresh map otherwise. Fixes #982
1 parent ee26dd6 commit 31a5261

2 files changed

Lines changed: 54 additions & 3 deletions

File tree

src/components/__tests__/map.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,42 @@ describe('map instance caching', () => {
209209
"map isn't recreated when unmounting and remounting with regular changed options"
210210
);
211211
test.todo('removed options are handled correctly');
212+
213+
test("doesn't crash when remounting with a broken cached map instance", async () => {
214+
// simulates the case where the initial map-creation failed (e.g. the Maps
215+
// JavaScript API didn't load correctly), leaving a cached map whose
216+
// getDiv() doesn't return a usable DOM node. Remounting must not throw.
217+
const center = {lat: 53.55, lng: 10.05};
218+
219+
const {unmount} = render(
220+
<GoogleMap mapId={'broken-cache'} reuseMaps center={center} zoom={12} />,
221+
{wrapper}
222+
);
223+
await waitFor(() => expect(screen.getByTestId('map')).toBeInTheDocument());
224+
225+
// make the cached instance return a non-Node from getDiv()
226+
const cachedMap = mockInstances.get(google.maps.Map).at(-1)!;
227+
jest.mocked(cachedMap.getDiv).mockReturnValue(undefined as never);
228+
229+
unmount();
230+
createMapSpy.mockReset();
231+
232+
expect(() =>
233+
render(
234+
<GoogleMap
235+
mapId={'broken-cache'}
236+
reuseMaps
237+
center={center}
238+
zoom={12}
239+
/>,
240+
{wrapper}
241+
)
242+
).not.toThrow();
243+
244+
await waitFor(() => expect(screen.getByTestId('map')).toBeInTheDocument());
245+
// a fresh map instance should have been created instead of reusing the broken one
246+
expect(createMapSpy).toHaveBeenCalled();
247+
});
212248
});
213249

214250
describe('camera configuration', () => {

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,21 @@ export function useMapInstance(
138138
let mapDiv: HTMLElement;
139139
let map: google.maps.Map;
140140

141-
if (reuseMaps && CachedMapStack.has(cacheKey)) {
142-
map = CachedMapStack.pop(cacheKey) as google.maps.Map;
143-
mapDiv = map.getDiv();
141+
// a cached map can end up in a broken state (e.g. when the initial
142+
// map-creation failed because the Maps JavaScript API didn't load
143+
// correctly). In that case `getDiv()` doesn't return a usable DOM node,
144+
// so we have to discard the cached instance instead of trying to reuse it.
145+
const cachedMap =
146+
reuseMaps && CachedMapStack.has(cacheKey)
147+
? (CachedMapStack.pop(cacheKey) as google.maps.Map)
148+
: null;
149+
const cachedMapDiv = cachedMap?.getDiv();
150+
const reusedMap =
151+
cachedMap && cachedMapDiv instanceof Node ? cachedMap : null;
152+
153+
if (reusedMap) {
154+
map = reusedMap;
155+
mapDiv = cachedMapDiv as HTMLElement;
144156

145157
container.appendChild(mapDiv);
146158
map.setOptions(mapOptions);
@@ -151,6 +163,9 @@ export function useMapInstance(
151163
// the map.
152164
setTimeout(() => map.moveCamera({}), 0);
153165
} else {
166+
// discard a broken cached instance so it doesn't get pushed back
167+
if (cachedMap) google.maps.event.clearInstanceListeners(cachedMap);
168+
154169
mapDiv = document.createElement('div');
155170
mapDiv.style.height = '100%';
156171
container.appendChild(mapDiv);

0 commit comments

Comments
 (0)