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

[Bug] Table组件hover每一行时会触发Table的重渲染 #32979

Closed
vclau-1996 opened this issue Nov 23, 2021 · 25 comments
Closed

[Bug] Table组件hover每一行时会触发Table的重渲染 #32979

vclau-1996 opened this issue Nov 23, 2021 · 25 comments
Assignees
Labels
🐛 Bug Ant Design Team had proved that this is a bug.

Comments

@vclau-1996
Copy link

重现链接

https://codesandbox.io/s/antd4-17-x-tablezu-jian-hoverxing-hui-chu-fa-chong-xuan-ran-wen-ti-fu-xian-slykl?file=/src/App.js

antd 版本

4.17.0

重现步骤

  1. 打开控制台
  2. 鼠标移入表格中的某一行观察控制台输出

期望的结果是什么?

  1. 鼠标移入表格并不会触发行中任何组件的重渲染,控制台无任何输出

实际的结果是什么?

  1. 会发现每一行的组件都会触发rerender输出"重渲染",即我移入某一行重渲染了当前表格页的所有row
  2. 相同代码antd 4.16.13无此问题
  3. 怀疑是rc-table 7.19版本更新引入的问题,本次更新引入了一个hoverContext,支持表格的hoverstyle,更新的代码链接react-component/table@2d0d403

浏览器

Chrome

浏览器版本

96.0.4664.45

操作系统

Windows

其他

@vclau-1996
Copy link
Author

vclau-1996 commented Nov 23, 2021

好像并不仅仅只是columns被重新渲染了,可以看看这个例子,每次hover到table中的某一行时都重新触发Table组件上的rowKey方法。
https://codesandbox.io/s/antd-tablezu-jian-hoverhui-chong-xuan-ran-zheng-ge-tablezu-jian-de-wen-ti-ejipm

@zombieJ
Copy link
Member

zombieJ commented Nov 23, 2021

嗯,为了解决 Table 合并行 hover 样式做了渲染更新。现行的数据结构有点问题导致在渲染之前无法确定哪些节点被影响到,比较蛋疼。

@vclau-1996
Copy link
Author

暂时没有解决办法吗?感觉数据量起来了的话性能还是存在问题的

@zombieJ
Copy link
Member

zombieJ commented Nov 23, 2021

嗯,需要将 column.render 拆分。

zombieJ added a commit to react-component/table that referenced this issue Nov 24, 2021
@DVSoftware
Copy link

This is really annoying and a huge performance hit.

@chillyistkult
Copy link
Contributor

You can mitigate it a bit by implementing shouldCellUpdate.

@DVSoftware
Copy link

You can mitigate it a bit by implementing shouldCellUpdate.

This would probably remove the ability to update the cells on MobX state change, unless i change all render methods to return the components that observe the values themselves, which would be a huge amount of work. I'm not sure why this change is implemented, as there is a perfectly functioning :hover state in CSS?

@chillyistkult
Copy link
Contributor

You can mitigate it a bit by implementing shouldCellUpdate.

This would probably remove the ability to update the cells on MobX state change, unless i change all render methods to return the components that observe the values themselves, which would be a huge amount of work. I'm not sure why this change is implemented, as there is a perfectly functioning :hover state in CSS?

The issue has already been fixed in rc-table.

We do have a wrapper for antd table that just adds a shouldCellUpdate to every column, but depending on how you pass dataSource to table it might or might not help. We generally memoize everything so for us it was even enough to just check of reference equality between prevRecord and record in shouldCellUpdate.

@DVSoftware
Copy link

Oh, cool, so the fix will probably land in ant in the upcoming weeks

@tychenjiajun
Copy link
Contributor

tychenjiajun commented Nov 29, 2021

Temporary solution: if you're using yarn, add

    "resolutions": {
        "rc-table": "7.18.1"
    },

to your package.json if you don't need the hover style.

Related react-component/table#686 . And actually there's comment pointing the issue in the original PR

