Fix disabled existing values blocking picker changes#975
Conversation
|
@QDyanbing is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough该 PR 修正了日期选择器在既有选中值被 ChangesDisabled Date Validation Refinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/PickerInput/SinglePicker.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/PickerInput/hooks/useRangeValue.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. tests/multiple.spec.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the date validation logic in SinglePicker and useRangeValue to ensure that existing selected dates which become disabled by an updated disabledDate configuration do not block subsequent changes. It introduces helper functions like isPopupInvalidateDate and includesTimestamp to check if a date is already part of the selected values, and adds comprehensive test cases for both multiple and range pickers. The reviewer suggested adding defensive Array.isArray checks to these helpers to prevent potential runtime errors if the array values are uninitialized.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function includesTimestamp<DateType extends object>( | ||
| generateConfig: GenerateConfig<DateType>, | ||
| dates: DateType[], | ||
| date: DateType, | ||
| ) { | ||
| return dates.some((prevDate) => isSameTimestamp(generateConfig, prevDate, date)); | ||
| } |
There was a problem hiding this comment.
为了提高代码的健壮性并防止潜在的运行时错误（例如 dates 为 null、undefined 或非数组类型时调用 some 导致崩溃），建议在 includesTimestamp 中增加对 dates 的数组类型校验。这符合防御性编程的最佳实践。
| function includesTimestamp<DateType extends object>( | |
| generateConfig: GenerateConfig<DateType>, | |
| dates: DateType[], | |
| date: DateType, | |
| ) { | |
| return dates.some((prevDate) => isSameTimestamp(generateConfig, prevDate, date)); | |
| } | |
| function includesTimestamp<DateType extends object>( | |
| generateConfig: GenerateConfig<DateType>, | |
| dates: DateType[], | |
| date: DateType, | |
| ) { | |
| return Array.isArray(dates) && dates.some((prevDate) => isSameTimestamp(generateConfig, prevDate, date)); | |
| } |
| const isPopupInvalidateDate = useEvent((date: DateType) => { | ||
| if ( | ||
| multiple && | ||
| mergedValue.some((valueDate) => isSameTimestamp(generateConfig, valueDate, date)) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| return isInvalidateDate(date, { activeIndex: 0 }); | ||
| }); |
There was a problem hiding this comment.
为了提高代码的健壮性，建议在 isPopupInvalidateDate 中对 mergedValue 进行数组类型校验。如果 mergedValue 在某些边界情况下未被正确初始化或非数组，直接调用 some 可能会导致运行时崩溃。
| const isPopupInvalidateDate = useEvent((date: DateType) => { | |
| if ( | |
| multiple && | |
| mergedValue.some((valueDate) => isSameTimestamp(generateConfig, valueDate, date)) | |
| ) { | |
| return false; | |
| } | |
| return isInvalidateDate(date, { activeIndex: 0 }); | |
| }); | |
| const isPopupInvalidateDate = useEvent((date: DateType) => { | |
| if ( | |
| multiple && | |
| Array.isArray(mergedValue) && | |
| mergedValue.some((valueDate) => isSameTimestamp(generateConfig, valueDate, date)) | |
| ) { | |
| return false; | |
| } | |
| return isInvalidateDate(date, { activeIndex: 0 }); | |
| }); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #975 +/- ##
==========================================
+ Coverage 98.81% 98.82% +0.01%
==========================================
Files 65 65
Lines 2695 2721 +26
Branches 749 766 +17
==========================================
+ Hits 2663 2689 +26
Misses 29 29
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 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
🧹 Nitpick comments (2)
src/PickerInput/hooks/useRangeValue.ts (2)
208-208: 💤 Low value区间模式检测逻辑较为脆弱。
使用
disabled.length > 0来判断是否为区间模式是一种启发式方法，可能在以下情况下失效：
- 如果 RangePicker 的
disabled属性被错误配置为单元素数组- 如果未来 API 发生变化
建议添加注释说明此判断的前提条件，或考虑使用更明确的方式来区分区间模式和单/多选模式（例如通过显式的
rangeValue参数，该参数在第 111 行已存在于useInnerValue中）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/PickerInput/hooks/useRangeValue.ts` at line 208, The current isRangeValue = disabled.length > 0 heuristic is brittle; replace it with an explicit check using the existing rangeValue signal from useInnerValue or at least guard with a comment about the assumption. Update useRangeValue to derive isRangeValue from the imported/returned rangeValue (or add a new explicit boolean param) instead of relying solely on disabled.length, and ensure references to disabled, isRangeValue, useInnerValue and rangeValue are updated accordingly so the detection is deterministic.
281-291: 为isInvalidateChangedDate的布尔含义与边界补齐注释/测试用例
isInvalidateChangedDate的返回条件是：nextInvalidate && (changed || !prevDate || !isInvalidateDate(prevDate, prevInfo))，配合validateDates里的取反逻辑，等价于：当start/end在当前信息下为无效时，若日期发生变化或前值不存在或前值在 prevInfo 下仍是有效值，则会阻止该更新；只有“无效且未变化（且前值本身也无效）”才允许保留。仓库
tests已覆盖invalidate警告、preserveInvalidOnBlur、以及部分 disabledTime/disabledDate 的提交阻断，但未看到针对“无效 A -> 无效 B（changed=true）”这一分支的专门用例（isInvalidateChangedDate相关断言在测试中没有出现）。
建议：
- 增加注释，把该返回值如何影响
validateDates的最终“是否阻止提交/更新”讲清楚- 补测试：range 下
start/end从无效改为另一个无效（changed=true）时，OK/提交是否应被阻止（end还需覆盖from信息变更场景）🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/PickerInput/hooks/useRangeValue.ts` around lines 281 - 291, Add a clear inline comment above isInvalidateChangedDate explaining that it returns true when the next date is invalid under nextInfo and should therefore be blocked from updating unless the previous date was already invalid under prevInfo and unchanged; reference how validateDates negates this to decide whether to prevent submission/update. Then add unit tests for range mode that cover the branch "invalid A -> invalid B (changed = true)" for both start and end: assert the UI/state blocks the update/submission when changed=true and prevInfo indicates previous value was valid, and also add a case where end's from field changes (prevInfo.from vs nextInfo.from) to ensure the blocking behavior matches the isInvalidateChangedDate logic. Ensure tests target the isInvalidateChangedDate behavior via validateDates outcomes (submission prevented vs allowed).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/PickerInput/hooks/useRangeValue.ts`:
- Around line 293-319: Add unit tests that cover the "invalid -> invalid"
transition where a date change flag is true but the new date remains invalid so
updates must be blocked; specifically, in tests/range.spec.tsx add scenarios for
range mode where startChanged or endChanged is true and isInvalidateChangedDate
still returns true to assert the other side's update is prevented, and in
tests/multiple.spec.tsx add a case where an item is changed (changed flag true)
and includesTimestamp(mergedValue, date) is false while isInvalidateDate(date)
is true to assert the new/changed entry is rejected; reference validateDates,
isInvalidateChangedDate, isInvalidateDate, includesTimestamp, mergedValue,
clone, disabled, startChanged and endChanged when locating relevant logic to
simulate and assert the behavior.
---
Nitpick comments:
In `@src/PickerInput/hooks/useRangeValue.ts`:
- Line 208: The current isRangeValue = disabled.length > 0 heuristic is brittle;
replace it with an explicit check using the existing rangeValue signal from
useInnerValue or at least guard with a comment about the assumption. Update
useRangeValue to derive isRangeValue from the imported/returned rangeValue (or
add a new explicit boolean param) instead of relying solely on disabled.length,
and ensure references to disabled, isRangeValue, useInnerValue and rangeValue
are updated accordingly so the detection is deterministic.
- Around line 281-291: Add a clear inline comment above isInvalidateChangedDate
explaining that it returns true when the next date is invalid under nextInfo and
should therefore be blocked from updating unless the previous date was already
invalid under prevInfo and unchanged; reference how validateDates negates this
to decide whether to prevent submission/update. Then add unit tests for range
mode that cover the branch "invalid A -> invalid B (changed = true)" for both
start and end: assert the UI/state blocks the update/submission when
changed=true and prevInfo indicates previous value was valid, and also add a
case where end's from field changes (prevInfo.from vs nextInfo.from) to ensure
the blocking behavior matches the isInvalidateChangedDate logic. Ensure tests
target the isInvalidateChangedDate behavior via validateDates outcomes
(submission prevented vs allowed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 493ad7e4-cfe4-4d1f-92a6-7308e4ceb3f0
📒 Files selected for processing (4)
src/PickerInput/SinglePicker.tsxsrc/PickerInput/hooks/useRangeValue.tstests/multiple.spec.tsxtests/range.spec.tsx
| const validateDates = isRangeValue | ||
| ? // Validate range dates. Existing invalid values should not block other updates. | ||
| (disabled[0] || | ||
| !start || | ||
| !isInvalidateChangedDate( | ||
| start, | ||
| prevStart, | ||
| startChanged, | ||
| { activeIndex: 0 }, | ||
| { activeIndex: 0 }, | ||
| )) && | ||
| (disabled[1] || | ||
| !end || | ||
| !isInvalidateChangedDate( | ||
| end, | ||
| prevEnd, | ||
| endChanged, | ||
| { from: prevStart, activeIndex: 1 }, | ||
| { from: start, activeIndex: 1 }, | ||
| )) | ||
| : // Validate single or multiple dates. Existing values may be disabled by updated `disabledDate`. | ||
| clone.every( | ||
| (date) => | ||
| !date || | ||
| includesTimestamp(generateConfig, mergedValue, date) || | ||
| !isInvalidateDate(date, { activeIndex: 0 }), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证相关测试用例是否覆盖了关键场景
echo "=== 检查区间模式测试 ==="
rg -n "does not block.*when existing" tests/range.spec.tsx
echo -e "\n=== 检查多选模式测试 ==="
rg -n "does not block.*when existing" tests/multiple.spec.tsx
echo -e "\n=== 检查是否有测试覆盖从无效到无效的变更 ==="
rg -n -C 3 "invalid.*to.*invalid|disabled.*to.*disabled" tests/ --type=tsRepository: react-component/picker
Length of output: 793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 读取 validateDates 所在的上下文（含 lines 293-319 附近）
echo "=== useRangeValue.ts: validateDates 上下文 ==="
FILE="src/PickerInput/hooks/useRangeValue.ts"
nl -ba "$FILE" | sed -n '260,360p'
# 定位 helper：isInvalidateChangedDate / isInvalidateDate 的实现
echo -e "\n=== 查找 isInvalidateChangedDate 实现位置 ==="
rg -n "function isInvalidateChangedDate|const isInvalidateChangedDate|isInvalidateChangedDate\s*=" "$FILE"
echo -e "\n=== 查找 isInvalidateDate 实现位置 ==="
rg -n "function isInvalidateDate|const isInvalidateDate|isInvalidateDate\s*=" "$FILE"
# 展示 helper 的实现片段（尽量缩小范围）
echo -e "\n=== isInvalidateChangedDate 实现片段 ==="
python3 - <<'PY'
import re, subprocess, os, textwrap, sys
file="src/PickerInput/hooks/useRangeValue.ts"
rg = subprocess.check_output(["rg","-n",r"isInvalidateChangedDate",file], text=True)
lines=[int(l.split(":")[0]) for l in rg.splitlines() if l.strip()]
if not lines:
print("not found")
sys.exit(0)
start=min(lines)-20
end=max(lines)+80
cmd=f"nl -ba {file} | sed -n '{start},{end}p'"
print(subprocess.check_output(["bash","-lc",cmd], text=True))
PY
echo -e "\n=== isInvalidateDate 实现片段 ==="
python3 - <<'PY'
import re, subprocess, os, textwrap, sys
file="src/PickerInput/hooks/useRangeValue.ts"
rg = subprocess.check_output(["rg","-n",r"isInvalidateDate",file], text=True)
lines=[int(l.split(":")[0]) for l in rg.splitlines() if l.strip()]
if not lines:
print("not found")
sys.exit(0)
start=min(lines)-20
end=max(lines)+80
cmd=f"nl -ba {file} | sed -n '{start},{end}p'"
print(subprocess.check_output(["bash","-lc",cmd], text=True))
PY
# 确认 disabled/start/end/prevStart/prevEnd/startChanged/endChanged 等变量定义来源
echo -e "\n=== 查找 disabled/start/end/prevStart/prevEnd/startChanged/endChanged 定义片段 ==="
for sym in disabled start end prevStart prevEnd startChanged endChanged isRangeValue clone:
echo -e "\n-- $sym --"
rg -n "\\b$sym\\b" "$FILE" | head -n 20
doneRepository: react-component/picker
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/PickerInput/hooks/useRangeValue.ts"
echo "=== useRangeValue.ts: validateDates 上下文（lines 293-330附近）==="
# 用 cat -n 替代 nl
cat -n "$FILE" | sed -n '280,350p'
echo -e "\n=== 查找 isInvalidateChangedDate 定义/实现位置 ==="
rg -n "function isInvalidateChangedDate|const isInvalidateChangedDate|isInvalidateChangedDate\s*=" "$FILE" || true
echo -e "\n=== 查找 isInvalidateDate 定义/实现位置 ==="
rg -n "function isInvalidateDate|const isInvalidateDate|isInvalidateDate\s*=" "$FILE" || true
echo -e "\n=== 提取 isInvalidateChangedDate 实现片段（尽量缩小范围）==="
# 以匹配到的最近行号为中心输出附近 60 行
ln=$(rg -n "isInvalidateChangedDate" "$FILE" | head -n 1 | cut -d: -f1)
start=$((ln-30)); end=$((ln+60))
cat -n "$FILE" | sed -n "${start},${end}p" || true
echo -e "\n=== 提取 isInvalidateDate 实现片段（尽量缩小范围）==="
ln=$(rg -n "isInvalidateDate" "$FILE" | head -n 1 | cut -d: -f1)
start=$((ln-30)); end=$((ln+60))
cat -n "$FILE" | sed -n "${start},${end}p" || true
echo -e "\n=== 查找 disabled/start/end/prevStart/prevEnd/startChanged/endChanged/clone 的使用上下文 ==="
for sym in disabled start end prevStart prevEnd startChanged endChanged clone:
do
echo -e "\n-- $sym --"
rg -n "\\b${sym}\\b" "$FILE" | head -n 20
doneRepository: react-component/picker
Length of output: 12243
补强：区间/单多选 validateDates 逻辑基本符合预期，但测试仍需补上“无效->无效（changed=true 且仍无效）”用例。
- 区间模式（293-312）：start/end 分别在
disabled[x] || 为空 || !isInvalidateChangedDate(...)下放行；当目标端仍无效但未变化且前值也无效时，isInvalidateChangedDate返回 false，从而不会阻塞另一端更新。 - 单/多选模式（314-319）：通过
includesTimestamp(...mergedValue, date)放行已有选择（即使当前isInvalidateDate(date)为真），只有新增/变化后的日期才要求!isInvalidateDate(date)，因此会阻止新增无效日期。 - 测试覆盖：已覆盖
disabledDate下的 existing value 不阻塞场景tests/range.spec.tsx（existing value disabled / existing end disabled）tests/multiple.spec.tsx（existing value disabled）
- 建议补充：未见专门覆盖“无效值从无效到无效”（对应
changed=true且nextInvalidate仍为 true，从而应拦截更新）的用例；可增加针对startChanged/endChanged为 true 时的阻断断言。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/PickerInput/hooks/useRangeValue.ts` around lines 293 - 319, Add unit
tests that cover the "invalid -> invalid" transition where a date change flag is
true but the new date remains invalid so updates must be blocked; specifically,
in tests/range.spec.tsx add scenarios for range mode where startChanged or
endChanged is true and isInvalidateChangedDate still returns true to assert the
other side's update is prevented, and in tests/multiple.spec.tsx add a case
where an item is changed (changed flag true) and includesTimestamp(mergedValue,
date) is false while isInvalidateDate(date) is true to assert the new/changed
entry is rejected; reference validateDates, isInvalidateChangedDate,
isInvalidateDate, includesTimestamp, mergedValue, clone, disabled, startChanged
and endChanged when locating relevant logic to simulate and assert the behavior.
1 participant
背景
当 DatePicker/RangePicker 已有选中值后来被 disabledDate 判定为 disabled 时，当前提交逻辑会全量校验已有值，导致用户修改其他合法日期时 onChange 不触发。antd issue: https://github.com/ant-design/ant-design/issues/58330。
修改内容
验证
Summary by CodeRabbit
发布说明
Bug Fixes
Tests