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(TagInput): onDragSort capture context error caused by useRef #3003

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

Heising
Copy link
Contributor

@Heising Heising commented Jul 21, 2024

🤔 这个 PR 的性质是?

  • 日常 bug 修复
  • 新特性提交
  • 文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • CI/CD 改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他

🔗 相关 Issue

💡 需求背景和解决方案

📝 更新日志

  • fix(TagInput): 如文档 demo 拖拽示例,新增数据拖拽会后丢失,因为 useRef 导致无法更新 onDragSort 函数,但是函数上下文已经被改变,旧的 onDragSort 函数上下文拿旧数据 tags1 ,导致拖拽调整顺序无法拿到最新的数据 tags1 进行正确的操作,还存在有时候拖拽无法交换的小问题,但起码不会丢失用户输入的内容

  • 本条 PR 不需要纳入 Changelog

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

Copy link
Contributor

github-actions bot commented Jul 22, 2024

完成

src/hooks/useDragSorter.tsx Outdated Show resolved Hide resolved
@NWYLZW NWYLZW self-requested a review July 22, 2024 02:43
Copy link
Collaborator

@NWYLZW NWYLZW left a comment

Choose a reason for hiding this comment

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

建议按照上面 review 换一下 hook

@Heising Heising requested a review from NWYLZW July 22, 2024 05:33
@Heising
Copy link
Contributor Author

Heising commented Jul 22, 2024

建议按照上面 review 换一下 hook

我个人觉得调用者传进来的函数应该由调用者进化优化,不然要在文档声明这个函数传进去会被优化

比如 Tas 同样引用了这个拖拽,示例里面(这个也有问题 #2988 )已经添加了 useCallback,这样不就多重封装了吗,多次嵌套来判断引用

我开源项目参与经验少,你们是核心维护者,你们要是觉得这样改好,那你们直接编辑修改就好了

@NWYLZW
Copy link
Collaborator

NWYLZW commented Jul 22, 2024

建议按照上面 review 换一下 hook

我个人觉得调用者传进来的函数应该由调用者进化优化,不然要在文档声明这个函数传进去会被优化

比如 Tas 同样引用了这个拖拽,示例里面(这个也有问题 #2988 )已经添加了 useCallback,这样不就多重封装了吗,多次嵌套来判断引用

我开源项目参与经验少,你们是核心维护者,你们要是觉得这样改好,那你们直接编辑修改就好了

关于 event 类型的回调,默认应该按照最新的引用去调用。这也是 useEventCallback 的设计出来的作用。属于通用并常见的约定,并不需要在文档中特定说明,用户对这个应该是无感知被优化的。

用户做了什么是他自己决定的,但是我们作为组件库是可以尽可能让用户处于可用的状态,哪怕没有 useCallback 也不会有性能之类的问题。至于包了 useCallback,也并不是所有的用户都会这么做。

关于这么改的问题,并不是因为我们是核心开发者所以这么说才好。是因为这个更合理,并不是因为我是什么角色说的就对。写正确的代码帮助每一个使用的用户才是真正的好。

如果你觉得我说的不对,作为重视用户和每一个贡献者的社区,我们有责任为每一个人负责。所以我们会尊重你的贡献,并提出适当的 review 建议,而不是直接自己编辑修改了。

@NWYLZW NWYLZW merged commit 7e41d7a into Tencent:develop Jul 31, 2024
6 checks passed
@github-actions github-actions bot mentioned this pull request Aug 1, 2024
16 tasks
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.

3 participants