fix: When value is null, the dateString in onchange is null #942
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. Walkthrough在测试用例 tests/multiple.spec.tsx 中引入并调用了从 ./util/commonUtil 新增导出的 clearValue()，在首次 onChange 断言后执行清空，再断言 onChange 收到 (null, null)。同时在 ./util/commonUtil 模块中新增加并导出了 clearValue() 方法。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Tester as 测试
participant Spec as multiple.spec.tsx
participant Comp as 组件/逻辑
participant Util as commonUtil.clearValue
participant CB as onChange 回调
Tester->>Spec: 运行测试
Spec->>Comp: 触发更改
Comp-->>CB: onChange(value, meta)
Note over Spec,CB: 断言首次 onChange
Spec->>Util: clearValue()
Util-->>Comp: 重置内部值
Comp-->>CB: onChange(null, null)
Note over Spec,CB: 断言清空后的 onChange
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 @EmilyyyLiu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| const everyEmpty = clone.every((val) => !val); | ||
| onChange( | ||
| // Return null directly if all date are empty | ||
| isNullValue && clone.every((val) => !val) ? null : clone, | ||
| getDateTexts(clone), | ||
| isNullValue && everyEmpty ? null : clone, | ||
| everyEmpty ? null : getDateTexts(clone), | ||
| ); |
There was a problem hiding this comment.
你好，这个改动将 dateString 在清空时设置为 null，这与 onChange 回调函数在 PickerProps 和 RangePickerProps 中的类型定义不一致，可能会导致 TypeScript 编译错误。
例如，在 RangePickerProps 中，dateStrings 的类型是 [string, string]，并不接受 null。
// src/PickerInput/RangePicker.tsx
export interface BaseRangePickerProps<DateType extends object> {
// ...
onChange?: (
dates: NoUndefinedRangeValueType<DateType> | null,
dateStrings: [string, string], // <--- 这里应该是 [string, string] | null
) => void;
// ...
}在 PickerProps (用于 SinglePicker) 中，dateString 的类型是 string | string[]，同样不接受 null。
// src/PickerInput/SinglePicker.tsx
export interface BasePickerProps<DateType extends object = any> {
// ...
onChange?: (date: DateType | DateType[], dateString: string | string[]) => void; // <--- 这里应该是 string | string[] | null
// ...
}虽然测试用例已经更新以适应 null 值，但 TypeScript 的类型定义也需要同步更新，以避免给库的使用者带来类型错误。
建议在 src/PickerInput/RangePicker.tsx 和 src/PickerInput/SinglePicker.tsx 中更新 onChange 的类型定义，以允许 dateString / dateStrings 为 null。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #942 +/- ##
=======================================
Coverage 98.80% 98.80%
=======================================
Files 65 65
Lines 2680 2682 +2
Branches 719 721 +2
=======================================
+ Hits 2648 2650 +2
Misses 29 29
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/PickerInput/hooks/useRangeValue.ts(1 hunks)tests/picker.spec.tsx(1 hunks)tests/range.spec.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test / react component workflow
🔇 Additional comments (2)
tests/range.spec.tsx (1)
205-206: 用例更新与新语义一致，LGTM清空后期望变更为
onChange(null, null)与改动一致。tests/picker.spec.tsx (1)
286-287: 单选清空返回(null, null)的断言合理与范围的处理保持一致性，改动可取。请同时确认单选
onChange的第二参数类型也已允许null，避免类型不匹配。如需核对类型定义，可复用我在另一个评论里提供的脚本来检索
PickerProps的onChange签名。
| const everyEmpty = clone.every((val) => !val); | ||
| onChange( | ||
| // Return null directly if all date are empty | ||
| isNullValue && clone.every((val) => !val) ? null : clone, | ||
| getDateTexts(clone), | ||
| isNullValue && everyEmpty ? null : clone, | ||
| everyEmpty ? null : getDateTexts(clone), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
仅在 clear（value 为 null）时将 dateStrings 置为 null，避免扩大行为面；并显式比较 null
当前实现对第二参采用 everyEmpty ? null : getDateTexts(clone)，即便不是 clear 操作、但两端都为 null 的场景也会返回 dateStrings = null。这超出了本 PR 目标“当 value 为 null 时，dateString 为 null”，且与第一参的判定不一致（第一参受 isNullValue && everyEmpty 约束）。
建议：
- 将第二参同样受
isNullValue && everyEmpty约束，保证只有 clear 时才传null。 everyEmpty判定使用val == null可读性更强、语义更精确。
可按如下修改：
- const everyEmpty = clone.every((val) => !val);
+ const everyEmpty = clone.every((val) => val == null);
onChange(
// Return null directly if all date are empty
- isNullValue && everyEmpty ? null : clone,
- everyEmpty ? null : getDateTexts(clone),
+ isNullValue && everyEmpty ? null : clone,
+ isNullValue && everyEmpty ? null : getDateTexts(clone),
);[uggest_recommended_refactor]
为确保类型一致性，请确认 onChange 第二参数类型已允许 null（单选与范围）。可用以下脚本检索类型定义并核对是否包含 | null：
🏁 Script executed:
#!/bin/bash
# 查找 RangePicker 与 Picker 的 onChange 类型定义
rg -nP -C3 '\binterface\s+RangePickerProps\b[\s\S]*?\bonChange\?\s*:\s*\(' --type ts --type tsx
rg -nP -C3 '\binterface\s+PickerProps\b[\s\S]*?\bonChange\?\s*:\s*\(' --type ts --type tsx
# 查找 ReplaceListType 定义
rg -nP -C2 'type\s+ReplaceListType' --type tsLength of output: 336
🏁 Script executed:
#!/bin/bash
# 查找 RangePickerProps 与 PickerProps 类型定义
rg -nP 'interface\s+RangePickerProps\b' -n -g '*.ts' -g '*.tsx'
rg -nP 'interface\s+PickerProps\b' -n -g '*.ts' -g '*.tsx'
# 查找所有 onChange? 签名，确认第二参数是否允许 null
rg -nP 'onChange\?\s*:' -n -g '*.ts' -g '*.tsx'Length of output: 779
🏁 Script executed:
#!/bin/bash
# 查看 RangePickerProps 接口及 onChange 签名
rg -nP -C5 'interface\s+RangePickerProps' -n src/PickerInput/RangePicker.tsx
rg -nP -C5 'onChange\?:' -n src/PickerInput/RangePicker.tsx
# 查看 PickerProps 接口及 onChange 签名
rg -nP -C5 'interface\s+PickerProps' -n src/PickerInput/SinglePicker.tsx
rg -nP -C5 'onChange\?:' -n src/PickerInput/SinglePicker.tsxLength of output: 2090
修复类型定义并仅在 clear 时将 dateStrings 设为 null
- src/PickerInput/hooks/useRangeValue.ts: 将
everyEmpty判断改为val == null，并将onChange第二参改为isNullValue && everyEmpty ? null : getDateTexts(clone) - src/PickerInput/RangePicker.tsx (69–72)：在
RangePickerProps中将onChange签名的dateStrings: [string, string]修改为dateStrings: [string, string] | null - src/PickerInput/SinglePicker.tsx (53,108)：在
PickerProps和ReplacedPickerProps中将dateString: string | string[]修改为string | string[] | null
🤖 Prompt for AI Agents
In src/PickerInput/hooks/useRangeValue.ts around lines 286–291, change the
empty-check to treat null/undefined as empty by using val == null instead of
!val, and change the onChange second argument to only be null when the overall
value is null AND every item is null: isNullValue && everyEmpty ? null :
getDateTexts(clone). Also update types: in src/PickerInput/RangePicker.tsx
(lines ~69–72) change RangePickerProps onChange signature so dateStrings is
typed as [string, string] | null, and in src/PickerInput/SinglePicker.tsx (lines
~53 and ~108) change PickerProps and ReplacedPickerProps dateString types from
string | string[] to string | string[] | null so callers can receive null on
clear.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/multiple.spec.tsx (2)
5-5: 引入 clearValue 合理，但建议在工具内断言元素存在当前
clearValue直接对document.querySelector('.rc-picker-clear')结果进行事件触发，若未渲染清除按钮将导致 NPE。建议在工具方法内断言元素存在，或在用例内先断言再调用。可在
tests/util/commonUtil.tsx中轻量保护：export function clearValue() { - const clearBtn = document.querySelector('.rc-picker-clear'); + const clearBtn = document.querySelector('.rc-picker-clear'); + if (!clearBtn) { + throw new Error('clear button not found: .rc-picker-clear'); + } fireEvent.mouseDown(clearBtn); fireEvent.mouseUp(clearBtn); fireEvent.click(clearBtn); }
60-62: 断言建议使用“最后一次调用”以增强稳定性
toHaveBeenCalledWith(null, null)只保证出现过该调用。这里更贴合语义地使用toHaveBeenLastCalledWith(null, null)。- expect(onChange).toHaveBeenCalledWith(null, null); + expect(onChange).toHaveBeenLastCalledWith(null, null);另外可顺手断言调用次数，便于回归定位：
expect(onChange).toHaveBeenCalledTimes(2);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/PickerInput/SinglePicker.tsx(1 hunks)tests/multiple.spec.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/multiple.spec.tsx (1)
tests/util/commonUtil.tsx (1)
clearValue(156-161)
EmilyyyLiu
force-pushed
the
clear-nullValue
branch
from
b56b494 to
190abe6
Compare
关联issue：ant-design/ant-design#54867
当清空数据，onChange 内 dateString 设置为null
Summary by CodeRabbit