@tychenjiajun
Copy link
Contributor

You can mitigate it a bit by implementing shouldCellUpdate.

This would probably remove the ability to update the cells on MobX state change, unless i change all render methods to return the components that observe the values themselves, which would be a huge amount of work. I'm not sure why this change is implemented, as there is a perfectly functioning :hover state in CSS?

I think :hover doesn't fit grouping rows well.

@vclau-1996
Copy link
Author

Temporary solution: if you're using yarn, add

    "resolutions": {
        "rc-table": "7.18.1"
    },

to your package.json if you don't need the hover style.

Related react-component/table#686 . And actually there's comment pointing the issue in the original PR

这个倒是临时解决此类问题的好方法,学习了

@DVSoftware
Copy link

DVSoftware commented Nov 29, 2021

I think :hover doesn't fit grouping rows well.

That's right, but that case can be detected and this feature can only be applied to these rows.

@alexeychirkov
Copy link

alexeychirkov commented Jan 11, 2022

I think this is still active bug even in 4.18.3
By hovering the table row - component of each cell get re-rendered (I am using "render" property of <Cell/>)

@toSayNothing
Copy link
Contributor

toSayNothing commented Jan 17, 2022

in rc-table package:

  Body(
    BodyRow(
      Cell(
        {props:hovering} // [`${cellPrefixCls}-row-hover`]: !cellProps && hovering,
      )
    )
  )

hover cell(onMouseEnter) => rerender all the cells to get the newest hovering props;


Instead of passing props to get a new classname, can we use web API eg document.querySelector in Body component to control those cells' class?
In Body, first, we store the row-cell relationship,
then create a function that use web API to add or remove hover class,
In Cell, onMouseEnter/Leave to execute that function to control the hover class.

I have done some local tests, using web API to control the element can improve the performance(prevent Cell re-rendering...).

@afc163 @zombieJ what do you think? or can I submit a PR about these changes?

@p-efimenko
Copy link

Any update about this issue? How to fix re-renrer on row hover?
Tables which have more then 10 rows lagging so much

@DVSoftware
Copy link

@p-efimenko for now, i solved it by pinning rc-table component version.

#32979 (comment)

@p-efimenko
Copy link

@DVSoftware thanks, I have seen this comment but, I don't use yarn :(
Google say do this trick with npm is complicated

@DVSoftware
Copy link

@DVSoftware thanks, I have seen this comment but, I don't use yarn :( Google say do this trick with npm is complicated

You can use https://www.npmjs.com/package/npm-force-resolutions

@vclau-1996
Copy link
Author

4.18.8 seems to have solved this problem in my project.
https://codesandbox.io/s/antd4-17-x-tablezu-jian-hoverxing-hui-chu-fa-chong-xuan-ran-wen-ti-fu-xian-slykl?file=/src/App.js

@fangqibin
Copy link

rc-table

这个会不会影响到antd表格功能

@fangqibin
Copy link

这个最终有什么好的解决方案呢

@SharkBaby
Copy link

还有一种方法是在td上绑定一个ref, 然后在这个ref上绑定focusout事件,也可以在光标在hover到其他行的时候触发事件
Event列表:https://developer.mozilla.org/en-US/docs/Web/Events
我这边是直接将ref作为ahooks里面的target,这样也能达到相同效果
https://ahooks.js.org/zh-CN/hooks/use-event-listener

import React, { useState } from 'react';
import { useEventListener } from 'ahooks';

export default () => {
  const [value, setValue] = useState('');

  useEventListener('keydown', (ev) => {
    setValue(ev.code);
  });

  return <p>Your press key is {value}</p>;
};

@martin-luo
Copy link

This issue still exists in antd@5.9.2: if I pass custom footer props in, and hover over the table row, the footer component will be re-rendered

@amBunkowski
Copy link

@afc163 what about antd 5.x?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Ant Design Team had proved that this is a bug.
Projects
None yet
Development

No branches or pull requests