From 40da7db47ca2f1165c02c8ca4c1acaf627727d4c Mon Sep 17 00:00:00 2001 From: Xiaoji Chen Date: Thu, 21 Apr 2022 11:57:42 -0700 Subject: [PATCH] Fix Popup and GeolocateControl under React StrictMode (#1836) --- docs/api-reference/popup.md | 4 ++-- examples/controls/src/app.tsx | 9 +++++++-- examples/controls/src/pin.tsx | 4 ++-- examples/webpack.config.local.js | 2 +- package.json | 2 +- src/components/geolocate-control.ts | 10 ++++++++++ src/components/popup.ts | 14 ++++++++++---- yarn.lock | 8 ++++---- 8 files changed, 37 insertions(+), 16 deletions(-) diff --git a/docs/api-reference/popup.md b/docs/api-reference/popup.md index 8ebb9794..10f541c8 100644 --- a/docs/api-reference/popup.md +++ b/docs/api-reference/popup.md @@ -91,11 +91,11 @@ CSS style override that applies to the popup's container. #### `onOpen`: (evt: [PopupEvent](/docs/api-reference/types.md#popupevent)) => void -Called when the popup is opened manually or programatically. +Called when the popup is opened. #### `onClose`: (evt: [PopupEvent](/docs/api-reference/types.md#popupevent)) => void -Called when the popup is closed manually or programatically. +Called when the popup is closed by the user clicking on the close button or outside (if `closeOnClick: true`). ## Source diff --git a/examples/controls/src/app.tsx b/examples/controls/src/app.tsx index e16018c8..606e9646 100644 --- a/examples/controls/src/app.tsx +++ b/examples/controls/src/app.tsx @@ -28,8 +28,14 @@ export default function App() { longitude={city.longitude} latitude={city.latitude} anchor="bottom" + onClick={e => { + // If we let the click event propagates to the map, it will immediately close the popup + // with `closeOnClick: true` + e.originalEvent.stopPropagation(); + setPopupInfo(city); + }} > - setPopupInfo(city)} /> + )), [] @@ -60,7 +66,6 @@ export default function App() { anchor="top" longitude={Number(popupInfo.longitude)} latitude={Number(popupInfo.latitude)} - closeOnClick={false} onClose={() => setPopupInfo(null)} >
diff --git a/examples/controls/src/pin.tsx b/examples/controls/src/pin.tsx index 02ac2c3c..c6693330 100644 --- a/examples/controls/src/pin.tsx +++ b/examples/controls/src/pin.tsx @@ -10,9 +10,9 @@ const pinStyle = { stroke: 'none' }; -function Pin({size = 20, onClick}: {size?: number; onClick?: () => void}) { +function Pin({size = 20}) { return ( - + ); diff --git a/examples/webpack.config.local.js b/examples/webpack.config.local.js index dc3b5fc3..485fdafb 100644 --- a/examples/webpack.config.local.js +++ b/examples/webpack.config.local.js @@ -28,7 +28,7 @@ const LOCAL_DEVELOPMENT_CONFIG = { alias: { // Imports the react-map-gl library from the src directory in this repo 'react-map-gl': SRC_DIR, - '../utils/mapboxgl': resolve(LIB_DIR, './node_modules/mapbox-gl/dist/mapbox-gl-dev.js'), + 'mapbox-gl': resolve(LIB_DIR, './node_modules/mapbox-gl/dist/mapbox-gl-dev.js'), react: resolve(LIB_DIR, './node_modules/react') } }, diff --git a/package.json b/package.json index 33b3cfb6..da9c20d5 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "babel-loader": "^8.0.0", "coveralls": "^3.0.0", "jsdom": "^15.0.0", - "mapbox-gl": "^2.1.0", + "mapbox-gl": "^2.8.0", "ocular-dev-tools": "beta", "pre-commit": "^1.2.2", "react": "^17.0.0", diff --git a/src/components/geolocate-control.ts b/src/components/geolocate-control.ts index 6160bbd6..3308aef0 100644 --- a/src/components/geolocate-control.ts +++ b/src/components/geolocate-control.ts @@ -74,6 +74,16 @@ const GeolocateControl = forwardRef( ({mapLib}) => { const gc = new mapLib.GeolocateControl(props); + // Hack: fix GeolocateControl reuse + // When using React strict mode, the component is mounted twice. + // GeolocateControl's UI creation is asynchronous. Removing and adding it back causes the UI to be initialized twice. + const setupUI = gc._setupUI; + gc._setupUI = args => { + if (!gc._container.hasChildNodes()) { + setupUI(args); + } + }; + gc.on('geolocate', e => { thisRef.current.props.onGeolocate?.(e as GeolocateResultEvent); }); diff --git a/src/components/popup.ts b/src/components/popup.ts index e736ae96..ea233ebe 100644 --- a/src/components/popup.ts +++ b/src/components/popup.ts @@ -81,19 +81,25 @@ function Popup(props: PopupProps) { const popup: MapboxPopup = useMemo(() => { const options = {...props}; const pp = new mapLib.Popup(options).setLngLat([props.longitude, props.latitude]); - pp.on('open', e => { + pp.once('open', e => { thisRef.current.props.onOpen?.(e as PopupEvent); }); - pp.on('close', e => { - thisRef.current.props.onClose?.(e as PopupEvent); - }); return pp; }, []); useEffect(() => { + const onClose = e => { + thisRef.current.props.onClose?.(e as PopupEvent); + }; + popup.on('close', onClose); popup.setDOMContent(container).addTo(map.getMap()); return () => { + // https://github.com/visgl/react-map-gl/issues/1825 + // onClose should not be fired if the popup is removed by unmounting + // When using React strict mode, the component is mounted twice. + // Firing the onClose callback here would be a false signal to remove the component. + popup.off('close', onClose); if (popup.isOpen()) { popup.remove(); } diff --git a/yarn.lock b/yarn.lock index 271d6a1d..f44ca1b5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7291,10 +7291,10 @@ map-visit@^1.0.0: dependencies: object-visit "^1.0.0" -mapbox-gl@^2.1.0: - version "2.6.1" - resolved "https://registry.yarnpkg.com/mapbox-gl/-/mapbox-gl-2.6.1.tgz#de8aadeb16b157b732d174b51aeaba0223ab71bb" - integrity sha512-faGbSZfcFuZ4GWwkWnJrRD3oICZAt/mVKnGuOmeBobCj9onfTRz270qSoOXeRBKd3po5VA2cCPI91YwA8DsAoQ== +mapbox-gl@^2.8.0: + version "2.8.0" + resolved "https://registry.yarnpkg.com/mapbox-gl/-/mapbox-gl-2.8.0.tgz#698f028575f202d25f2ccb5d6359761d1affb8e2" + integrity sha512-sP7TclFDmGhZWfcBlptnA24xO9T4a419W2Y6znGvuRXLkucnGErDIG+/7WLVDrebMYp4E2FtCG+1RtGcYVRIFQ== dependencies: "@mapbox/geojson-rewind" "^0.5.1" "@mapbox/geojson-types" "^1.0.2"