fix(MeasureRow): clone the column without ref so that ref can point to the real columnHeader. #1362
Conversation
…o the real columnHeader.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Linkodt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request resolves a bug within the MeasureRow component where column titles containing ref properties could cause issues during the measurement process. The fix involves cloning the column title elements and explicitly nullifying their ref property when they are used for measurement, ensuring proper ref assignment to the actual column header and preventing unintended side effects.
Highlights
- Bug Fix for MeasureRow: Addresses an issue in the
MeasureRowcomponent where column titles, especially those containingrefproperties, were not handled correctly during the measurement process, potentially leading to errors or unexpected behavior. - Ref Handling in Column Titles: Implements a solution to explicitly clone column title elements and set their
refproperty tonullbefore passing them toMeasureCell. This ensures that therefpoints to the intendedcolumnHeaderand prevents conflicts during measurement.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
当前还存在的问题： |
There was a problem hiding this comment.
Code Review
This pull request addresses a bug where a ref on a column title was being incorrectly used by the MeasureRow component, which is intended for measuring column widths. The fix correctly clones the title element while nullifying its ref, preventing potential ref conflicts and ensuring the original ref points to the correct column header element. The implementation is clean and effectively solves the described issue. To prevent regressions, it would be beneficial to add a test case that covers this specific scenario.
|
https://github.com/react-component/table/actions/runs/17836173599/job/50718050719?pr=1362 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1362 +/- ##
=======================================
Coverage 96.10% 96.11%
=======================================
Files 57 57
Lines 3441 3445 +4
Branches 630 632 +2
=======================================
+ Hits 3307 3311 +4
Misses 129 129
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Walkthrough在 MeasureRow 中引入对列标题的清洗：从 column.title 派生 titleForMeasure。若标题为 React 元素则克隆并移除 ref（设为 null）；否则直接使用原值。随后以 titleForMeasure 替代原先的 column?.title 传入 MeasureCell。其余渲染流程保持不变。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant MR as MeasureRow
participant C as Column
participant MC as MeasureCell
MR->>C: 读取 column.title
alt 标题为 React 元素
MR->>MR: cloneElement(title, { ref: null })\n生成 titleForMeasure
else 非元素
MR->>MR: 直接使用原始 title 作为 titleForMeasure
end
MR->>MC: 渲染并传入 titleForMeasure
MC-->>MR: 完成测量渲染
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Body/MeasureRow.tsx (1)
37-39: 小优化：避免在 map 中重复线性查找列（O(n²)）。列数较多时可见。可预构建 Map 提升可读性与性能，属非功能性建议。
可以这样处理（在组件内、
columns变更时重建索引）：const columnByKey = React.useMemo( () => new Map(columns.map(c => [c.key, c] as const)), [columns], ); // 使用处 const column = columnByKey.get(columnKey);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Body/MeasureRow.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test / react component workflow
🔇 Additional comments (3)
src/Body/MeasureRow.tsx (3)
39-42: 关于“是否需要递归移除嵌套 ref”——不建议，移除顶层 ref 足够。
- 目标是防止列头外部传入的“顶层 ref”被度量行劫持；处理顶层即可达成目的。
- 深度清洗会破坏子树内部组件自用的 ref，副作用大且成本高。
- 若后续出现真实用例需要，可在受控开关下再考虑深度处理。
请确认相关 Issue 中的案例仅依赖顶层 ref（与目前修复相符）。若发现仍被子节点 ref 干扰，可补充复现用例再权衡。
48-48: 将清洗后的 title 传给 MeasureCell 的改动是正确的。这能避免度量行篡改外部 ref 的归属，符合修复目标。
40-42: 修复 TS2769：避免向 cloneElement 传递 ref（CI 类型错误）位置：src/Body/MeasureRow.tsx:40-42
React 18 的类型签名对 cloneElement 第二参较严格，直接传 { ref: null } 会在多数 @types/react 版本报 TS2769；改为用 createElement 重新构造元素可自然丢弃 ref 并避免类型问题。
- const titleForMeasure = React.isValidElement<React.RefAttributes<any>>(rawTitle) - ? React.cloneElement(rawTitle, { ref: null }) - : rawTitle; + const titleForMeasure = React.isValidElement(rawTitle) + ? React.createElement( + (rawTitle as React.ReactElement).type, + { + ...(rawTitle as React.ReactElement).props, + // 仅保留 key，不传 ref（自然被丢弃） + key: (rawTitle as React.ReactElement).key, + }, + ) + : rawTitle;验证：
- 本地运行：npx tsc --noEmit，应无类型错误。
- 手动检查 MeasureCell 中 title 渲染正常、样式/事件未丢失。
递归调用我担心有性能问题，先按你这个最简单的方式处理。 另外 Tour target 如果用 id 和 className 来拿节点，目前也会有问题，暂不处理。 |
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
ant-design/ant-design#55039
close #55039
💡 Background and Solution
clone the column title without ref
📝 Change Log
Summary by CodeRabbit