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

useClickAway错误调用 #2435

Closed
pule-dpr opened this issue Jan 9, 2024 · 12 comments
Closed

useClickAway错误调用 #2435

pule-dpr opened this issue Jan 9, 2024 · 12 comments
Labels
wontfix This will not be worked on

Comments

@pule-dpr
Copy link

pule-dpr commented Jan 9, 2024

我看了相关的issues,试了一些说的方法比如composedPath等,还是无法解决,具体内容复现以及问题都在demo中写清楚了,我希望能得到你的解答
demo:https://codesandbox.io/p/sandbox/useclickaway-bug-forked-ngt6qk?file=%2FApp.tsx%3A60%2C67

@pule-dpr
Copy link
Author

你好,过了三天了,我的问题是在看吗?为什么没有回复,能告知一下吗?

@Damon0820
Copy link

看你的描述,log日志貌似符合预期。你点击图标,在ref里面,所以 ef?.current?.contains(e.target)true。你点击其他区域,在ref外边,所以ef?.current?.contains(e.target)false。有什么问题吗?

@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

这个问题确实在热更新时存在,但是首次挂载不会,待进一步排查问题根源~

用 dom 获取的方式是没问题的: (也有问题)

useClickAway(
  (e) => { /** xxx */ },
  () => document.querySelector("#xxx"),
);

用 ref 有问题:

useClickAway(
  (e) => { /** xxx */ },
  ref,
);

热更新后拿到的 ref 估计有问题,但还不确定根源在哪.

@crazylxr crazylxr added the bug Something isn't working label Feb 26, 2024
@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

奇怪,热更新目前测试情况如下:CRA、Vite、Umi、Dumi v2.x 创建的项目,热更新后,无法复现这个问题。目前就 ahooks 这个仓库里的 demo 中可以复现这个问题(dumi v1.x)

@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

又用了 dumi v1.x 创建一个新的项目,也是复现不了这个问题。

目前只有 ahooks 的仓库里的 demo 能复现,这个问题估计和啥配置有关~ 有点难搞

@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

@pule-dpr 你的项目里,组件第一次挂载时,会有这个问题吗?还是说热更新后才会有问题?

@liuyib liuyib added the needs more info Needs more information to continue label Feb 26, 2024
Copy link

Hi, pule-dpr.

It seems that this issue is a bit vague and lacks some necessary information.

看起来这条 issue 描述得有些模糊,缺少一些必要的信息。

@liuyib
Copy link
Collaborator

liuyib commented Feb 26, 2024

demo:codesandbox.io/p/sandbox/useclickaway-bug-forked-ngt6qk?file=%2FApp.tsx%3A60%2C67

这个在线 demo 也可以热更新后复现这个问题。

回头我再升级下这个仓库的 dumi 试试能否复现问题。

Copy link

github-actions bot commented Mar 1, 2024

Since the issue was labeled with needs more info, but no response in 3 days. This issue will be closed. If you have any questions, you can comment and reply.
由于该 issue 被标记为需要更多信息,却 3 天未收到回应。现关闭 issue,若有任何问题,可评论回复。

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2024
@liuyib liuyib removed the needs more info Needs more information to continue label Mar 1, 2024
@liuyib liuyib reopened this Mar 1, 2024
@pule-dpr
Copy link
Author

pule-dpr commented Mar 1, 2024

@pule-dpr 你的项目里,组件第一次挂载时,会有这个问题吗?还是说热更新后才会有问题?

实在不好意思有些事情处理太久没有看电脑了,我想我找到原因了:其hook内部通过js api contains来判断点击的dom是否在ref内部,也就是ref.current.contains(event.target)来判断是否触发回调,contains内部原理其实是类似虚拟dom树的递归对比方法来判断其父元素是否包含ref。因为绑定ref的dom元素内部的iconfont图标组件,由于点击状态的更新,导致父组件带动自组件的更新,iconfont组件内部会重新执行createIconFont函数,createIconFont作用是新建icontfont的实例,接收参数scriptUrl,函数内部会根据url动态创建script标签,引入js资源,然后渲染,所以点击之后每次的图标都不是之前的图标,自然ref.current.contains(event.target)这个判断就始终为false,就触发回调。所以又回到一个问题,这种表现是否需要完善这个hook呢?还是就是我使用不当?我认为useClickAway本意是点击之外元素触发,那至于之内元素是否是新元素或者是否切换元素,是不是不重要。

@liuyib
Copy link
Collaborator

liuyib commented Mar 4, 2024

点击之后每次的图标都不是之前的图标,自然ref.current.contains(event.target)这个判断就始终为false

是这个原因呢~

我认为useClickAway本意是点击之外元素触发,那至于之内元素是否是新元素或者是否切换元素,是不是不重要。

能做到这种效果更好,我后面尝试处理~

@liuyib
Copy link
Collaborator

liuyib commented Jun 5, 2024

热更新时的动态 dom 导致 useClickAway 失效,这种情况应该不是 ahooks 本身的问题,有可能是热更新机制的问题。至于怎么验证:我们手动更新 dom,如果 composedPath 有用的话,说明是热更新本身的问题,那么 ahooks 无法处理也不需要处理这个问题。

对于这点,这个 issue #2321 就是手动更新 dom 的情况,已经有 PR 在处理了,这种情况下 composedPath 是有效的。所以,热更新的情况下 composedPath 都判断不了动态 dom,那确实没办法了。

@liuyib liuyib closed this as completed Jun 5, 2024
@liuyib liuyib added wontfix This will not be worked on and removed bug Something isn't working labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants