-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implement hooks for editable table functionality, including data sour… #9295
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
base: master
Are you sure you want to change the base?
Conversation
…ce management, creator button rendering, and table content styling. Enhance pagination and search capabilities, and add utility functions for row key handling and toolbar rendering.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthrough重构并扩展可编辑表格内部实现:EditableTable 与 Table 拆分并新增多项内部 hooks 与 helper(行键映射、创建按钮、表单同步、分页/嵌套插入);useEditableArray/useEditableMap 重写为扁平化/重建与键索引驱动的编辑流;新增大量取消/保存/受控相关测试与若干快照调整。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI(用户)
participant ET as EditableTable
participant Hooks as 组合 Hooks
participant Form as 表单(formRef)
participant Utils as 编辑工具(useEditableArray / useEditableMap)
rect rgba(135,206,235,0.12)
UI->>ET: 触发 新增/编辑/取消/保存
ET->>Hooks: 计算 mergedDataSource / rowKey / creatorButtonProps
Hooks-->>ET: 返回渲染节点与 props
end
rect rgba(144,238,144,0.12)
ET->>Form: onValuesChange / 获取表单值
Form->>Utils: 提交编辑值或取消/保存指令
Utils->>Utils: 扁平化/索引映射/校验/更新 dataSource 或 newLineCache
Utils-->>ET: 返回更新后的 dataSource 与 editableKeys
ET-->>UI: 触发重渲染(表格、分页、按钮、编辑态)
end
note over ET,Form: 受控模式:useDeepCompareEffect 同步外部 value 至表单,排除当前正在编辑的行
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 分钟
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Files:
🧠 Learnings (2)📚 Learning: 2025-08-01T09:28:29.050ZApplied to files:
📚 Learning: 2025-08-01T09:28:29.050ZApplied to files:
🧬 Code graph analysis (2)src/utils/useEditableArray/index.tsx (2)
tests/table/editor-table.test.tsx (3)
🪛 ESLintsrc/utils/useEditableArray/index.tsx[error] 1169-1177: Unnecessary try/catch wrapper. (no-useless-catch) ⏰ 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). (6)
🔇 Additional comments (5)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 @chenshuai2144, 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 significantly refactors and enhances the 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant refactoring by extracting logic into several custom hooks to improve the modularity and maintainability of the editable table component. The changes are generally good, but I've found a few issues that need attention. These include a critical issue of a duplicated type definition, a few high-severity issues related to potential runtime errors from non-null assertions and a missing dependency in a useMemo hook that could lead to stale data, and a medium-severity issue with some redundant code. Addressing these points will enhance the robustness and quality of the new hooks.
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [maxLength, recordCreatorProps, value?.length]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useMemo hook for creatorButtonDom has a disabled ESLint rule for exhaustive dependencies. The intl object is used inside the memoized function to get a message, but it's not included in the dependency array. This can lead to stale closures, where the button text doesn't update if the intl object changes (e.g., when switching languages). Please add intl to the dependency array and remove the eslint-disable-next-line comment.
}, [intl, maxLength, recordCreatorProps, value?.length]);
| headerTitle={headerTitle} | ||
| hideToolbar={hideToolbar} | ||
| selectedRows={selectedRows} | ||
| selectedRowKeys={selectedRowKeys!} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-null assertion operator (!) is used on selectedRowKeys, but its type is (string | number | Key)[] | undefined. If selectedRowKeys is undefined, this will cause a runtime error. It's safer to provide a default value, such as an empty array, to prevent this potential crash.
selectedRowKeys={selectedRowKeys || []}
| } | ||
| return ( | ||
| <Alert<T> | ||
| selectedRowKeys={selectedRowKeys!} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The non-null assertion operator (!) is used on selectedRowKeys, but its type is (string | number | Key)[] | undefined. If propsRowSelection is not false but selectedRowKeys is undefined, this will cause a runtime error. To prevent this, you should provide a default value, like an empty array.
selectedRowKeys={selectedRowKeys || []}
| if (toolbarDom && pagination === false) { | ||
| return { | ||
| paddingBlockStart: 0, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| buttonRenderProps: Record<string, any>; | ||
| }; | ||
|
|
||
| function useEditableDataSource<DataType>({ |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
To fix the problem, we should remove the unused useEditableDataSource function declaration from src/table/components/EditableTable/index.tsx. Delete lines 209-272 that declare the function since it is not used or exported, and hence clutters the code. No changes to imports, additional method definitions, or exports are needed, since its removal does not affect the rest of the shown code.
-
Copy modified line R209
| @@ -206,71 +206,8 @@ | ||
| return result; | ||
| } | ||
|
|
||
| function useEditableDataSource<DataType>({ | ||
| actionDataSource, | ||
| editableUtils, | ||
| pagination, | ||
| getRowKey, | ||
| childrenColumnName, | ||
| }: { | ||
| actionDataSource: readonly DataType[] | undefined; | ||
| editableUtils: { | ||
| newLineRecord?: { | ||
| options?: { | ||
| position?: 'top' | 'bottom' | string; | ||
| parentKey?: React.Key; | ||
| recordKey?: React.Key; | ||
| }; | ||
| defaultValue?: DataType; | ||
| }; | ||
| }; | ||
| pagination: false | TablePaginationConfig | undefined; | ||
| getRowKey: GetRowKey<any>; | ||
| childrenColumnName?: string; | ||
| }): DataType[] { | ||
| return useMemo(() => { | ||
| const newLineConfig = editableUtils?.newLineRecord; | ||
| const baseData = Array.isArray(actionDataSource) | ||
| ? [...actionDataSource] | ||
| : []; | ||
| # (delete lines 209 through 272 altogether; do not replace with anything) | ||
|
|
||
| if (!newLineConfig?.defaultValue) { | ||
| return baseData; | ||
| } | ||
|
|
||
| const { options: newLineOptions, defaultValue } = newLineConfig; | ||
|
|
||
| if (newLineOptions?.parentKey) { | ||
| return handleNestedRowInsert( | ||
| baseData, | ||
| defaultValue, | ||
| newLineOptions, | ||
| getRowKey, | ||
| childrenColumnName || 'children', | ||
| ); | ||
| } | ||
|
|
||
| if (newLineOptions?.position === 'top') { | ||
| return [defaultValue, ...baseData]; | ||
| } | ||
|
|
||
| const pageConfig = | ||
| pagination && typeof pagination === 'object' ? pagination : undefined; | ||
|
|
||
| if (pageConfig?.current && pageConfig?.pageSize) { | ||
| return handlePaginationInsert(baseData, defaultValue, pageConfig); | ||
| } | ||
|
|
||
| return [...baseData, defaultValue]; | ||
| }, [ | ||
| actionDataSource, | ||
| childrenColumnName, | ||
| editableUtils?.newLineRecord, | ||
| getRowKey, | ||
| pagination, | ||
| ]); | ||
| } | ||
|
|
||
| /** | ||
| * 检查是否应该显示创建按钮 | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/table/components/EditableTable/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
src/table/components/EditableTable/index.tsx
🧬 Code graph analysis (1)
src/table/components/EditableTable/index.tsx (4)
src/utils/useEditableArray/index.tsx (2)
recordKeyToString(45-48)editableRowByKey(179-308)src/table/typing.ts (2)
ProTableProps(211-457)ActionType(459-476)src/descriptions/useFetchData.tsx (1)
UseFetchDataAction(9-14)src/table/utils/index.ts (1)
mergePagination(33-74)
🪛 ESLint
src/table/components/EditableTable/index.tsx
[error] 552-552: 'FormRender' is not defined.
(react/jsx-no-undef)
[error] 638-638: 'Toolbar' is not defined.
(react/jsx-no-undef)
[error] 698-698: 'Alert' is not defined.
(react/jsx-no-undef)
⏰ 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: Analyze (javascript)
🔇 Additional comments (1)
src/table/components/EditableTable/index.tsx (1)
208-216: 分页插入逻辑验证 - 无问题经验证,该组件采用客户端分页设计模式。
baseData包含组件管理的全部数据,而非仅当前页数据。第 208-216 行的逻辑是正确的:
baseData.length对比的是全部数据总数,而非当前页数据量- 当
pageSize > 总数据量时,表示数据少于一页,直接追加到末尾- 否则在当前页末尾位置插入(
insertIndex = current * pageSize - 1)该逻辑适用于组件的客户端分页设计,无需修改。
| return useMemo(() => { | ||
| const node = | ||
| search === false && type !== 'form' ? null : ( | ||
| <FormRender<T, U> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
补充缺失的组件导入
代码中使用了未导入的组件:
- 第 552 行:
FormRender未定义 - 第 638 行:
Toolbar未定义 - 第 698 行:
Alert未定义
这将导致运行时错误。请在文件顶部添加这些组件的导入语句。
运行以下脚本查找这些组件的定义位置:
Also applies to: 638-638, 698-698
🏁 Script executed:
#!/bin/bash
# 查找 FormRender, Toolbar, Alert 组件的导出位置
echo "=== Searching for FormRender ==="
ast-grep --pattern $'export $_ FormRender'
echo -e "\n=== Searching for Toolbar ==="
ast-grep --pattern $'export $_ Toolbar'
echo -e "\n=== Searching for Alert ==="
ast-grep --pattern $'export $_ Alert'Length of output: 92
🏁 Script executed:
#!/bin/bash
# 查看完整文件头部导入,确认组件是否已导入
echo "=== Checking imports at top of EditableTable file ==="
head -100 src/table/components/EditableTable/index.tsx | grep -E "^import|^from"
echo -e "\n=== Searching for FormRender usage in file ==="
rg -n "FormRender" src/table/components/EditableTable/index.tsx -A2 -B2
echo -e "\n=== Searching for Toolbar usage in file ==="
rg -n "Toolbar" src/table/components/EditableTable/index.tsx -A2 -B2
echo -e "\n=== Searching for Alert usage in file ==="
rg -n "Alert" src/table/components/EditableTable/index.tsx -A2 -B2
echo -e "\n=== Searching for these components in entire codebase ==="
echo "FormRender definitions:"
rg -n "FormRender\s*=" --type=ts --type=tsx | head -5
echo -e "\nToolbar definitions:"
rg -n "Toolbar\s*=" --type=ts --type=tsx | head -5
echo -e "\nAlert definitions:"
rg -n "Alert\s*=" --type=ts --type=tsx | head -5Length of output: 4136
🏁 Script executed:
#!/bin/bash
# 查找FormRender, Toolbar, Alert组件文件位置
echo "=== Searching for FormRender component ==="
fd -e tsx -e ts -e jsx -e js "FormRender" src/
echo -e "\n=== Searching for Toolbar component ==="
fd -e tsx -e ts -e jsx -e js "Toolbar" src/
echo -e "\n=== Searching for Alert component ==="
fd -e tsx -e ts -e jsx -e js "Alert" src/ | grep -v node_modules
echo -e "\n=== Check table subdirectory structure ==="
ls -la src/table/components/
echo -e "\n=== Search for exports in table subcomponents ==="
rg "export.*FormRender\|export.*Toolbar\|export.*Alert" src/table/ -t tsx -t tsLength of output: 871
🏁 Script executed:
#!/bin/bash
# 查找FormRender, Toolbar, Alert组件文件位置
echo "=== Searching for FormRender component ==="
fd -e tsx -e ts -e jsx -e js "FormRender" src/
echo -e "\n=== Searching for Toolbar component ==="
fd -e tsx -e ts -e jsx -e js "Toolbar" src/
echo -e "\n=== Searching for Alert component ==="
fd -e tsx -e ts -e jsx -e js "Alert" src/ | grep -v node_modules
echo -e "\n=== Check table subdirectory structure ==="
ls -la src/table/components/
echo -e "\n=== Search for exports in table subcomponents ==="
rg "export.*FormRender|export.*Toolbar|export.*Alert" src/table/ -t tsx -t tsLength of output: 871
🏁 Script executed:
#!/bin/bash
# 检查各组件导出情况
echo "=== FormRender.tsx exports ==="
cat src/table/components/Form/FormRender.tsx | head -30
echo -e "\n=== Alert directory contents ==="
ls -la src/table/components/Alert/
echo -e "\n=== Alert component file ==="
cat src/table/components/Alert/index.tsx 2>/dev/null || cat src/table/components/Alert/*.tsx | head -30
echo -e "\n=== ToolBar directory contents ==="
ls -la src/table/components/ToolBar/
echo -e "\n=== ToolBar component file ==="
cat src/table/components/ToolBar/index.tsx 2>/dev/null || cat src/table/components/ToolBar/*.tsx | head -30
echo -e "\n=== ListToolBar directory contents ==="
ls -la src/table/components/ListToolBar/
echo -e "\n=== ListToolBar component file ==="
cat src/table/components/ListToolBar/index.tsx 2>/dev/null || cat src/table/components/ListToolBar/*.tsx | head -30
echo -e "\n=== Check table index exports ==="
cat src/table/components/index.tsx 2>/dev/null | grep -E "export|FormRender|Toolbar|Alert"Length of output: 24678
🏁 Script executed:
#!/bin/bash
# Check full FormRender exports
echo "=== FormRender.tsx complete export statements ==="
rg "export" src/table/components/Form/FormRender.tsx
echo -e "\n=== Check if there's an index file re-exporting ==="
cat src/table/components/index.tsx
echo -e "\n=== Check EditableTable imports line by line ==="
head -50 src/table/components/EditableTable/index.tsxLength of output: 1927
补充缺失的组件导入并修正组件名称
代码在多处使用了未导入的组件,且存在组件名称不匹配的问题:
- 第 552 行:
FormRender需从./Form/FormRender导入 - 第 638 行:
Toolbar使用但未导入(注意:导出名称为ToolbarRender,应改用ToolBar或正确导入别名) - 第 698 行:
Alert使用但未导入(注意:实际导出名称为TableAlert)
建议在文件顶部添加:
import FormRender from './Form/FormRender';
import ToolBar from './ToolBar'; // 或 import ToolbarRender as ToolBar from './ToolBar';
import TableAlert as Alert from './Alert'; // 或在使用处改为 TableAlert然后将第 638 行的 <Toolbar<T> 改为 <ToolBar<T> 或使用正确的导入别名,第 698 行的 <Alert<T> 改为 <TableAlert<T> 或使用正确的导入别名。
🧰 Tools
🪛 ESLint
[error] 552-552: 'FormRender' is not defined.
(react/jsx-no-undef)
🤖 Prompt for AI Agents
In src/table/components/EditableTable/index.tsx around line 552 (and usages at
~638 and ~698), missing/incorrect component imports cause runtime errors: import
FormRender from './Form/FormRender'; import the toolbar component under the name
used in JSX (either import ToolBar from './ToolBar' or import { ToolbarRender as
ToolBar } from './ToolBar') and import the alert component with the correct
exported name (either import TableAlert from './Alert' and use
<TableAlert<T>.../> or import { TableAlert as Alert } from './Alert' if you
prefer using <Alert<T>.../>). Also update the JSX usages to match the chosen
import names (change <Toolbar<T> to <ToolBar<T> or vice versa, and change
<Alert<T> to <TableAlert<T> or the alias you imported).
…aging editable data sources, handling nested row inserts, and improving pagination logic. Refactor existing code for better readability and performance, while ensuring compatibility with new features. Add comprehensive tests for cancel operations in editable rows to ensure reliability.
| return { paddingBlockStart: 0 }; | ||
| } | ||
|
|
||
| if (toolbarDom && pagination === false) { |
Check warning
Code scanning / CodeQL
Useless conditional Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 11 days ago
To fix this problem, we must remove the useless conditional. The check if (toolbarDom && pagination === false) is unreachable: if toolbarDom is truthy, the previous block already executed and returned. Therefore, the entire block starting at line 202 (the conditional and its body) can be removed without altering any logic or behavior. No additional methods, imports, or definitions are needed: simply delete the unreachable conditional and its block from the function.
-
Copy modified lines R203-R205
| @@ -199,10 +199,10 @@ | ||
| return { paddingBlockStart: 0 }; | ||
| } | ||
|
|
||
| if (toolbarDom && pagination === false) { | ||
| return { paddingBlockStart: 0 }; | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| return { padding: 0 }; | ||
| }, [ | ||
| hideToolbar, |
| }); | ||
|
|
||
| // 通过 actionRender 访问 preEditRowRef | ||
| const actionConfig = editableUtils.actionRender({ |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
The best way to address the issue is to remove the declaration and assignment of the unused variable actionConfig. This will improve code clarity and eliminate unnecessary computation. The change is isolated to the snippet within the TestComponentWithRef component in the test file. Specifically, you should remove the following lines:
// 通过 actionRender 访问 preEditRowRef
const actionConfig = editableUtils.actionRender({
id: 1,
name: 'test1',
index: 0,
});This removal will not affect the rest of the code, as the returned value is not being used, and no side effects are apparent from the context provided.
| @@ -617,11 +617,6 @@ | ||
| }); | ||
|
|
||
| // 通过 actionRender 访问 preEditRowRef | ||
| const actionConfig = editableUtils.actionRender({ | ||
| id: 1, | ||
| name: 'test1', | ||
| index: 0, | ||
| }); | ||
|
|
||
| return ( | ||
| <Form> |
…ate handling. Introduce useRefFunction for better performance and clarity in editable operations, including validation and cancellation. Update logic for managing editable keys and enhance the overall structure for maintainability.
…. Update comments for clarity and improve the structure of editable key handling logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/utils/useEditableArray-cancel.test.tsx (1)
89-90: 建议改进测试辅助代码的类型安全性将
editableUtils暴露到window上是测试中访问内部状态的常见做法,但使用(window as any)会失去类型检查。建议定义明确的接口以提高类型安全性:
+declare global { + interface Window { + __editableUtils?: ReturnType<typeof useEditableArray<TestRecordType>>; + } +} + - (window as any).__editableUtils = editableUtils; + window.__editableUtils = editableUtils;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/table/Table.tsx(8 hunks)src/table/components/EditableTable/index.tsx(8 hunks)src/utils/useEditableArray/index.tsx(10 hunks)tests/utils/useEditableArray-cancel.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
tests/utils/useEditableArray-cancel.test.tsxsrc/table/components/EditableTable/index.tsxsrc/utils/useEditableArray/index.tsxsrc/table/Table.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Applied to files:
src/table/components/EditableTable/index.tsx
🧬 Code graph analysis (4)
tests/utils/useEditableArray-cancel.test.tsx (1)
src/utils/useEditableArray/index.tsx (3)
RecordKey(43-43)NewLineConfig(58-61)useEditableArray(577-1258)
src/table/components/EditableTable/index.tsx (2)
src/utils/useEditableArray/index.tsx (2)
recordKeyToString(45-48)editableRowByKey(292-326)src/utils/index.tsx (4)
runFunction(114-114)useRefFunction(135-135)useDeepCompareEffect(122-122)isDeepEqualReact(93-93)
src/utils/useEditableArray/index.tsx (1)
src/utils/useEditableMap/index.tsx (1)
AddLineOptions(41-44)
src/table/Table.tsx (3)
src/table/typing.ts (4)
ProTableProps(211-457)ActionType(459-476)UseFetchDataAction(51-61)PageInfo(39-43)src/utils/useEditableArray/index.tsx (2)
recordKeyToString(45-48)editableRowByKey(292-326)src/table/utils/index.ts (1)
mergePagination(33-74)
⏰ 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: Analyze (javascript)
🔇 Additional comments (21)
tests/utils/useEditableArray-cancel.test.tsx (2)
128-168: 测试覆盖良好测试用例正确验证了取消编辑时的回调调用和状态清理,覆盖了
onCancel参数映射和tableName上下文场景。
467-517: 验证新行取消时 newLineConfig 的传递此测试用例正确验证了取消新行编辑时
newLineConfig参数的传递,确保了新增行的取消流程与现有行的取消流程区分处理。src/table/Table.tsx (3)
92-172: useEditableDataSource hook 实现正确此 hook 正确处理了可编辑数据源的合并逻辑,包括:
- 新增行的顶部/底部位置插入
- 嵌套行的父子关系处理
- 分页场景下的插入位置计算
依赖数组完整,memo 优化得当。
174-215: useTableCardBodyStyle hook 逻辑清晰此 hook 根据不同条件正确计算卡片 body 样式:
- 无需卡片 DOM 时返回空对象
- 隐藏工具栏时设置
padding: 0- 有工具栏时设置
paddingBlockStart: 0条件判断清晰,依赖数组完整。
585-591: useEditableDataSource hook 的正确使用此处正确调用了新提取的
useEditableDataSourcehook,将数据源合并逻辑从内联代码提取到可复用的 hook 中,提高了代码的可维护性和可测试性。src/utils/useEditableArray/index.tsx (6)
225-287: rebuildTreeStructure 实现良好此函数正确地从扁平化的 Map 重建树结构,处理了:
- 父子关系的重建
- 顶部/底部插入位置
- 嵌套子节点的递归处理
逻辑清晰,实现正确。
601-624: buildDataSourceKeyIndexMap 实现正确此函数构建了索引键与行键之间的双向映射,支持嵌套记录的递归遍历。这对于
tableName场景下的键映射至关重要。实现逻辑清晰,递归处理正确。
712-729: validateCanStartEdit 验证逻辑正确此验证函数正确检查了单行编辑模式下的限制,并提供了适当的用户提示。条件判断清晰,错误消息国际化处理得当。
786-815: cancelEditable 实现增强此函数增强了取消编辑的逻辑:
- 支持键映射场景下的递归重试
- 正确清理新行缓存
- 调用新的
clearEditableState辅助函数实现正确,逻辑清晰。
834-906: 数据源更新辅助函数实现良好
updateDataSourceWithEditableRows和getCurrentEditRow两个辅助函数正确处理了:
- 编辑行的数据源更新
- 新行缓存的默认值处理
- 表单字段路径的构建
- 数据源中行的查找
逻辑清晰,职责分离良好。
1037-1089: addEditRecord 实现增强此函数增强了添加可编辑行的逻辑:
- 正确的验证(validateCanAddRecord, validateRecordKey)
- 避免重复添加已编辑的键
- 支持数据源模式和缓存模式
- 处理嵌套行的父键关系
实现正确,边界处理完善。
src/table/components/EditableTable/index.tsx (10)
23-26: 正确导入必要的工具函数已从
src/utils/useEditableArray正确导入editableRowByKey和recordKeyToString,这些函数在handleNestedRowInsert和其他辅助函数中被使用。
209-272: useEditableDataSource hook 实现正确此 hook 整合了数据源处理逻辑,包括:
- 基础数据源的复制
- 新行配置的验证
- 嵌套行和分页场景的处理
- 顶部/底部位置插入
依赖数组完整,memo 优化得当。与
src/table/Table.tsx中的同名 hook 功能一致,考虑未来是否可以复用。
137-152: RecordCreator 组件正确处理 actionRef第 144 行在调用
addEditRecord前正确检查actionRef?.current是否存在,避免了空引用错误。这是良好的防御性编程实践。
494-566: 表单同步辅助函数实现优秀
syncFormValuesExcludingEditing函数正确处理了表单值同步:
- 使用
Set优化编辑键查找性能- 使用
Map将 O(n²) 复杂度降至 O(n)- 正确保留正在编辑行的表单值
- 区分 name 模式和非 name 模式的处理
- 添加了错误边界处理
性能优化得当,逻辑清晰,注释详细。
571-622: 行键转换辅助函数实现正确行键转换辅助函数正确处理了不同场景:
convertIndexToRowKey: 数字索引转换为实际 rowKeyconvertRowKeyToIndex: rowKey 转换为数字索引buildFormFieldPath: 构建表单字段路径这些辅助函数支持了 name 模式和非 name 模式的键映射需求,实现逻辑清晰。
627-674: 数据访问方法实现完善
getRowData、getRowsData和setRowData三个方法提供了便捷的数据访问接口:
- 正确的参数验证(空值检查)
- 适当的错误处理
- 支持按索引和按键访问
setRowData正确合并了现有数据这些方法将作为
EditableFormInstance的公共 API,实现质量良好。
691-708: 受控模式同步逻辑正确第 691-698 行的
useDeepCompareEffect正确处理了受控模式下的表单值同步:
- 使用深度比较优化性能,避免频繁更新
- 正确保护正在编辑的行,避免用户输入丢失
- 条件判断清晰(仅在受控模式且表单存在时执行)
实现正确,注释清晰。
728-763: 值变化处理和可编辑属性构建良好
handleValuesChange函数正确处理了值变化回调:
- 按正确顺序调用
onValuesChange和用户回调- 仅在非受控模式下调用
onChange,避免循环更新
editableProps使用useMemo优化性能,条件性地添加onValuesChange,避免不必要的回调注册。逻辑清晰,性能优化得当。
795-805: 数据源变化时正确同步表单值在
onDataSourceChange回调中正确处理了 name 模式下的表单同步:
- 仅在 name 模式下执行同步
- 创建编辑键集合并保护正在编辑的行
- 调用
syncFormValuesExcludingEditing执行同步实现逻辑正确,避免了表单值和数据源不一致的问题。
810-846: ProFormDependency 监听值变化的优化使用
ProFormDependency监听 name 模式下的值变化,实现了高效的变化检测:
- 添加了空值检查,避免后续操作出错
- 使用
findIndex同时获取变化项和索引- 仅在找到变化项时触发回调
- 在触发回调后更新
preData,确保比较正确实现逻辑清晰,性能优化得当,注释详细。
| function useToolbarDom<T extends Record<string, any>>( | ||
| context: CreatorToolbarContext<T>, | ||
| ): React.ReactNode { | ||
| const { | ||
| toolBarRender, | ||
| headerTitle, | ||
| hideToolbar, | ||
| selectedRows, | ||
| selectedRowKeys, | ||
| tableColumn, | ||
| tooltip, | ||
| toolbar, | ||
| isLightFilter, | ||
| searchNode, | ||
| options, | ||
| optionsRender, | ||
| actionRef, | ||
| setFormSearch, | ||
| formSearch, | ||
| } = context; | ||
|
|
||
| return useMemo(() => { | ||
| if (toolBarRender === false) { | ||
| return null; | ||
| } | ||
| return ( | ||
| <Toolbar<T> | ||
| headerTitle={headerTitle} | ||
| hideToolbar={hideToolbar} | ||
| selectedRows={selectedRows} | ||
| selectedRowKeys={selectedRowKeys!} | ||
| tableColumn={tableColumn} | ||
| tooltip={tooltip} | ||
| toolbar={toolbar} | ||
| onFormSearchSubmit={(newValues) => { | ||
| setFormSearch({ | ||
| ...(formSearch || {}), | ||
| ...newValues, | ||
| }); | ||
| }} | ||
| searchNode={isLightFilter ? searchNode : null} | ||
| options={options} | ||
| optionsRender={optionsRender} | ||
| actionRef={actionRef} | ||
| toolBarRender={toolBarRender} | ||
| /> | ||
| ); | ||
| }, [ | ||
| actionRef, | ||
| formSearch, | ||
| headerTitle, | ||
| hideToolbar, | ||
| isLightFilter, | ||
| options, | ||
| optionsRender, | ||
| searchNode, | ||
| selectedRowKeys, | ||
| selectedRows, | ||
| setFormSearch, | ||
| tableColumn, | ||
| toolBarRender, | ||
| tooltip, | ||
| toolbar, | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修复 selectedRowKeys 的非空断言问题
第 462 行对 selectedRowKeys 使用了非空断言运算符,但其类型为 (string | number | Key)[] | undefined。如果 selectedRowKeys 为 undefined,将导致运行时错误。
建议提供默认值:
selectedRowKeys={selectedRowKeys!}
+ selectedRowKeys={selectedRowKeys || []}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/table/Table.tsx around lines 432 to 496, the code uses a non-null
assertion on selectedRowKeys (line ~462) but its type allows undefined, which
can crash at runtime; remove the non-null assertion and pass a safe default
instead (e.g., selectedRowKeys={selectedRowKeys ?? []}) or adjust the prop to
accept undefined in Toolbar; update the JSX prop to use the nullish-coalescing
default so selectedRowKeys is always an array when rendered.
| function useAlertDom<T extends Record<string, any>>({ | ||
| propsRowSelection, | ||
| selectedRowKeys, | ||
| selectedRows, | ||
| onCleanSelected, | ||
| tableAlertOptionRender, | ||
| tableAlertRender, | ||
| }: { | ||
| propsRowSelection: ProTableProps<T, any, any>['rowSelection']; | ||
| selectedRowKeys: (string | number | Key)[] | undefined; | ||
| selectedRows: T[]; | ||
| onCleanSelected: () => void; | ||
| tableAlertOptionRender: ProTableProps<T, any, any>['tableAlertOptionRender']; | ||
| tableAlertRender: ProTableProps<T, any, any>['tableAlertRender']; | ||
| }): React.ReactNode { | ||
| return useMemo(() => { | ||
| if (propsRowSelection === false) { | ||
| return null; | ||
| } | ||
| return ( | ||
| <Alert<T> | ||
| selectedRowKeys={selectedRowKeys!} | ||
| selectedRows={selectedRows} | ||
| onCleanSelected={onCleanSelected} | ||
| alertOptionRender={tableAlertOptionRender} | ||
| alertInfoRender={tableAlertRender} | ||
| alwaysShowAlert={propsRowSelection?.alwaysShowAlert} | ||
| /> | ||
| ); | ||
| }, [ | ||
| onCleanSelected, | ||
| propsRowSelection, | ||
| selectedRowKeys, | ||
| selectedRows, | ||
| tableAlertOptionRender, | ||
| tableAlertRender, | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修复 selectedRowKeys 的非空断言问题
第 519 行对 selectedRowKeys 使用了非空断言运算符,但其类型为 (string | number | Key)[] | undefined。如果 propsRowSelection 不为 false 但 selectedRowKeys 为 undefined,将导致运行时错误。
建议提供默认值:
- selectedRowKeys={selectedRowKeys!}
+ selectedRowKeys={selectedRowKeys || []}🤖 Prompt for AI Agents
In src/table/Table.tsx around lines 498 to 535, the code uses a non-null
assertion on selectedRowKeys (type `(string | number | Key)[] | undefined`)
which can be undefined at runtime; change the Alert prop to pass a safe default
(e.g., selectedRowKeys ?? []) instead of selectedRowKeys! so Alert always
receives an array, and ensure the dependency array remains correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/useEditableMap/index.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
src/utils/useEditableMap/index.tsx
🧬 Code graph analysis (1)
src/utils/useEditableMap/index.tsx (2)
src/utils/index.tsx (3)
useRefFunction(135-135)recordKeyToString(112-112)editableRowByKey(85-85)src/utils/useEditableArray/index.tsx (3)
RecordKey(43-43)recordKeyToString(45-48)editableRowByKey(292-326)
| const newKeys = | ||
| editableType === 'single' | ||
| ? [keyStr] | ||
| : [...(editableKeys || []), keyStr]; | ||
|
|
||
| setEditableRowKeys(newKeys); | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
请保持 editableKeys 的键类型一致
这里把 recordKey 统一转换成字符串后再写回 editableKeys。若调用方原本使用数字型 React.Key(比如 1)并依赖严格等于比较,现在会拿到字符串 '1',导致外部状态同步和 includes(1) 等判断失效。因此需要继续以原始 React.Key 形态保存,内部查找时再做字符串化即可。Based on learningsturn1code1
- const keyStr = String(recordKeyToString(recordKey));
+ const key = recordKeyToString(recordKey);
+ const keyStr = String(key);
@@
- const newKeys =
- editableType === 'single'
- ? [keyStr]
- : [...(editableKeys || []), keyStr];
+ const newKeys =
+ editableType === 'single'
+ ? [key]
+ : [...(editableKeys || []), key];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/useEditableMap/index.tsx around lines 160 to 166, the code
currently writes back stringified record keys into editableKeys (using keyStr),
which changes the original React.Key type and breaks external equality checks;
preserve the original recordKey type when updating editableKeys (use the
original recordKey value for both the 'single' and multi branches) and only
stringify keys for internal lookup/comparison operations (e.g., when checking
includes or map keys) so external consumers still receive numbers or symbols as
they originally provided.
This commit introduces a new test suite for the useEditableMap hook, covering various scenarios including editing, canceling, and saving operations. The tests ensure proper handling of single and multiple editable fields, nested fields, and controlled editable keys. Additionally, it verifies the correct invocation of callback functions and the management of editable states, enhancing overall reliability and maintainability of the hook.
|
|
||
| // 通过 actionRender 获取配置,然后手动调用 onCancel | ||
| const editableUtils = (window as any).__editableUtils; | ||
| const actionRender = editableUtils.actionRender('name'); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
To resolve the problem, the unused variable actionRender should be removed. This entails deleting line 285, which assigns editableUtils.actionRender('name'); to actionRender, and any associated code if present (none seems present). The comments above clarify the purpose, and those may be left to retain context, but the unused variable itself should be removed.
The change is confined to one line in tests/utils/useEditableMap.test.tsx.
| @@ -282,7 +282,6 @@ | ||
|
|
||
| // 通过 actionRender 获取配置,然后手动调用 onCancel | ||
| const editableUtils = (window as any).__editableUtils; | ||
| const actionRender = editableUtils.actionRender('name'); | ||
|
|
||
| // actionRender 返回的是 React 元素数组,我们需要通过内部配置来访问 onCancel | ||
| // 由于 useEditableMap 不直接暴露 onCancel,我们通过 actionRender 的配置来测试 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/utils/useEditableMap.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
tests/utils/useEditableMap.test.tsx
🧬 Code graph analysis (1)
tests/utils/useEditableMap.test.tsx (2)
src/utils/useEditableArray/index.tsx (2)
RecordKey(43-43)NewLineConfig(58-61)demos/table/edittable-rules.tsx (1)
editableKeys(17-200)
⏰ 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: Analyze (javascript)
…e and cancel operations. Update test structure to render action buttons within a Form context, ensuring proper interaction and verification of onSave and onCancel callbacks. Improve clarity of comments to reflect the testing strategy and expected behaviors.
…rolled mode updates and improve data handling. Update form value management for name mode, ensuring correct row updates in arrays. Refactor onChange behavior to trigger in controlled mode. Improve error handling and state management in useEditableArray for better reliability. Update snapshots to reflect new empty state rendering in various table demos.
…a handling in controlled mode. Update form value management for name mode to ensure accurate row updates in arrays. Improve error handling and state management in useEditableArray, including adjustments to onCancel behavior for new row edits. Enhance tests for cancel operations to cover multiple editable scenarios and ensure proper interaction with the form context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/utils/useEditableArray-cancel.test.tsx (1)
620-624: 请移除未使用的actionConfig变量。这里将
editableUtils.actionRender(...)的返回值赋给actionConfig却从未使用。仓库开启noUnusedLocals后会导致 TypeScript 直接报错,CI 也会因此失败。建议直接调用actionRender即可,无需保留这个多余的常量。- const actionConfig = editableUtils.actionRender({ + editableUtils.actionRender({ id: 1, name: 'test1', index: 0, });
🧹 Nitpick comments (15)
tests/snapshot/snapshot-demos-table-rtl_table.html (1)
641-693: 空态块内联渲染一致性良好,但建议降低快照脆弱性
- 变更方向正确,RTL 专用类名与文案无误,colspan="5" 与表头列数需保持同步。
- 建议通过快照序列化器过滤动态 style/aria 属性,或改为断言存在
.ant-empty与描述文案,减少因微小 DOM/样式变化导致的 flaky。tests/snapshot/snapshot-demos-table-theme.html (1)
872-923: 深色主题快照对颜色过敏感,建议弱化断言当前快照包含硬编码 SVG 填充色(如 #272727、#3e3e3e)。主题 token 变更或浏览器差异会导致快照频繁波动。建议:
- 仅断言
.ant-empty存在与“暂无数据”文案;- 或在序列化器中过滤 SVG 属性(fill/stroke)。
tests/snapshot/snapshot-demos-table-single.html (1)
868-920: 空态可视化提升 OK,建议断言更稳健改为完整空态 UI 合理。为降低快照维护成本,建议改成结构/文案级断言(存在
.ant-empty与“暂无数据”),避免对 SVG 与内联样式做强绑定。tests/snapshot/snapshot-demos-table-crud.html (1)
860-914: 空态渲染路径统一,注意与列数及主题同步
- 与其它快照保持一致,方向正确。
- 请确保
colspan与列数变更时同步更新;- 建议使用更稳健的断言或快照过滤,减少 UI 微调造成的快照抖动。
tests/table/snapshot/snapshot-demos-table-crud.html (1)
860-914: 两处空态块重复,建议统一快照策略同一文件出现两段相同空态 DOM。建议:
- 引入快照序列化器(过滤 SVG/内联样式);
- 或用语义断言替代整段 DOM 快照,降低重复维护成本。
Also applies to: 859-914
tests/table/snapshot/snapshot-demos-table-rtl_table.html (1)
641-693: RTL 空态渲染良好,建议加入稳定断言方向与文案正确。为减少快照波动,建议只断言
.ant-empty存在与描述文本“لا توجد بيانات”。tests/table/editor-table-two.test.tsx (1)
676-696: 新增双重校验提升稳定性,建议补充内容断言
- 使用
getRowsData与getFieldValue('table')双重校验很稳健。- 建议再断言新增行的某个关键字段(如 id 或 title)以提高语义性,而非仅长度。
tests/utils/useEditableMap.test.tsx (5)
36-43: 慎用全局 fake timers,避免与 waitFor/动画交织造成假阴性本文件在 beforeAll 启用
vi.useFakeTimers(),但未统一推进计时器。除非确需控制计时,建议:
- 直接移除 fake timers,或
- 使用
vi.useFakeTimers({ shouldAdvanceTime: true })并在需要时vi.advanceTimersByTime(...);结束处恢复定时器。
这能避免与 antd 动画/RTL waitFor 定时器交叉导致的 flaky。- beforeAll(() => { - vi.useFakeTimers(); - }); + // 如无硬依赖计时控制,可使用真实定时器,减少 flaky + // beforeAll(() => { + // vi.useFakeTimers({ shouldAdvanceTime: true }); + // });
103-106: 测试向 window 泄漏临时对象,建议在 afterEach 清理
__editableUtils/__dataSource注入到全局会跨用例残留。请在 afterEach 删除,避免污染。afterEach(() => { cleanup(); vi.clearAllMocks(); + // 清理全局注入,避免跨用例干扰 + // @ts-expect-error test globals cleanup + delete (window as any).__editableUtils; + // @ts-expect-error test globals cleanup + delete (window as any).__dataSource; });
474-532: 覆盖 onSave 返回 false 的行为,方向正确;避免依赖中文文案定位按钮测试通过渲染
actionRender并点击“保存”触发校验,解决了之前用例空转的问题,赞。为降低 i18n 变更带来的脆弱性,建议通过 role/顺序定位或给按钮容器加测试 id,而非依赖中文文案。- const saveButton = actionWrapper.getByText('保存'); + const saveButton = actionWrapper.getAllByRole('button')[0]; // 或通过更稳健选择器
534-595: 覆盖 onCancel 返回 false 的行为,结论与实现一致;同样建议弱化对文案的依赖用例验证了当前实现会仍然取消编辑状态,符合注释说明。为减少 i18n 影响,避免以“取消”文本定位,改为基于 role/次序或容器 testid。
- const cancelButton = actionWrapper.getByText('取消'); + const buttons = actionWrapper.getAllByRole('button'); + const cancelButton = buttons[buttons.length - 1];
652-685: 空数据源默认值未被断言,建议补充验证已验证可进入编辑态,但未断言传入的默认值被写入/可见。建议补一条断言,确保默认值逻辑不回归。
act(() => { fireEvent.click(wrapper.getByTestId('start-edit-name')); }); -expect(wrapper.getByTestId('editable-keys').textContent).toBe('name'); +expect(wrapper.getByTestId('editable-keys').textContent).toBe('name'); +// 补充:校验默认值已应用到数据源 +expect( + JSON.parse(wrapper.getByTestId('data-source').textContent!) +).toMatchObject({ name: 'Default Name' });tests/field/field.test.tsx (1)
1990-2024: 补强断言,验证“展开/收起”而非仅存在性当前仅断言元素存在,无法证明下拉是否被成功 toggle。建议用可访问性状态与可见性断言,减少对样式类名耦合。
可参考如下修改(示意):
const labelElement = html.baseElement.querySelector( '.ant-pro-core-field-label', ); expect(labelElement).toBeInTheDocument(); - // Check that the select component is rendered - const selectElement = html.baseElement.querySelector('.ant-select'); - expect(selectElement).toBeInTheDocument(); - - // Test that the component can be clicked (basic functionality) - await userEvent.click(labelElement!); - - // Verify that the component responds to clicks - expect(labelElement).toBeInTheDocument(); + // 使用可访问性断言验证展开/收起 + const combobox = html.getByRole('combobox'); + expect(combobox).toHaveAttribute('aria-expanded', 'false'); + + await userEvent.click(labelElement!); + await waitFor(() => { + expect(combobox).toHaveAttribute('aria-expanded', 'true'); + }); + await waitFor(() => { + expect(screen.getByRole('listbox')).toBeVisible(); + }); + + // 关闭(Esc 或点击外部) + await userEvent.keyboard('{Escape}'); + await waitFor(() => { + expect(screen.queryByRole('listbox')).not.toBeInTheDocument(); + });如担心 portal 下获取不到 listbox,可降级断言 wrapper 拥有
ant-select-open。基于测试规范“web-first assertions”。
Based on learningstests/table/sorter.test.tsx (1)
682-689: 精简断言&去重,提升可读性已加入边界检查,建议进一步用更语义化断言并去重重复代码:
- 直接对行或单元格使用
toHaveTextContent;- 抽取小工具以减少冗余查询。
示例(就地小改):
-const firstCell = rows[0].querySelector('.ant-table-cell'); -expect(firstCell?.textContent).toContain('项目 B'); -const secondCell = rows[1]?.querySelector('.ant-table-cell'); -expect(secondCell?.textContent).toContain('项目 C'); -const thirdCell = rows[2]?.querySelector('.ant-table-cell'); -expect(thirdCell?.textContent).toContain('项目 A'); +expect(rows[0]).toHaveTextContent('项目 B'); +expect(rows[1]).toHaveTextContent('项目 C'); +expect(rows[2]).toHaveTextContent('项目 A');或封装:
const cellText = (i: number) => container.querySelectorAll('.ant-table-row')[i] ?.querySelector('.ant-table-cell')?.textContent ?? '';两处断言均可复用,减少重复。
Based on learningsAlso applies to: 696-704
tests/table/snapshot/snapshot-demos-table-single.html (1)
871-920: 单表空状态快照同步,OK若后续仍出现快照波动,建议给空状态容器加
data-testid="table-empty"并在断言中锚定,降低对内部 SVG/结构的敏感度。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/table/components/EditableTable/index.tsx(8 hunks)src/utils/useEditableArray/index.tsx(10 hunks)tests/field/field.test.tsx(1 hunks)tests/snapshot/snapshot-demos-table-config-provider.html(1 hunks)tests/snapshot/snapshot-demos-table-crud.html(1 hunks)tests/snapshot/snapshot-demos-table-rtl_table.html(1 hunks)tests/snapshot/snapshot-demos-table-single.html(1 hunks)tests/snapshot/snapshot-demos-table-theme.html(1 hunks)tests/table/editor-table-two.test.tsx(1 hunks)tests/table/editor-table.test.tsx(1 hunks)tests/table/snapshot/snapshot-demos-table-config-provider.html(1 hunks)tests/table/snapshot/snapshot-demos-table-crud.html(1 hunks)tests/table/snapshot/snapshot-demos-table-rtl_table.html(1 hunks)tests/table/snapshot/snapshot-demos-table-single.html(1 hunks)tests/table/snapshot/snapshot-demos-table-theme.html(1 hunks)tests/table/sorter.test.tsx(1 hunks)tests/utils/useEditableArray-cancel.test.tsx(1 hunks)tests/utils/useEditableMap.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
tests/field/field.test.tsxtests/table/editor-table.test.tsxtests/utils/useEditableArray-cancel.test.tsxtests/table/editor-table-two.test.tsxtests/utils/useEditableMap.test.tsxsrc/utils/useEditableArray/index.tsxtests/table/sorter.test.tsxsrc/table/components/EditableTable/index.tsx
🧠 Learnings (6)
📚 Learning: 2025-08-01T09:28:29.050Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/test.mdc:0-0
Timestamp: 2025-08-01T09:28:29.050Z
Learning: Applies to test/**/* : Prefer to use web-first assertions (`toBeVisible`, `toHaveText`, etc.) whenever possible.
Applied to files:
tests/table/sorter.test.tsx
📚 Learning: 2025-08-01T09:28:29.050Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/test.mdc:0-0
Timestamp: 2025-08-01T09:28:29.050Z
Learning: Applies to test/**/* : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) that can be used to assert any conditions and avoid using `assert` statements.
Applied to files:
tests/table/sorter.test.tsx
📚 Learning: 2025-08-01T09:28:29.050Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/test.mdc:0-0
Timestamp: 2025-08-01T09:28:29.050Z
Learning: Applies to test/**/* : Use `page.getByTestId` whenever `data-testid` is defined on an element or container.
Applied to files:
tests/table/sorter.test.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Applied to files:
src/table/components/EditableTable/index.tsx
🧬 Code graph analysis (4)
tests/utils/useEditableArray-cancel.test.tsx (1)
src/utils/useEditableArray/index.tsx (3)
RecordKey(43-43)NewLineConfig(58-61)useEditableArray(579-1339)
tests/utils/useEditableMap.test.tsx (1)
src/utils/useEditableArray/index.tsx (2)
RecordKey(43-43)NewLineConfig(58-61)
src/utils/useEditableArray/index.tsx (2)
src/utils/index.tsx (3)
useRefFunction(135-135)recordKeyToString(112-112)editableRowByKey(85-85)src/utils/useEditableMap/index.tsx (1)
AddLineOptions(42-45)
src/table/components/EditableTable/index.tsx (2)
src/utils/useEditableArray/index.tsx (2)
recordKeyToString(45-48)editableRowByKey(292-328)src/utils/index.tsx (4)
runFunction(114-114)useRefFunction(135-135)useDeepCompareEffect(122-122)isDeepEqualReact(93-93)
⏰ 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). (6)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build (20.x, ubuntu-latest)
- GitHub Check: build
- GitHub Check: build (20.x, ubuntu-latest)
- GitHub Check: build
🔇 Additional comments (5)
tests/utils/useEditableMap.test.tsx (1)
165-172: 初始化断言清晰、快速通过基础状态断言简洁到位,可读性好。
tests/table/editor-table.test.tsx (1)
451-452: 可选链与空值兜底合理,LGTM该改动消除了嵌套路径缺失导致的异常风险。建议在本用例里追加一个对新子节点真实插入的断言(如校验包含
data-row-key="555"的行是否出现),与onchange(1)相互印证,防止“仅回调但未入树”的假阳性。tests/table/snapshot/snapshot-demos-table-config-provider.html (1)
652-701: 引入空状态组件快照,符合一致性将空单元格替换为标准空状态块(含 SVG 与“暂无数据”文案)是合理的快照更新。请确认该用例 locale 固定为 zh-CN,避免文案导致的快照抖动。
tests/table/snapshot/snapshot-demos-table-theme.html (1)
874-923: 主题空状态快照更新妥当深色主题下空状态 SVG/填充色变化符合预期。建议确保主题 token 固定或通过测试用 theme provider 固化,降低快照变更频率。
tests/snapshot/snapshot-demos-table-config-provider.html (1)
652-701: 空状态快照一致化,LGTM该快照与其它场景保持一致的空状态渲染,无需调整。
| const eachIndex = (parentIndex || 0) * 10 + index; | ||
| const recordKey = getRowKey(record, eachIndex).toString(); | ||
|
|
||
| const hasChildren = | ||
| record && typeof record === 'object' && childrenColumnName in record; | ||
|
|
||
| if (hasChildren) { | ||
| const children = (record as any)[childrenColumnName] || []; | ||
| const childrenMap = flattenRecordsToMap( | ||
| children, | ||
| getRowKey, | ||
| childrenColumnName, | ||
| recordKey, | ||
| eachIndex, | ||
| ); | ||
| childrenMap.forEach((value, key) => kvMap.set(key, value)); | ||
| } | ||
|
|
||
| const newRecord = { | ||
| ...record, | ||
| map_row_key: recordKey, | ||
| map_row_parentKey: parentKey, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修正嵌套索引计算避免键值被覆盖。
eachIndex = (parentIndex || 0) * 10 + index 在父节点索引 ≥1 且子节点索引超过一位数时会产生碰撞。例如父节点索引为 1、子索引 2 与父索引 0、子索引 12 均得到 12,导致 kvMap.set(recordKey, newRecord) 覆盖已有记录。树形数据一旦被覆盖,后续编辑/保存都会错行,属于阻断性缺陷。
建议改为为每个节点构建层级路径(字符串),并在回退到索引键时使用该路径保证唯一性:
-function flattenRecordsToMap<RecordType>(
- records: RecordType[],
- getRowKey: GetRowKey<RecordType>,
- childrenColumnName: string,
- parentKey?: React.Key,
- parentIndex?: number,
+function flattenRecordsToMap<RecordType>(
+ records: RecordType[],
+ getRowKey: GetRowKey<RecordType>,
+ childrenColumnName: string,
+ parentKey?: React.Key,
+ parentPath: string = '',
): Map<
string,
RecordType & { map_row_key?: string; map_row_parentKey?: React.Key }
> {
const kvMap = new Map<
@@
- const eachIndex = (parentIndex || 0) * 10 + index;
- const recordKey = getRowKey(record, eachIndex).toString();
+ const path = parentPath ? `${parentPath}_${index}` : index.toString();
+ const originKey = getRowKey(record, index);
+ const normalizedKey = recordKeyToString(originKey);
+ const recordKey =
+ parentKey != null && (normalizedKey == null || normalizedKey === index)
+ ? path
+ : (normalizedKey ?? path).toString();
@@
- const childrenMap = flattenRecordsToMap(
- children,
- getRowKey,
- childrenColumnName,
- recordKey,
- eachIndex,
- );
+ const childrenMap = flattenRecordsToMap(
+ children,
+ getRowKey,
+ childrenColumnName,
+ recordKey,
+ path,
+ );这样可以在保留原有 rowKey 的前提下,为缺省 key 的树节点提供稳定且唯一的路径标识,避免编辑映射被破坏。
Committable suggestion skipped: line range outside the PR's diff.
| // 通过 actionRender 获取配置,然后手动调用 onCancel | ||
| const editableUtils = (window as any).__editableUtils; | ||
| const actionRender = editableUtils.actionRender('name'); | ||
|
|
||
| // actionRender 返回的是 React 元素数组,我们需要通过内部配置来访问 onCancel | ||
| // 由于 useEditableMap 不直接暴露 onCancel,我们通过 actionRender 的配置来测试 | ||
| // 这里我们测试 cancelEditable 的行为,它应该清除编辑状态 | ||
| act(() => { | ||
| editableUtils.cancelEditable('name'); | ||
| }); | ||
|
|
||
| await waitFor(() => { | ||
| expect(wrapper.getByTestId('editable-keys').textContent).toBe('none'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
未使用的本地变量 actionRender,可删除以消除噪音
此处获取 actionRender 却未使用,可能触发 lint/静态扫描告警。可直接移除或补充断言。
- const actionRender = editableUtils.actionRender('name');
- // actionRender 返回的是 React 元素数组,我们需要通过内部配置来访问 onCancel
- // 由于 useEditableMap 不直接暴露 onCancel,我们通过 actionRender 的配置来测试
- // 这里我们测试 cancelEditable 的行为,它应该清除编辑状态
+ // 直接测试 cancelEditable 行为
act(() => {
editableUtils.cancelEditable('name');
});📝 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.
| // 通过 actionRender 获取配置,然后手动调用 onCancel | |
| const editableUtils = (window as any).__editableUtils; | |
| const actionRender = editableUtils.actionRender('name'); | |
| // actionRender 返回的是 React 元素数组,我们需要通过内部配置来访问 onCancel | |
| // 由于 useEditableMap 不直接暴露 onCancel,我们通过 actionRender 的配置来测试 | |
| // 这里我们测试 cancelEditable 的行为,它应该清除编辑状态 | |
| act(() => { | |
| editableUtils.cancelEditable('name'); | |
| }); | |
| await waitFor(() => { | |
| expect(wrapper.getByTestId('editable-keys').textContent).toBe('none'); | |
| }); | |
| }); | |
| // 通过 actionRender 获取配置,然后手动调用 onCancel | |
| const editableUtils = (window as any).__editableUtils; | |
| // 直接测试 cancelEditable 行为 | |
| act(() => { | |
| editableUtils.cancelEditable('name'); | |
| }); | |
| await waitFor(() => { | |
| expect(wrapper.getByTestId('editable-keys').textContent).toBe('none'); | |
| }); |
🤖 Prompt for AI Agents
In tests/utils/useEditableMap.test.tsx around lines 283 to 297, the local
variable actionRender is retrieved but never used, causing a
lint/static-analysis warning; remove the unused assignment (const actionRender =
editableUtils.actionRender('name');) or replace it with a meaningful assertion
that verifies actionRender's shape/behavior before calling cancelEditable, so
the test either eliminates the dead variable or asserts on the returned value to
justify its presence.
…sistent type conversion. Update onChange logging in editor-table tests for better debugging and data tracking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/utils/useEditableArray/index.tsx (1)
192-193: 修正已标记的关键问题这些问题在之前的审查中已被标记但尚未修复:
- 第 192-193 行的嵌套索引计算会导致键值碰撞 (阻断性缺陷):
eachIndex = (parentIndex || 0) * 10 + index在父节点索引 ≥1 且子节点索引超过一位数时会产生相同的键值- 第 215 行使用
delete直接修改对象 违反了不可变性原则请参考之前的审查意见进行修复。
Also applies to: 215-217
🧹 Nitpick comments (1)
src/utils/useEditableArray/index.tsx (1)
1107-1107: 简化冗余的条件判断第 1107 行的条件包含冗余检查。当
recordKey == null为真时(recordKey 是 null 或 undefined),后续的recordKey !== 0和recordKey !== ''必然为真。建议简化为:
- if (recordKey == null && recordKey !== 0 && recordKey !== '') { + if (recordKey == null) {这样更清晰地表达了意图:拒绝 null 和 undefined,但允许 0 和空字符串作为有效的键。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/utils/useEditableArray/index.tsx(11 hunks)tests/table/editor-table.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
tests/table/editor-table.test.tsxsrc/utils/useEditableArray/index.tsx
🧬 Code graph analysis (1)
src/utils/useEditableArray/index.tsx (1)
src/utils/useEditableMap/index.tsx (1)
AddLineOptions(42-45)
⏰ 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). (6)
- GitHub Check: build (20.x, ubuntu-latest)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: build (20.x, ubuntu-latest)
🔇 Additional comments (6)
tests/table/editor-table.test.tsx (1)
455-455: 代码改进:使用可选链和空值合并将非空断言操作符(
!)替换为可选链(?.)和空值合并(??)是一个很好的安全改进,避免了在访问嵌套属性时可能出现的运行时错误。这与 PR 中更广泛的数据访问安全重构保持一致。src/utils/useEditableArray/index.tsx (5)
225-289: 重建逻辑依赖于扁平化函数的正确性
rebuildTreeStructure的实现逻辑基本合理,但其正确性完全依赖于flattenRecordsToMap的输出。由于flattenRecordsToMap存在键值碰撞问题(第 192-193 行),可能导致重建的树结构数据错误。
602-637: 双向映射逻辑正确
buildDataSourceKeyIndexMap的实现清晰且正确地构建了索引键与实际行键之间的双向映射,正确处理了嵌套结构和递归遍历。
904-1004: 良好的函数拆分重构将
onValuesChange的复杂逻辑拆分为buildFormFieldPath、updateDataSourceWithEditableRows和getCurrentEditRow等辅助函数,显著提高了代码的可读性和可维护性。每个函数职责单一,逻辑清晰。
1119-1179: 清晰的验证流程和模式处理
addEditRecord的重构提高了代码质量:
- 使用
validateCanAddRecord和validateRecordKey进行验证- 正确处理重复键的情况
- 清晰区分 dataSource 模式和 cache 模式
- 支持 parentKey 作为函数的灵活用法
1188-1237: 保存逻辑正确区分了根级和嵌套记录
actionSaveRef的实现正确处理了两种场景:
- 第 1202-1209 行: 根级新增记录直接添加到 dataSource
- 第 1210-1231 行: 嵌套新增或更新记录使用
editableRowByKey维护树结构这种区分确保了树形数据的完整性。第 1234 行统一调用
cancelEditable退出编辑模式,避免了状态不一致。
| // 如果提供了 onCancel,尝试调用它(用于测试场景) | ||
| // 注意:在实际使用中,onCancel 应该在 CancelEditableAction 中被调用 | ||
| if (props.onCancel && isInEditableSet) { | ||
| const keyForFind = Array.isArray(recordKey) ? recordKey[0] : recordKey; | ||
| const record = findRecordByKey(keyForFind); | ||
| const originRow = preEditRowRef.current; | ||
| // 比较 recordKey 时需要考虑类型转换 | ||
| // newLineRecordCache.options.recordKey 是 addEditRecord 时设置的 recordKey | ||
| // 而 recordKey 是 cancelEditable 的参数,需要确保它们匹配 | ||
| const cacheRecordKey = newLineRecordCache?.options?.recordKey; | ||
| const cacheKey = | ||
| cacheRecordKey != null ? recordKeyToString(cacheRecordKey) : null; | ||
| const cacheKeyStr = cacheKey != null ? cacheKey.toString() : null; | ||
| // 检查 newLineRecordCache 是否匹配当前的 recordKey | ||
| const newLineConfig = | ||
| newLineRecordCache != null && | ||
| cacheRecordKey != null && | ||
| (cacheRecordKey === recordKey || | ||
| (cacheKeyStr != null && | ||
| relayKeyStr != null && | ||
| cacheKeyStr === relayKeyStr) || | ||
| cacheRecordKey?.toString() === recordKey?.toString() || | ||
| String(cacheRecordKey) === String(recordKey)) | ||
| ? newLineRecordCache | ||
| : undefined; | ||
|
|
||
| // 调用 onCancel,即使找不到记录(新行编辑场景) | ||
| // 对于新行编辑,record 可能为 null,但 newLineConfig 应该包含 defaultValue | ||
| try { | ||
| await props.onCancel( | ||
| recordKey, | ||
| record || (newLineConfig?.defaultValue as any) || ({} as any), | ||
| originRow || | ||
| record || | ||
| (newLineConfig?.defaultValue as any) || | ||
| ({} as any), | ||
| newLineConfig, | ||
| ); | ||
| } catch (error) { | ||
| // 如果 onCancel 抛出异常,仍然继续清理状态 | ||
| console.error('onCancel error:', error); | ||
| } | ||
| } | ||
|
|
||
| // 清理 newLineRecordCache,需要比较 recordKey(考虑类型转换) | ||
| if (newLineRecordCache) { | ||
| const cacheRecordKey = newLineRecordCache.options.recordKey; | ||
| // 重用之前计算的 relayKeyStr | ||
| const cacheKeyStr = | ||
| cacheRecordKey != null | ||
| ? recordKeyToString(cacheRecordKey)?.toString() | ||
| : null; | ||
| if ( | ||
| cacheRecordKey === recordKey || | ||
| (cacheKeyStr != null && | ||
| relayKeyStr != null && | ||
| cacheKeyStr === relayKeyStr) || | ||
| cacheRecordKey?.toString() === recordKey?.toString() || | ||
| String(cacheRecordKey) === String(recordKey) | ||
| ) { | ||
| setNewLineRecordCache(undefined); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
提取重复的键比较逻辑
第 830-845 行和第 866-883 行包含几乎相同的键比较逻辑。这种重复增加了维护成本,且多个比较条件表明键类型不一致。
建议提取为辅助函数:
+const isRecordKeyEqual = (key1: RecordKey | undefined, key2: RecordKey): boolean => {
+ if (key1 == null) return false;
+ const str1 = recordKeyToString(key1)?.toString();
+ const str2 = recordKeyToString(key2)?.toString();
+ return (
+ key1 === key2 ||
+ (str1 != null && str2 != null && str1 === str2) ||
+ key1?.toString() === key2?.toString() ||
+ String(key1) === String(key2)
+ );
+};然后简化比较逻辑:
- const newLineConfig =
- newLineRecordCache != null &&
- cacheRecordKey != null &&
- (cacheRecordKey === recordKey ||
- (cacheKeyStr != null &&
- relayKeyStr != null &&
- cacheKeyStr === relayKeyStr) ||
- cacheRecordKey?.toString() === recordKey?.toString() ||
- String(cacheRecordKey) === String(recordKey))
- ? newLineRecordCache
- : undefined;
+ const newLineConfig =
+ newLineRecordCache != null && isRecordKeyEqual(cacheRecordKey, recordKey)
+ ? newLineRecordCache
+ : undefined;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/useEditableArray/index.tsx around lines 821 to 883, extract the
duplicated key-comparison logic used in lines ~830-845 and ~866-883 into a
single helper (e.g. function isSameRecordKey(a: any, b: any, relayKeyStr?:
string): boolean) that performs the same null-safe checks currently repeated
(strict equality, recordKeyToString comparison, toString()/String() fallbacks)
and reuse it in both places: use it to compute newLineConfig (pass
cacheRecordKey and recordKey and relayKeyStr) and to decide whether to clear
newLineRecordCache; keep existing use of recordKeyToString inside the helper and
preserve types/null checks so behavior is unchanged.
| console.log( | ||
| 'onChange called with data:', | ||
| JSON.stringify(data, null, 2), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
移除调试代码
测试中遗留了 console.log 调试语句,会污染测试输出。建议在提交前移除。
应用此 diff 移除调试代码:
- onChange={(data) => {
- console.log(
- 'onChange called with data:',
- JSON.stringify(data, null, 2),
- );
- onchange(data[0]?.children?.[0]?.children?.length ?? 0);
+ onChange={(data) => {
+ onchange(data[0]?.children?.[0]?.children?.length ?? 0);
}}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tests/table/editor-table.test.tsx around lines 451 to 454 there is a leftover
console.log used for debugging; remove the console.log(...) call (and any
adjacent temporary debug-only code) so tests don't produce extraneous output,
then run the test suite to confirm no other debug logs remain.
…led mode synchronization. Improve form value management by ensuring accurate updates from props and form state. Update tests to validate form value synchronization and enhance error handling in editable scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/useEditableArray/index.tsx (1)
1132-1145: parentKey 为函数时的校验行为可能不符合预期
validateCanAddRecord中对parentKey的存在性检验直接用的是options?.parentKey:if ( options?.parentKey && !dataSourceKeyIndexMapRef.current.has( recordKeyToString(options?.parentKey).toString(), ) ) { console.warn("can't find record by key", options?.parentKey); return false; }但在
addEditRecord里真正使用 parentKey 时,会先对函数型parentKey调用一次:const parentKeyValue = typeof options?.parentKey === 'function' ? (options.parentKey as any)() : options?.parentKey;也就是说,如果外部通过
parentKey: () => 624674790传入函数形式,此处的校验会对函数本身调用recordKeyToString,而不是对实际返回值,has一定失败,从而误报can't find record by key。当前大部分调用(包括组件内部的
RecordCreator)已经在传到addEditRecord之前把parentKey解析成具体值,所以不会立刻出问题。但从 API 语义上看,校验应与实际使用的值保持一致,建议在这里也复用parentKeyValue的逻辑:- const validateCanAddRecord = useRefFunction( - (options?: AddLineOptions): boolean => { + const validateCanAddRecord = useRefFunction( + (options?: AddLineOptions): boolean => { + const parentKeyValue = + typeof options?.parentKey === 'function' + ? (options.parentKey as any)() + : options?.parentKey; if ( - options?.parentKey && - !dataSourceKeyIndexMapRef.current.has( - recordKeyToString(options?.parentKey).toString(), - ) + parentKeyValue != null && + !dataSourceKeyIndexMapRef.current.has( + recordKeyToString(parentKeyValue).toString(), + ) ) { - console.warn("can't find record by key", options?.parentKey); + console.warn("can't find record by key", parentKeyValue); return false; } ... }, );属于防御性修正,可按需调整。
♻️ Duplicate comments (2)
tests/table/editor-table.test.tsx (1)
451-455: 移除测试中的调试日志这里的
console.log仅用于调试,会污染测试输出,也可能在 CI 中造成不必要的噪音。建议直接移除这几行,仅保留onchange逻辑。- onChange={(data) => { - console.log( - 'onChange called with data:', - JSON.stringify(data, null, 2), - ); - onchange(data[0]?.children?.[0]?.children?.length ?? 0); - }} + onChange={(data) => { + onchange(data[0]?.children?.[0]?.children?.length ?? 0); + }}src/utils/useEditableArray/index.tsx (1)
173-223: 树结构扁平化/重建整体思路正确,但存在索引碰撞与轻微变异问题这块新增的
flattenRecordsToMap+rebuildTreeStructure思路是对的:
先将整棵树 flatten 到Map中,再按map_row_parentKey重建 children 关系,方便editableRowByKey做 delete / update / top 等操作。有两个点建议调整:
嵌套索引
eachIndex存在碰撞风险
当前使用:const eachIndex = (parentIndex || 0) * 10 + index; const recordKey = getRowKey(record, eachIndex).toString();如果父节点索引较大或子节点超过一位数,
eachIndex会产生重复值(例如父 index 为 1、子 index 为 2 与父 index 为 0、子 index 为 12 均为 12),在 rowKey 依赖 index 的场景会导致recordKey冲突,kvMap中后写覆盖先写,最终树结构错乱。这在多层、宽树形数据下属于潜在阻断缺陷。建议改为基于“路径”的稳定 key,而不是十进制合成的数字,例如:
- parentIndex?: number,
- parentPath: string = '',
...
- const eachIndex = (parentIndex || 0) * 10 + index;
- const recordKey = getRowKey(record, eachIndex).toString();
- const path = parentPath ?
${parentPath}_${index}: index.toString();- const originKey = getRowKey(record, index);
- const normalizedKey = recordKeyToString(originKey);
- const recordKey = (normalizedKey ?? path).toString();
...
const childrenMap = flattenRecordsToMap(children,getRowKey,childrenColumnName,recordKey,eachIndex,);
const childrenMap = flattenRecordsToMap(children,getRowKey,childrenColumnName,recordKey,path,);这样即便外部用 index 作为 rowKey,也能通过 path 提供稳定、唯一标识。
delete修改新对象的小变异问题(已拷贝但仍可更优)目前写法是:
const newRecord = { ...record, map_row_key: recordKey, map_row_parentKey: parentKey, }; delete (newRecord as any).children;虽然是基于
record的浅拷贝再删除children,不会影响原始数据,但可以用解构一次性去除 children,更直观可读:
- const newRecord = {
...record,map_row_key: recordKey,map_row_parentKey: parentKey,- };
- delete (newRecord as any).children;
- const { [childrenColumnName]: _omitChildren, ...recordWithoutChildren } =
record as any;- const newRecord = {
...recordWithoutChildren,map_row_key: recordKey,map_row_parentKey: parentKey,- };
整体算法方向是好的,上述改动主要是提高健壮性和可维护性。 Also applies to: 228-331, 336-374 </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/table/components/EditableTable/index.tsx (1)</summary><blockquote> `513-589`: **editableFormRef 能力和表单同步逻辑设计良好,仅需注意极端 key 场景** 这部分增加了不少内部工具,用于支持 `editableFormRef` 与 name / 非 name 模式下的表单同步: - `createEditingKeysSet` + `syncFormValuesExcludingEditing`:在 `onDataSourceChange` 中同步表单值时跳过正在编辑的行,用 `Map` 减少查找复杂度,对性能与 UX 都是加分项。 - `convertIndexToRowKey` / `convertRowKeyToIndex` / `coverRowKey`:对 index 与 rowKey 的互转封装合理,能支持测试中同时以 index 和 rowKey 调用 `getRowData` / `setRowData` 的需求。 - `buildFormFieldPath` + `getRowData` / `getRowsData` / `setRowData`: - 非 name 模式下按 `rowKey` 扁平存储; - name 模式下按 `['name', index]` 路径存储整行,并在 `setRowData` 中通过 `getRowKey` 回填正确行,兼容受控与非受控。 - `useDeepCompareEffect` 在受控模式下,仅在 `value` 明确传入时同步表单,并分别处理 name / 非 name 模式,避免覆盖 `initialValues`,设计合理。 小建议(无需阻塞合并): - `convertRowKeyToIndex` / `buildFormFieldPath` 在 name 模式下统一把 index 转成字符串(`'0'`),对 rc-field-form 来说通常没问题,但在极端场景下(外部直接使用数字索引路径)可能产生不一致。如果后续遇到此类问题,可以考虑在 NamePath 中保留 number 类型索引,例如 `['table', 0]`。 总体而言,这些改动很好地补齐了 `editableFormRef` 的能力,并解决了之前 name 模式下 index / rowKey 双调用的痛点。 Also applies to: 635-745, 747-795 </blockquote></details> <details> <summary>src/utils/useEditableArray/index.tsx (1)</summary><blockquote> `823-941`: **cancelEditable 新逻辑功能完整,但 key 比较代码有重复可抽取** `cancelEditable` 现在做了不少额外工作: - 通过 `dataSourceKeyIndexMapRef` 支持在 `tableName` 模式下从 index 映射到真实 `recordKey` 再递归取消; - 在不直接通过 UI 的场景下,显式调用 `props.onCancel`,并根据 `newLineRecordCache` 是否匹配当前 key 传入对应的 `newLineConfig`; - 根据 key 匹配情况清理 `newLineRecordCache`,并在最后调用 `clearEditableState` 和重置 `preEditRowRef`。 逻辑上基本正确,不过 key 匹配部分有明显的重复代码,例如: - 判断 `newLineConfig` 是否匹配当前 `recordKey`(行 874-888); - 清理 `newLineRecordCache` 时再次比较(行 911-923)。 建议提炼一个内部辅助函数,比如: ```ts const isSameRecordKey = (a: RecordKey | undefined, b: RecordKey): boolean => { if (a == null) return false; const aStr = recordKeyToString(a)?.toString(); const bStr = recordKeyToString(b)?.toString(); return ( a === b || (aStr != null && bStr != null && aStr === bStr) || String(a) === String(b) ); };然后在两处直接调用
isSameRecordKey(cacheRecordKey, recordKey),既减少重复,又降低今后维护 key 比较逻辑出错的概率。功能角度这块是 ok 的,属于可选重构。
Also applies to: 910-927
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/table/components/EditableTable/index.tsx(9 hunks)src/utils/useEditableArray/index.tsx(11 hunks)tests/table/editor-table-two.test.tsx(1 hunks)tests/table/editor-table.test.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{tsx,jsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Files:
tests/table/editor-table-two.test.tsxsrc/table/components/EditableTable/index.tsxtests/table/editor-table.test.tsxsrc/utils/useEditableArray/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Applied to files:
src/table/components/EditableTable/index.tsx
📚 Learning: 2025-08-01T09:28:05.125Z
Learnt from: CR
Repo: ant-design/pro-components PR: 0
File: .cursor/rules/react.mdc:0-0
Timestamp: 2025-08-01T09:28:05.125Z
Learning: Applies to **/*.{tsx,jsx} : Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Applied to files:
src/table/components/EditableTable/index.tsx
🧬 Code graph analysis (4)
tests/table/editor-table-two.test.tsx (1)
tests/util.ts (1)
waitForWaitTime(11-21)
src/table/components/EditableTable/index.tsx (2)
src/utils/useEditableArray/index.tsx (2)
recordKeyToString(45-48)editableRowByKey(336-374)src/utils/index.tsx (4)
runFunction(114-114)useRefFunction(135-135)useDeepCompareEffect(122-122)isDeepEqualReact(93-93)
tests/table/editor-table.test.tsx (1)
tests/util.ts (1)
waitForWaitTime(11-21)
src/utils/useEditableArray/index.tsx (1)
src/utils/useEditableMap/index.tsx (1)
AddLineOptions(42-45)
⏰ 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). (6)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build (20.x, ubuntu-latest)
- GitHub Check: build
- GitHub Check: build (20.x, ubuntu-latest)
- GitHub Check: build
🔇 Additional comments (5)
tests/table/editor-table.test.tsx (1)
659-661: 新增记录与等待逻辑整体合理
recordCreatorProps.record使用随机 ID 只影响测试内的行 key,不影响断言结果,当前用法可接受。- 为
position="top"场景增加editable={{ type: 'multiple' }}配置有助于覆盖多行编辑路径。- 通过
waitForWaitTime+waitFor检查表格中是否已经渲染出输入框,再读取首行 input 的值,显著提高了用例稳定性和对异步渲染的鲁棒性。这些改动在逻辑上是自洽的,不需要进一步调整。
Also applies to: 839-841, 849-865
src/table/components/EditableTable/index.tsx (1)
819-861: 受控/非受控 onValuesChange 分流与 editableProps 组装合理
handleValuesChange统一封装了表单值变化时的回调顺序:
- 先调用
props.editable?.onValuesChange(record, dataSource);- 再调用
props.onValuesChange?.(dataSource, record);- 若开启
controlled且传入onChange,则在此同步触发onChange(dataSource),便于外部维护受控value。editableProps使用useMemo只在相关依赖变动时重建,且仅在存在外部监听(onValuesChange/editable.onValuesChange/controlled && onChange)时才包一层handleValuesChange,避免了多余包装。这套设计清晰地区分了受控与非受控模式下
onChange的触发职责,与onDataSourceChange中的非受控逻辑也没有冲突,看起来是合理的演进方向。Also applies to: 885-913, 915-951
src/utils/useEditableArray/index.tsx (2)
646-681: 数据源 key 映射与编辑状态控制逻辑较完善这一段主要是围绕
dataSourceKeyIndexMapRef和编辑状态做增强,整体设计是合理的:
buildDataSourceKeyIndexMap
- 对每个节点建立“双向”映射:
indexKey(如 '0_1') => recordKey以及recordKey => indexKey,并递归处理children,便于在 index 与真正的 rowKey 之间互相转换。- 在
useDeepCompareEffectDebounce中按 dataSource 变动重建映射,降低 map 与数据不同步的风险。
isEditable+checkKeyInEditableList
- 同时考虑 “用 index 取 key” 与 “用真实 key” 的两种情况,并在当前与上一轮 editableKeys 基础上计算
isEditable/preIsEditable,为过渡动画或 Diff 判断准备数据,这是很实用的抽象。
validateCanStartEdit+findRecordByKey
- 在
startEditable前阻止多行编辑(type: 'single')并给出国际化文案的 warning;- 查找记录集中使用
getRowKey(record, index),而不是硬编码主键字段,跟外部 rowKey 配置保持一致。这一组辅助函数让编辑状态管理更集中、可读性更好,不建议再拆。
Also applies to: 683-721, 731-782, 787-821
948-1032: onValuesChange 重写为基于 Map 的数据推导,逻辑清晰
buildFormFieldPath统一了根据tableName+recordKey构造 NamePath 的方式,使get/set调用更集中。updateDataSourceWithEditableRows:
- 遍历当前
editableKeys,对每个 key 通过editableRowByKey将编辑行回写到临时updatedDataSource;- 明确跳过正在编辑的新行(由
newLineRecordCache.options.recordKey标识),避免在缓存阶段就把它写回正式 dataSource。getCurrentEditRow:
- 根据
changedValues提取当前变化的 key;- 结合
newLineRecordCache.defaultValue与 form 中的值构造出“当前编辑行”的最终对象;- 利用
dataSourceKeyIndexMapRef判断该 key 是否存在于 dataSource 中,优先返回真实 dataSource 中的行,否则回退到新行数据。最后通过
onValuesChange将(editRow, updatedDataSource)传给外部的props.onValuesChange(带 debounce),相比之前只给一部分信息更加完整。整体设计合理,可读性也不错,这块可以保持现状。
Also applies to: 1034-1047
tests/table/editor-table-two.test.tsx (1)
677-698: 基于getRowsData的断言明显提升了用例稳定性这里把原先直接通过
getFieldValue('table')读取长度的断言,改成:
- 先用
waitFor等待formRef.current?.getRowsData?.()返回长度为 2;- 再在短暂
waitForWaitTime后,用getFieldValue('table')和getRowsData交叉验证最终值是一个长度为 2 的数组。这种做法有几个好处:
- 首选
getRowsData这一高层 API,避免直接依赖表单内部结构;- 通过
waitFor包裹getRowsData,对异步编辑/新增行场景更鲁棒,不容易因为刷新顺序导致偶发失败;getFieldValue只作为辅助验证,不再成为易受时序影响的单一信号源。整体改动方向很好,可以保持现状。
| /** | ||
| * 处理嵌套行的新增 | ||
| */ | ||
| function handleNestedRowInsert<DataType>( | ||
| baseData: DataType[], | ||
| defaultValue: DataType, | ||
| newLineOptions: { | ||
| parentKey?: React.Key; | ||
| recordKey?: React.Key; | ||
| position?: 'top' | 'bottom' | string; | ||
| }, | ||
| getRowKey: GetRowKey<any>, | ||
| childrenColumnName: string, | ||
| ): DataType[] { | ||
| if (!newLineOptions.recordKey) { | ||
| return baseData; | ||
| } | ||
|
|
||
| const actionProps = { | ||
| data: baseData, | ||
| getRowKey, | ||
| row: { | ||
| ...defaultValue, | ||
| map_row_parentKey: recordKeyToString( | ||
| newLineOptions.parentKey!, | ||
| )?.toString(), | ||
| }, | ||
| key: newLineOptions.recordKey, | ||
| childrenColumnName, | ||
| }; | ||
|
|
||
| return editableRowByKey( | ||
| actionProps, | ||
| newLineOptions.position === 'top' ? 'top' : 'update', | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * 处理分页场景下的新增 | ||
| */ | ||
| function handlePaginationInsert<DataType>( | ||
| baseData: DataType[], | ||
| defaultValue: DataType, | ||
| pageConfig: TablePaginationConfig, | ||
| ): DataType[] { | ||
| if (pageConfig.pageSize! > baseData.length) { | ||
| return [...baseData, defaultValue]; | ||
| } | ||
| const insertIndex = pageConfig.current! * pageConfig.pageSize! - 1; | ||
| const result = [...baseData]; | ||
| result.splice(insertIndex, 0, defaultValue); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
新增行数据源处理逻辑整体正确,但分页插入存在边界风险
handleNestedRowInsert通过editableRowByKey+map_row_parentKey来处理嵌套新增,和recordKeyToString配合,可以兼容字符串与数字 key,整体设计合理。useEditableDataSource对新增行优先级为:parentKey嵌套插入 →position === 'top'头部插入 → 当前分页插入 → 默认追加,逻辑清晰。
需要留意两点小风险:
-
分页插入下标越界(
handlePaginationInsert)
当pageConfig.current * pageConfig.pageSize - 1大于baseData.length时,splice会在数组尾部之后插入,产生稀疏数组。这在页码被外部错误配置或数据长度变化后未同步时容易出现。建议在计算插入位置时做一次截断:
- const insertIndex = pageConfig.current! * pageConfig.pageSize! - 1;
- const rawIndex = pageConfig.current! * pageConfig.pageSize! - 1;
- const insertIndex = Math.min(rawIndex, baseData.length);
-
parentKey判空使用 truthy 判断
当前if (newLineOptions?.parentKey)决定是否走嵌套插入分支,这会把合法的0或''视为“无 parentKey”。虽然现有用例都用非 0 的 id,建议改成显式判null/undefined,避免未来踩坑:
- if (newLineOptions?.parentKey) {
- if (newLineOptions && newLineOptions.parentKey !== undefined && newLineOptions.parentKey !== null) {
整体上这块逻辑结构良好,上述修改属于防御性增强。
Also applies to: 209-272
<!-- fingerprinting:phantom:medusa:olive -->
<!-- This is an auto-generated comment by CodeRabbit -->
| /** | ||
| * 检查是否应该显示创建按钮 | ||
| */ | ||
| function shouldShowCreatorButton( | ||
| maxLength: number | undefined, | ||
| valueLength: number, | ||
| recordCreatorProps: EditableProTableProps<any, any>['recordCreatorProps'], | ||
| ): boolean { | ||
| if (typeof maxLength === 'number' && maxLength <= valueLength) { | ||
| return false; | ||
| } | ||
| return recordCreatorProps !== false; | ||
| } | ||
|
|
||
| /** | ||
| * 创建按钮 DOM | ||
| */ | ||
| function createButtonDom<DataType>( | ||
| recordCreatorProps: Exclude< | ||
| EditableProTableProps<DataType, any>['recordCreatorProps'], | ||
| false | undefined | ||
| >, | ||
| value: readonly DataType[] | undefined, | ||
| intl: ReturnType<typeof useIntl>, | ||
| ): React.ReactNode { | ||
| const { | ||
| record, | ||
| position, | ||
| creatorButtonText, | ||
| newRecordType, | ||
| parentKey, | ||
| style, | ||
| ...restButtonProps | ||
| } = recordCreatorProps; | ||
|
|
||
| return ( | ||
| <RecordCreator | ||
| record={runFunction(record, value?.length, value) || {}} | ||
| position={position} | ||
| parentKey={runFunction(parentKey, value?.length, value)} | ||
| newRecordType={newRecordType} | ||
| > | ||
| <Button | ||
| type="dashed" | ||
| style={{ | ||
| display: 'block', | ||
| margin: '10px 0', | ||
| width: '100%', | ||
| ...style, | ||
| }} | ||
| icon={<PlusOutlined />} | ||
| {...restButtonProps} | ||
| > | ||
| {creatorButtonText || | ||
| intl.getMessage('editableTable.action.add', '添加一行数据')} | ||
| </Button> | ||
| </RecordCreator> | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * 创建顶部按钮的渲染属性 | ||
| */ | ||
| function createTopButtonProps( | ||
| creatorButtonDom: React.ReactNode, | ||
| columnsLength: number | undefined, | ||
| ) { | ||
| return { | ||
| components: { | ||
| header: { | ||
| wrapper: ({ | ||
| className, | ||
| children, | ||
| }: { | ||
| className: string; | ||
| children: React.ReactNode; | ||
| }) => ( | ||
| <thead className={className}> | ||
| {children} | ||
| <tr style={{ position: 'relative' }}> | ||
| <td colSpan={0} style={{ visibility: 'hidden' }}> | ||
| {creatorButtonDom} | ||
| </td> | ||
| <td | ||
| style={{ position: 'absolute', left: 0, width: '100%' }} | ||
| colSpan={columnsLength} | ||
| > | ||
| {creatorButtonDom} | ||
| </td> | ||
| </tr> | ||
| </thead> | ||
| ), | ||
| }, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * 创建底部按钮的渲染属性 | ||
| */ | ||
| function createBottomButtonProps( | ||
| creatorButtonDom: React.ReactNode, | ||
| tableViewRender: ProTableProps<any, any>['tableViewRender'], | ||
| ) { | ||
| return { | ||
| tableViewRender: (_: any, dom: any) => ( | ||
| <> | ||
| {tableViewRender?.(_, dom) ?? dom} | ||
| {creatorButtonDom} | ||
| </> | ||
| ), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
新增按钮辅助函数结构清晰,useCreatorButton 中 ESLint 禁用可移除
-
CreatorButtonResult类型 +shouldShowCreatorButton/createButtonDom/createTopButtonProps/createBottomButtonProps把创建按钮的条件、DOM、头部/底部渲染拆得很干净,提高了可读性与可测试性。 -
useCreatorButton现在只负责:- 根据
maxLength/recordCreatorProps/value.length判断是否展示按钮; - 基于
isTop返回不同的buttonRenderProps。
- 根据
-
useMemo的依赖数组已经补全了intl:}, [maxLength, recordCreatorProps, value?.length, intl]);
因此这里的 // eslint-disable-next-line react-hooks/exhaustive-deps 已经没有必要,保留只会掩盖未来依赖遗漏的问题,建议删除这条注释,让 ESLint 正常帮你兜底。
- return createButtonDom(recordCreatorProps, value, intl);
- // eslint-disable-next-line react-hooks/exhaustive-deps
- }, [maxLength, recordCreatorProps, value?.length, intl]);
+ return createButtonDom(recordCreatorProps, value, intl);
+ }, [maxLength, recordCreatorProps, value?.length, intl]);整体按钮相关逻辑可以保持当前结构不变。
Also applies to: 388-438, 409-425, 427-435
🤖 Prompt for AI Agents
In src/table/components/EditableTable/index.tsx around lines 274-386 (also
applies to 388-438, 409-425, 427-435), the eslint-disable-next-line
react-hooks/exhaustive-deps comments in useCreatorButton are no longer needed
because the useMemo dependency arrays correctly include intl (and other
dependencies). Remove those eslint-disable-next-line comments, ensure each
hook's dependency array is complete (including intl, maxLength,
recordCreatorProps, value?.length, isTop as applicable), and run ESLint to
confirm no exhaustive-deps warnings remain.
| const saveRefsMap = useRef< | ||
| Map<React.Key, React.RefObject<SaveEditableActionRef>> | ||
| >(new Map<React.Key, React.RefObject<SaveEditableActionRef>>()); | ||
|
|
||
| useEffect(() => { | ||
| // 确保只保留编辑状态的,其它的都删除掉 | ||
| const editableKeysSet = new Set( | ||
| editableKeys?.map((key) => key?.toString()) ?? [], | ||
| ); | ||
| saveRefsMap.current.forEach((ref, key) => { | ||
| if (!editableKeysSet.has(key)) { | ||
| if (!editableKeysSet.has(key?.toString())) { | ||
| saveRefsMap.current.delete(key); | ||
| } | ||
| }); | ||
| }, [saveRefsMap, editableKeysSet]); | ||
| }, [editableKeys]); | ||
|
|
||
| /** | ||
| * 获取保存引用 | ||
| */ | ||
| const getSaveRef = useRefFunction((recordKey: RecordKey) => { | ||
| const relayKey = recordKeyToString(recordKey); | ||
| return ( | ||
| saveRefsMap.current.get(relayKey) || | ||
| saveRefsMap.current.get(relayKey.toString()) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
saveEditable 在 name 模式下无法找到保存引用,是阻断性缺陷
当前 saveEditable / getSaveRef / actionRender 之间对 key 的处理在存在 tableName(即 name 模式)时存在不一致,导致通过 ActionType.saveEditable 保存失败:
-
在
actionRender中,当props.tableName存在时,会将saveRefsMap的 key 设置为 索引 key:if (props.tableName) { saveRefsMap.current.set( dataSourceKeyIndexMapRef.current.get(recordKeyToString(key)) || recordKeyToString(key), renderResult.saveRef, ); }
这里
dataSourceKeyIndexMapRef将recordKey.toString()映射为如'0'的 indexKey,因此 saveRef 实际存成'0'而不是'624748504'之类的真实 key。 -
getSaveRef只用传入的recordKey自己去查saveRefsMap:const relayKey = recordKeyToString(recordKey); return ( saveRefsMap.current.get(relayKey) || saveRefsMap.current.get(relayKey.toString()) );
在 name 模式下:
- 对于
saveEditable(真实 recordKey):relayKey是'624748504',但saveRefsMap里只有'0',查不到; - 对于
saveEditable(0):relayKey是'0',isInEditableSet为 false,进入递归saveEditable(mappedKey, false),mappedKey变成'624748504',再次调用时又回到上一种情况,仍然查不到 saveRef。
- 对于
结果是:name 模式下无论用 index 还是真实 key 调用 saveEditable,saveRef 都是 undefined,函数直接返回 false,保存行为失效(包括测试中 saveEditable(624748504) 和 saveEditable(0) 的场景)。
建议修复方案(保留现有映射的基础上最小修改)
在 getSaveRef 中同时尝试“原始 key”与“通过映射得到的 key”,例如:
- const getSaveRef = useRefFunction((recordKey: RecordKey) => {
- const relayKey = recordKeyToString(recordKey);
- return (
- saveRefsMap.current.get(relayKey) ||
- saveRefsMap.current.get(relayKey.toString())
- );
- });
+ const getSaveRef = useRefFunction((recordKey: RecordKey) => {
+ const relayKey = recordKeyToString(recordKey);
+ const relayKeyStr = relayKey != null ? relayKey.toString() : '';
+
+ // 先用原始 key 查一次
+ const directRef =
+ saveRefsMap.current.get(relayKey) ||
+ (relayKeyStr ? saveRefsMap.current.get(relayKeyStr) : undefined);
+ if (directRef) return directRef;
+
+ // 再尝试通过 dataSourceKeyIndexMapRef 映射后的 key(indexKey 或 recordKey)
+ const mappedKey =
+ relayKeyStr && dataSourceKeyIndexMapRef.current.get(relayKeyStr);
+ if (mappedKey) {
+ const mappedKeyStr = mappedKey.toString();
+ return (
+ saveRefsMap.current.get(mappedKey) ||
+ saveRefsMap.current.get(mappedKeyStr)
+ );
+ }
+
+ return undefined;
+ });这样:
- 非 name 模式保持原有行为(只存真实 recordKey);
- name 模式下:
saveEditable(真实 recordKey)会通过mappedKey找到 indexKey 对应的 saveRef;saveEditable(0)会在第一次调用中直接命中saveRefsMap.get('0'),无需递归或在递归中同样能找到。
修复后可再次运行相关用例(尤其是 EditableProTable saveEditable should save and quit editing 中对 saveEditable(624748504) 和 saveEditable(0) 的断言),确认受 name 模式影响的保存逻辑都能工作。
Also applies to: 1079-1111, 1132-1145, 1150-1158, 1163-1222, 1245-1275
🤖 Prompt for AI Agents
In src/utils/useEditableArray/index.tsx around lines 1050-1074 (also apply same
fix to the other affected ranges noted: 1079-1111, 1132-1145, 1150-1158,
1163-1222, 1245-1275): getSaveRef currently only looks up saveRefsMap by the
recordKey string, which fails in name/tableName mode because actionRender stored
refs under the indexKey (from dataSourceKeyIndexMapRef). Update getSaveRef to
try, in order, the relayKey, the mapped index key from
dataSourceKeyIndexMapRef.current.get(relayKey), and the toString variants (e.g.,
relayKey.toString() and mappedKey?.toString()) when calling saveRefsMap.get;
return the first non-undefined ref so both name mode and non-name mode resolve
correctly.
…and useEditableArray components. Improve validation error propagation during save and cancel operations, ensuring proper feedback in the form. Update tests to validate new delete functionality and confirm behavior when onDelete returns false, preventing unintended deletions.
| { timeout: 2000 }, | ||
| ); | ||
|
|
||
| const initialRowCount = defaultData.length; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
To fix the unused variable issue, simply remove the line const initialRowCount = defaultData.length; from the test. This eliminates unnecessary computation and cleans up the code, improving readability and maintainability. No further changes are required since no other code depends on this variable.
| @@ -1665,7 +1665,6 @@ | ||
| { timeout: 2000 }, | ||
| ); | ||
|
|
||
| const initialRowCount = defaultData.length; | ||
|
|
||
| // 查找删除按钮 | ||
| const allElements = wrapper.container.querySelectorAll('a, button'); |
| ); | ||
| await waitForWaitTime(100); | ||
|
|
||
| const originalTitle = defaultData[0]?.title; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 days ago
To fix the detected unused variable, simply delete the line that defines originalTitle (line 1884). This can be done safely because its value is never used beyond its assignment. There is no need for additional imports, method or definition changes in this case. Only one line in the test case needs to be removed.
-
Copy modified line R1884
| @@ -1881,7 +1881,7 @@ | ||
| ); | ||
| await waitForWaitTime(100); | ||
|
|
||
| const originalTitle = defaultData[0]?.title; | ||
| // Removed unused variable `originalTitle` | ||
|
|
||
| // 修改数据 | ||
| act(() => { |
…ce management, creator button rendering, and table content styling. Enhance pagination and search capabilities, and add utility functions for row key handling and toolbar rendering.
Summary by CodeRabbit
发版说明
✏️ Tip: You can customize this high-level summary in your review settings.