Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: offsetHeight null error (#259) #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lift46252
Copy link

@lift46252 lift46252 commented Jul 3, 2024

remove findDOMNode and used element instead

issue to fix #259

Copy link

vercel bot commented Jul 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
virtual-list ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 3, 2024 7:11am

@@ -30,10 +29,10 @@ export default function useHeights<T>(
const doCollect = () => {
instanceRef.current.forEach((element, key) => {
if (element && element.offsetParent) {
const htmlElement = findDOMNode<HTMLElement>(element);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rc-util/findDOMNode will return element directly if its HTMLElement.

@yukui630
Copy link

这个问题修复了吗?在哪个版本?

@yoyo837
Copy link
Member

yoyo837 commented Aug 28, 2024

这个问题修复了吗?在哪个版本?

没有,希望先给个复现找找问题是什么

@yukui630
Copy link

ReactDOM.findDOMNode 不是在React 17中已经弃用了吗
image
image

@yukui630
Copy link

我这里是用qiankun嵌入到主应用之后,子应用所有涉及到select下拉的地方都报错

@yukui630
Copy link

我把rc-util的findDOMNode修改直接返回node 也能解决这个问题,代码能否做兼容
image

@yoyo837
Copy link
Member

yoyo837 commented Aug 28, 2024

我们希望能看看原始问题是什么,你截图的废弃方法只是一个兜底行为。

@yukui630
Copy link

我们希望能看看原始问题是什么,你截图的废弃方法只是一个兜底行为。

这个问题我们排查出来了,复现仓库提供有点困难。出现的原因是使用了wangeditor且添加了wangeditor的公式插件@wangeditor/plugin-formula,就是这个公式插件导致的问题,并且单独打开子应用访问的时候没有报错,通过qiankun嵌入到主应用后会报错。

@zombieJ
Copy link
Member

zombieJ commented Aug 29, 2024

没有复现就没办法看具体原因是什么了,这个 PR 本身是不解决问题的。如果元素非有效 ele 就会导致收集高度失败而无法滚动。

@lift46252
Copy link
Author

lift46252 commented Sep 2, 2024

@zombieJ hello sorry for late reply here is a reproductable demo of our case https://stackblitz.com/edit/vitejs-vite-cguoy3?file=src%2FApp.tsx
Снимок экрана от 2024-09-02 10-15-46

to work please update the iframe URL to current url of page in stackblitz (page should be opened in new tab)

video with steps
https://jmp.sh/bkYSU56w

@yoyo837
Copy link
Member

yoyo837 commented Sep 2, 2024

@zombieJ hello sorry for late reply here is a reproductable demo of our case https://stackblitz.com/edit/vitejs-vite-cguoy3?file=src%2FApp.tsx Снимок экрана от 2024-09-02 10-15-46

I don't see this error in the console

@lift46252
Copy link
Author

lift46252 commented Sep 2, 2024

@zombieJ hello sorry for late reply here is a reproductable demo of our case https://stackblitz.com/edit/vitejs-vite-cguoy3?file=src%2FApp.tsx Снимок экрана от 2024-09-02 10-15-46

I don't see this error in the console

@yoyo837 @zombieJ i updated comment with needed steps to appear need to update iframe URL and open in new tab

@yoyo837
Copy link
Member

yoyo837 commented Sep 2, 2024

image

@lift46252
Copy link
Author

lift46252 commented Sep 2, 2024

image

@yoyo837 are you changed the iframe URL ? if yes try and to open in new tab
image

@yoyo837
Copy link
Member

yoyo837 commented Sep 2, 2024

@yoyo837 are you changed the iframe URL ?

Now, the new screenshots, yes, I changed it.

image

@yoyo837
Copy link
Member

yoyo837 commented Sep 2, 2024

Oh, I saw it now.

@lift46252
Copy link
Author

Oh, I saw it now.

@yoyo837 sorry ,but can you please confirm you can reproduce the error or need to make some changes in demo ?

@yoyo837
Copy link
Member

yoyo837 commented Sep 2, 2024

Oh, I saw it now.

@yoyo837 sorry ,but can you please confirm you can reproduce the error or need to make some changes in demo ?

Yes, The operation to reproduce the problem is more complicated, but it is much better than not being able to reproduce it.

@zombieJ
Copy link
Member

zombieJ commented Sep 2, 2024

Thanks for your reproduce. It seems rc-util isDOM function not working with iframe element check which is not rc-virtual-list bug:

ref https://github.com/react-component/util/blob/master/src/Dom/findDOMNode.ts#L4

@zombieJ
Copy link
Member

zombieJ commented Sep 2, 2024

Thanks for your reproduce. It seems rc-util isDOM function not working with iframe element check which is not rc-virtual-list bug:

ref https://github.com/react-component/util/blob/master/src/Dom/findDOMNode.ts#L4

@lift46252, do you have interested to handle this?

@lift46252
Copy link
Author

Thanks for your reproduce. It seems rc-util isDOM function not working with iframe element check which is not rc-virtual-list bug:
ref https://github.com/react-component/util/blob/master/src/Dom/findDOMNode.ts#L4

@lift46252, do you have interested to handle this?

@zombieJ yes i will try but this will take some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useHeights内htmlElement为null兼容
4 participants