uiwjs / react-amap

基于 React 封装的高德地图组件,帮助你轻松的接入地图到 React 项目中。
https://uiwjs.github.io/react-amap
MIT License
418 stars 69 forks source link

React 18.3: `unmountComponentAtNode` is deprecated #367

Closed martesi closed 3 months ago

martesi commented 3 months ago

Description

unmountComponentAtNode used in usePortal is deprecated, possibly removed in React 19. Now in React 18.3, you will get an error message in console when this API is used, e.g., Marker(check the reproduction link below).

image

Assuming we continue to support React 19, we may need a workaround for its current usage.

Links

jaywcjlove commented 3 months ago

@martesi I cannot guarantee full support for React 19. All issues will be addressed in version 7 of react-amap.

martesi commented 3 months ago

@jaywcjlove Thank you for quick response. In 7.0.0, the unmountComponentAtNode warning is replaced by a new one.

image

You can check the demo here.

The original issue is solved, and this is just a warning, should I close this one, wait and see if this causes real damage?

jaywcjlove commented 3 months ago

@martesi Upgrade v7.0.1

martesi commented 3 months ago

@jaywcjlove see demo here. Click the "forcing update" button then you get a DOMException.

image

Actually, I was thinking maybe we can just use useEffect to append our holder to the container, then remove it in the cleanup. The portal can be render by user application anyway, we just need a stable host as target.

I think recourses are tied with user app, since the portal is rendered by it. So we might not need unmountComponentAtNode in the first place. I could be wrong, please tell me if I am.

jaywcjlove commented 3 months ago

@martesi This might not be an issue with the component.

<Marker
-  key={key}
  visiable={true}
  title="北京市"
  position={new AMap.LngLat(116.405285, 39.904989)}
 >
<div style={{ fontSize: 20 }}>hello </div>
</Marker>
<button
  style={{ position: "absolute", top: 0, left: 0 }}
  onClick={() => setKey((v) => ++v)}
 >
+  forcing refresh{key}
</button>
martesi commented 3 months ago

@jaywcjlove check demo here. The same code but pinned version to 7.0.0. Works as expected.

jaywcjlove commented 3 months ago

@martesi I don't know how to fix it; if you don't change the key, it works fine.

https://github.com/uiwjs/react-amap/assets/1680273/3a8198ae-5e86-4ced-9806-32f8b06e878c

martesi commented 3 months ago

@jaywcjlove try my branch forcing-refresh-old and forcing-refresh-new. I implemented what I mentioned earlier, it seems to work.

martesi commented 3 months ago

I also created a draft PR, maybe this is more convenient for you to try?

jaywcjlove commented 3 months ago

@martesi thx! Upgrade v7.0.2