fix: change not trigger on Date #734
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此拉取请求对 Changes
Assessment against linked issues
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #734 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 18 18
Lines 1256 1256
Branches 314 310 -4
=======================================
Hits 1249 1249
Misses 7 7 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
tests/field.test.tsx (2)
4-7: 需要清理未使用的导入代码中导入了
Input和getInput，但在当前测试文件中并未使用。如果这些导入确实不需要，建议将其删除以保持代码整洁。建议应用以下修改：
import { act, fireEvent, render } from '@testing-library/react'; -import { Input } from './common/InfoField'; -import { getInput } from './common'; import timeout from './common/timeout';🧰 Tools
🪛 eslint
[error] 5-5: 'Input' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 6-6: 'getInput' is defined but never used.
(@typescript-eslint/no-unused-vars)
44-75: 测试用例设计合理，建议增加更多边界场景测试测试用例很好地验证了日期变更的核心功能，符合 #51611 issue 的修复目标。代码结构清晰，使用了正确的异步测试模式。
建议考虑添加以下测试场景：
- 验证相同日期值的处理
- 测试日期对象为 null 的情况
- 添加错误处理测试用例
建议添加如下测试用例：
// 测试相同日期值 it('should not trigger change for same date value', async () => { const onValuesChange = jest.fn(); const fixedDate = new Date('2024-01-01'); const MockDateInput = (props: { onChange?: (val: Date) => void }) => ( <button onClick={() => props.onChange?.(fixedDate)}> Mock </button> ); const { container } = render( <Form onValuesChange={onValuesChange}> <Field name="date"> <MockDateInput /> </Field> </Form>, ); // 连续触发两次相同的日期 fireEvent.click(container.querySelector('button')); await act(async () => { await timeout(); }); fireEvent.click(container.querySelector('button')); await act(async () => { await timeout(); }); expect(onValuesChange).toHaveBeenCalledTimes(1); });tests/legacy/field-props.test.tsx (1)
77-89: 建议改进normalize回调的类型安全性当前的normalize回调函数使用了可选链操作符，建议通过TypeScript类型定义来增强类型安全性。
建议按照以下方式改进：
- normalize={value => value?.replace(/\D/g, '') || undefined} + normalize={(value: string | undefined): string | undefined => + value ? value.replace(/\D/g, '') : undefined + }src/Field.tsx (1)
Line range hint
623-629: 建议添加相关测试用例为确保 Date 对象的处理逻辑正确，建议添加专门的测试用例。
建议添加以下测试场景：
- Date 对象值变更的情况
- Date 对象值不变的情况
- 其他复杂对象类型的处理
// 建议添加的测试用例 it('should handle Date object value changes correctly', () => { const onChange = jest.fn(); const wrapper = mount( <Form> <Form.Item name="date"> <DatePicker onChange={onChange} /> </Form.Item> </Form> ); // 测试日期变更 const newDate = new Date(); wrapper.find('DatePicker').simulate('change', newDate); expect(onChange).toHaveBeenCalled(); // 测试日期不变 wrapper.find('DatePicker').simulate('change', newDate); expect(onChange).toHaveBeenCalledTimes(1); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/Field.tsx(1 hunks)tests/control.test.tsx(0 hunks)tests/field.test.tsx(2 hunks)tests/legacy/field-props.test.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- tests/control.test.tsx
🧰 Additional context used
🪛 eslint
tests/field.test.tsx
[error] 5-5: 'Input' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 6-6: 'getInput' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome
tests/legacy/field-props.test.tsx
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
tests/legacy/field-props.test.tsx (1)
4-4: 导入语句修改正确！
导入语句的更改符合新增测试用例的需求，同时保持了代码的清晰度。
src/Field.tsx (1)
Line range hint 623-629: 值比较逻辑的改变需要仔细验证
将 isEqual 深度比较改为 !== 严格不等比较确实可以解决 Date 对象的问题，但这个改动可能会影响其他复杂对象类型的处理。
建议：
- 添加对 Date 对象的专门处理逻辑
- 或者在比较之前对 Date 对象进行标准化处理
-if (newValue !== value) {
+// 为 Date 对象添加特殊处理
+const isDateValue = newValue instanceof Date && value instanceof Date;
+if (isDateValue ? newValue.getTime() !== value.getTime() : !isEqual(newValue, value)) {
dispatch({
type: 'updateValue',
namePath,
value: newValue,
});
}| describe('normalize', () => { | ||
| it('basic', async () => { | ||
| const form = React.createRef<FormInstance>(); | ||
| const { container } = render( | ||
| <div> | ||
| <Form ref={form}> | ||
| <Field name="normal" normalize={v => v && v.toUpperCase()}> | ||
| <Input /> | ||
| </Field> | ||
| </Form> | ||
| </div>, | ||
| ); | ||
|
|
||
| await changeValue(getInput(container), 'a'); | ||
| await changeValue(getInput(container), 'a'); | ||
|
|
||
| expect(form.current?.getFieldValue('normal')).toBe('A'); | ||
| expect(getInput(container).value).toBe('A'); | ||
| }); |
There was a problem hiding this comment.
💡 Codebase verification
建议补充Date对象规范化（normalize）的测试用例
经检查，项目中已有基本的Date对象变更检测测试（在field.test.tsx中），但缺少针对Date对象规范化的测试场景。建议添加以下测试场景：
- Date对象的格式转换
- 无效日期的处理
- 日期范围的规范化
🔗 Analysis chain
测试用例结构清晰，建议补充Date对象相关测试
测试用例很好地验证了基本的值规范化功能。考虑到此PR主要解决Date对象的变更检测问题，建议添加专门针对Date对象的测试用例。
运行以下脚本检查是否存在其他Date相关的测试：
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索项目中与Date对象相关的测试
rg -t tsx -t ts "new Date\(" "test"
Length of output: 63
Script:
#!/bin/bash
# 搜索项目中与Date对象相关的测试，使用正确的文件类型参数
rg "new Date\(" tests/
Length of output: 85
Script:
#!/bin/bash
# 搜索测试文件中可能包含日期相关的测试用例
rg -i "date|moment" tests/
Length of output: 13344
Script:
#!/bin/bash
# 查找测试目录下所有测试文件
fd -e ts -e tsx -e js -e jsx . tests/
Length of output: 816
Script:
#!/bin/bash
# 查看field.test.tsx中的Date相关测试用例的具体实现
rg -A 10 "date type as change" tests/field.test.tsx
Length of output: 336
🧰 Tools
🪛 Biome
[error] 64-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| ); | ||
|
|
||
| await changeValue(getInput(container), 'bamboo'); | ||
| expect(fn).toHaveBeenCalledTimes(0); |
There was a problem hiding this comment.
为啥把，执行 onChange，不调用 onValueChange 的 test 给删了？
Date 对象的数据在闭包里，equals 无法检测降级一下：
fix ant-design/ant-design#51611
Summary by CodeRabbit
新功能
Field组件的值比较逻辑，简化了值更新的触发条件。错误修复
文档