feat: add forceRender api #1386
Conversation
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
功能概览表格组件的 expandable 配置中添加了新的 forceRender 选项，允许通过布尔值或函数来控制指定行是否强制渲染展开内容，无论该行当前是否处于展开状态。 变更汇总
序列图sequenceDiagram
participant User
participant Table
participant Body
participant BodyRow
User->>Table: 传入 expandable 配置 (含 forceRender)
Table->>Table: 从 expandableConfig 提取 forceRender
Table->>Table: 将 forceRender 放入 TableContext
Body->>Body: 从 TableContext 解构 forceRender
Body->>BodyRow: 逐行传递 forceRender prop
BodyRow->>BodyRow: 调用 shouldForceRender(record, index)
alt forceRender 返回 true
BodyRow->>BodyRow: 强制渲染展开行内容
else forceRender 返回 false
BodyRow->>BodyRow: 根据现有 expanded 状态决定是否渲染
end
代码审查工作量评估🎯 3 (中等复杂度) | ⏱️ ~20-25 分钟 该 PR 涉及多个文件的协调变更，从接口定义到组件实现，再到上下文传播形成了完整的特性链路。变更模式相对一致（均为在各层添加 forceRender 支持），但需要验证：(1) 类型定义的一致性；(2) 上下文传播链路的正确性；(3) BodyRow 中的条件渲染逻辑是否正确处理了 forceRender 与现有 expanded 状态的交互。 可能相关的 PR
建议审查者
兔子的诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Summary of ChangesHello @rubbishmaker, 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 introduces a new Highlights
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new forceRender API to the expandable configuration, allowing expanded rows to be rendered even when collapsed. The implementation looks good, but I've found a few areas for improvement, mainly concerning type consistency and code clarity. I've suggested changes to align the function signatures for forceRender across different files, simplify some code, and remove a redundant dependency. These changes will improve the robustness and maintainability of the new feature.
| expandableType: ExpandableType; | ||
| expandRowByClick: boolean; | ||
| expandedRowRender: ExpandedRowRender<RecordType>; | ||
| forceRender?: boolean | ((record: RecordType) => boolean); |
There was a problem hiding this comment.
The type for forceRender is inconsistent with its usage in BodyRow.tsx, which passes both record and index to the function. To maintain consistency and allow developers to use the row index, the type should be boolean | ((record: RecordType, index: number) => boolean).
| forceRender?: boolean | ((record: RecordType) => boolean); | |
| forceRender?: boolean | ((record: RecordType, index: number) => boolean); |
| columnWidth?: number | string; | ||
| fixed?: FixedType; | ||
| expandedRowOffset?: number; | ||
| forceRender?: boolean|((record: RecordType) => boolean); |
There was a problem hiding this comment.
The type definition for forceRender is missing the index parameter in its function signature. In BodyRow.tsx, this function is called with both record and index. To ensure type consistency and provide the index to the user, the signature should be (record: RecordType, index: number) => boolean. Also, adding a space around | would improve code style consistency.
| forceRender?: boolean|((record: RecordType) => boolean); | |
| forceRender?: boolean | ((record: RecordType, index: number) => boolean); |
| forceRender: record => { | ||
| return record?.key === 0; | ||
| }, |
There was a problem hiding this comment.
This arrow function can be simplified to a single line for better readability.
forceRender: record => record?.key === 0,
| // ======================== Expand Row ========================= | ||
| let expandRowNode: React.ReactElement<ExpandedRowProps>; | ||
| if (rowSupportExpand && (expandedRef.current || expanded)) { | ||
| if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender?.())) { |
There was a problem hiding this comment.
The optional chaining ?.() is unnecessary here as shouldForceRender is a function declared in the component's scope and will always be defined. You can call it directly with shouldForceRender().
| if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender?.())) { | |
| if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender())) { |
| onTriggerExpand, | ||
| expandableConfig.expandIconColumnIndex, | ||
| expandableConfig.indentSize, | ||
| expandableConfig.forceRender, |
There was a problem hiding this comment.
This dependency expandableConfig.forceRender is redundant because the entire expandableConfig object is already included as a dependency on the next line. Including the whole object ensures that any change to its properties, including forceRender, will trigger the useMemo hook.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/examples/expandedRowRender.tsx (1)
149-151: 示例用法 OK，但建议在文档中说明回调的第二参数与行为语义
- 代码示例正确展示了“按记录强制渲染”的场景。
- 为避免歧义，建议在文档中明确
forceRender回调的第二参数（若提供）代表的含义（是展平后的可视行序号，还是源数据下标），以及“强制渲染但保持折叠”的行为语义（保持挂载用于保留内部状态）。如需，我可以补一段 README 说明与注意事项（虚拟滚动/可访问性影响等）。
src/Table.tsx (1)
957-959: 避免在 useMemo 依赖中加入整个expandableConfig对象当前同时依赖了各个字段（如
expandedRowRender、expandedRowOffset等）以及整个expandableConfig对象。由于对象引用易变，这会导致不必要的重计算与下游重渲染。[建议]
- 保留对
expandableConfig.forceRender的显式依赖；- 移除对整个
expandableConfig的依赖，保持依赖最小化（已有字段依赖已覆盖）。- expandableConfig.forceRender, - expandableConfig, + expandableConfig.forceRender,Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/examples/expandedRowRender.tsx(1 hunks)src/Body/BodyRow.tsx(4 hunks)src/Body/index.tsx(3 hunks)src/Table.tsx(2 hunks)src/context/TableContext.tsx(1 hunks)src/interface.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-08T12:53:09.293Z
Learnt from: bbb169
PR: react-component/table#1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在 `src/Table.tsx` 文件的 React 组件 `Table` 中，即使 `bodyScrollLeft` 频繁更新，也需要在 `TableContextValue` 的 `useMemo` 依赖数组中包含 `bodyScrollLeft` 和 `headerCellRefs`，因为每次滚动时重新计算 `TableContextValue` 是解决该问题所必须的。
Applied to files:
src/Body/index.tsx
🔇 Additional comments (2)
src/Body/index.tsx (1)
39-40: 传递与订阅forceRender的管线接入正确
- 已从
TableContext读取并透传到BodyRow，同时加入useContext的选择键，逻辑正确。- 小提示：该特性会在“未展开但已挂载”的情况下增加额外 DOM 行，若结合虚拟滚动或可访问性（ARIA）需要确认行为是否符合预期。
请确认：1) 使用虚拟表体时是否需要显式跳过“强制渲染”的隐藏行；2) 未展开时
ExpandedRow是否带aria-hidden或等效处理，避免读屏干扰。Also applies to: 54-55, 116-117
src/Table.tsx (1)
895-897: 将forceRender注入 TableContext 的改动正确已将
expandableConfig.forceRender暴露到TableContext，与 Body/BodyRow 的消费链路一致，符合预期。
| forceRender?: boolean | ((record: RecordType, index: number) => boolean); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
统一 forceRender 签名、去除重复调试渲染与细节优化
- 类型一致性：此处回调签名为
(record, index) => boolean，而TableContext/ExpandableConfig目前为单参数。建议统一为“带可选 index”的形式，避免调用/声明不一致。 - 细节优化：
devRenderTimes(props)在文件内被调用了两次（行 123 与 169）；保留一次即可。shouldForceRender是本地常量，无需可选链调用，直接shouldForceRender()。
export interface BodyRowProps<RecordType> {
- forceRender?: boolean | ((record: RecordType, index: number) => boolean);
+ // 与对外 API / Context 对齐，index 可选
+ forceRender?: boolean | ((record: RecordType, index?: number) => boolean);
}
@@
- if (process.env.NODE_ENV !== 'production') {
- devRenderTimes(props);
- }
+ // 调试渲染计数仅保留一次调用
@@
- const shouldForceRender = () => {
+ const shouldForceRender = () => {
if (typeof forceRender === 'function') {
- return forceRender(record, index);
+ return forceRender(record, index);
}
return !!forceRender;
};
@@
- if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender?.())) {
+ if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender())) {另需在文档中说明回调的 index 语义（展平后的行序号 vs 源数据下标），以免使用方误解。
Also applies to: 142-143, 162-168, 236-237
🏁 Script executed:
#!/bin/bash
# Gather information about BodyRow.tsx and related type definitions
echo "=== Checking BodyRow.tsx content (key lines) ==="
cat src/Body/BodyRow.tsx | head -250
echo -e "\n=== Searching for devRenderTimes calls ==="
rg -n "devRenderTimes" src/Body/BodyRow.tsx
echo -e "\n=== Searching for TableContext definition ==="
fd -e ts -e tsx | xargs grep -l "TableContext" | head -5
echo -e "\n=== Searching for ExpandableConfig definition ==="
fd -e ts -e tsx | xargs grep -l "ExpandableConfig" | head -5Length of output: 7681
🏁 Script executed:
#!/bin/bash
# Search for forceRender definitions in TableContext and ExpandableConfig
echo "=== TableContext definition for forceRender ==="
rg -n "forceRender" src/context/TableContext.tsx -A 2 -B 2
echo -e "\n=== ExpandableConfig definition ==="
rg -n "ExpandableConfig" src/interface.ts -A 10 -B 2
echo -e "\n=== Expand hook definition ==="
rg -n "forceRender" src/hooks/useExpand.ts -A 2 -B 2
echo -e "\n=== Table.tsx for forceRender ==="
rg -n "forceRender" src/Table.tsx -A 2 -B 2Length of output: 1405
🏁 Script executed:
#!/bin/bash
# Verify what index parameter represents and check ExpandableConfig for forceRender
echo "=== Check if ExpandableConfig has forceRender ==="
rg -n "interface ExpandableConfig" src/interface.ts -A 25
echo -e "\n=== Verify index usage context in BodyRow ==="
rg -n "index" src/Body/BodyRow.tsx | head -20Length of output: 1847
统一 forceRender 签名、去除重复调试渲染与细节优化
验证结果确认以下问题：
- 类型签名不一致：
TableContext（行56）和ExpandableConfig（行270）中forceRender为单参数(record: RecordType) => boolean，而BodyRow.tsx（行34-35）添加了index参数。建议统一为"带可选 index"的形式。 - 重复调试调用：
devRenderTimes(props)在行124和170被调用两次，应保留其中一次。 - 不必要的可选链：行237的
shouldForceRender?.()应改为shouldForceRender()，因为这是本地常量。
export interface BodyRowProps<RecordType> {
- forceRender?: boolean | ((record: RecordType, index: number) => boolean);
+ forceRender?: boolean | ((record: RecordType, index?: number) => boolean);
}
@@ (行 123-125)
- if (process.env.NODE_ENV !== 'production') {
- devRenderTimes(props);
- }
+ // 调试渲染计数仅保留一次调用
@@ (行 167-172)
const shouldForceRender = () => {
if (typeof forceRender === 'function') {
return forceRender(record, index);
}
return !!forceRender;
};
@@ (行 237)
- if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender?.())) {
+ if (rowSupportExpand && (expandedRef.current || expanded || shouldForceRender())) {建议同步更新 TableContext 和 ExpandableConfig 的 forceRender 类型定义，以保持接口一致性。
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/Body/BodyRow.tsx around lines 34-35, the forceRender signature includes
an extra index parameter causing type inconsistency with other definitions;
change it to (record: RecordType, index?: number) => boolean to accept an
optional index, remove the duplicated devRenderTimes(props) call so it only
executes once (keep the most appropriate invocation between lines ~124 and
~170), replace the unnecessary optional chaining at line ~237 from
shouldForceRender?.() to shouldForceRender(), and then sync the updated
forceRender signature in TableContext and ExpandableConfig type definitions to
maintain a consistent interface.
| forceRender?: boolean | ((record: RecordType) => boolean); | ||
| expandIcon: RenderExpandIcon<RecordType>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
上下文类型与对外接口对齐（允许可选 index）
为与 ExpandableConfig.forceRender 和 BodyRowProps.forceRender 一致，建议将 TableContextProps.forceRender 的函数签名统一为带可选 index：
- forceRender?: boolean | ((record: RecordType) => boolean);
+ forceRender?: boolean | ((record: RecordType, index?: number) => boolean);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| forceRender?: boolean | ((record: RecordType) => boolean); | |
| expandIcon: RenderExpandIcon<RecordType>; | |
| forceRender?: boolean | ((record: RecordType, index?: number) => boolean); | |
| expandIcon: RenderExpandIcon<RecordType>; |
🤖 Prompt for AI Agents
In src/context/TableContext.tsx around lines 56 to 57, update the
TableContextProps.forceRender function signature to match
ExpandableConfig.forceRender and BodyRowProps.forceRender by adding an optional
index parameter: change the function type from (record: RecordType) => boolean
to (record: RecordType, index?: number) => boolean; then update any related
usages and type imports in this file (and exported interfaces if any) to accept
and pass the optional index where applicable so the public API is consistent.
| forceRender?: boolean|((record: RecordType) => boolean); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
对外 API 的函数签名需要统一（建议允许可选 index 参数）
当前 ExpandableConfig<RecordType>.forceRender 定义为 boolean | ((record: RecordType) => boolean)，而在 BodyRowProps 中使用的是 (record, index) => boolean。为避免用户在传入 (record, index) 时的类型不一致，并与组件内部调用保持一致，建议将公共类型统一为带可选 index：
- forceRender?: boolean|((record: RecordType) => boolean);
+ // 与 BodyRow / TableContext 对齐，允许按行下标做条件判断
+ forceRender?: boolean | ((record: RecordType, index?: number) => boolean);同时顺便对联合类型两侧加空格，保持代码风格一致。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| forceRender?: boolean|((record: RecordType) => boolean); | |
| } | |
| // 与 BodyRow / TableContext 对齐，允许按行下标做条件判断 | |
| forceRender?: boolean | ((record: RecordType, index?: number) => boolean); | |
| } |
🤖 Prompt for AI Agents
In src/interface.ts around lines 270-271, the public API type for
ExpandableConfig<RecordType>.forceRender is currently boolean|((record:
RecordType) => boolean) which mismatches BodyRowProps usage of (record, index)
=> boolean; update the type to boolean | ((record: RecordType, index?: number)
=> boolean) so callers can optionally accept index and keep internal calls
consistent, and add spaces around the union operator to match code style; also
verify BodyRowProps uses the same (record, index?: number) => boolean signature.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1386 +/- ##
==========================================
- Coverage 96.09% 96.05% -0.05%
==========================================
Files 57 57
Lines 3434 3447 +13
Branches 632 634 +2
==========================================
+ Hits 3300 3311 +11
- Misses 129 131 +2
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
ant-design/ant-design#54798
#54798
Summary by CodeRabbit
新功能
forceRender选项，允许根据指定条件强制渲染行的展开内容。支持布尔值或自定义回调函数配置，提供更灵活的展开行控制能力。