fix(form): prevent useWatch from triggering twice during Form.List up… #772
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. |
|
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移除 BatchUpdate 批量更新组件并引入 WatcherCenter 以集中批处理通知；重构 useForm/useWatch 通知路径与模块导入；测试改为使用假定时并新增用例验证 Form.List 删除时无中间 undefined 状态。 Changes
Sequence Diagram(s)sequenceDiagram
participant Form as FormStore
participant Watcher as WatcherCenter
participant Scheduler as MessageChannel (macroTask)
participant WatchCB as Watcher Callback
Note over Form,Watcher: 字段/API 更新导致 notify 调用
Form->>Watcher: notify(namePath[])
Watcher->>Watcher: 合并并去重 namePathList
Watcher->>Scheduler: postMessage()（调度宏任务）
Scheduler-->>Watcher: onmessage（执行宏任务）
Watcher->>Form: getFieldsValue()/getFieldsValue(true)
Watcher->>WatchCB: forEach(cb) cb(values, allValues, namePathList)
Watcher->>Watcher: 清空 namePathList
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 需要重点检查：
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 @QDyanbing, 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 addresses a specific bug within 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}
There was a problem hiding this comment.
Code Review
这个 PR 旨在修复 Form.List 更新（如 remove）时 useWatch 会被多次触发并可能读到不稳定中间态数据的问题。通过在 updateValue 方法中将 notifyWatch 的直接调用改为 batchNotifyWatch，利用 useLayoutEffect 将通知批量处理，确保 useWatch 只在所有同步状态更新完成后收到一次通知，从而获取到最终的稳定状态。这个改动是合理的，并且新增的测试用例也很好地覆盖了 Form.List remove 操作的场景，验证了中间状态不会被观察到。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/useWatch.test.tsx (1)
540-603: 建议使用remove(index)保持与现有测试的一致性。测试逻辑正确，有效验证了修复效果。但是，Line 562 使用了硬编码的
remove(1)，而现有的列表测试（Line 184）使用的是remove(index)。建议应用以下修改以保持一致性：
- <a className="remove" onClick={() => remove(1)} /> + <a className="remove" onClick={() => remove(index)} />这样做不会改变测试行为（因为测试只点击第一个 remove 按钮），但会使代码更符合既定模式，减少未来维护时的困惑。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/useForm.ts(1 hunks)tests/useWatch.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/useWatch.test.tsx (4)
docs/examples/useWatch-selector.tsx (1)
form(12-42)docs/examples/useWatch.tsx (1)
form(38-123)src/index.tsx (2)
List(34-34)Field(34-34)tests/common/InfoField.tsx (1)
Input(9-11)
🔇 Additional comments (2)
src/useForm.ts (2)
220-227: 实现正确：批量通知机制有效防止中间状态。
batchNotifyWatch方法正确地将多个 namePath 更新累积到一起，并通过现有的批处理机制在单次回调中触发所有监听器。这确保了useWatch不会收到中间的 undefined 值。
778-778: 正确应用批量通知，修复了 Form.List 删除项时的中间状态问题。将
notifyWatch([namePath])替换为batchNotifyWatch(namePath)确保了在快速连续更新期间（如 Form.List 删除操作），监听器只会在批处理完成后收到最终状态，而不会收到中间的 undefined 值。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #772 +/- ##
==========================================
- Coverage 99.53% 99.53% -0.01%
==========================================
Files 19 19
Lines 1299 1291 -8
Branches 319 303 -16
==========================================
- Hits 1293 1285 -8
Misses 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Form.tsx (1)
1-189: 移除 src/interface.ts 中未使用的 BatchUpdate 导入。在 src/interface.ts 中发现了对不存在的 BatchUpdate 模块的未使用导入：
import type { BatchTask } from './BatchUpdate';。该导入应被完全移除，以完成 BatchUpdate 组件的彻底清理。
🧹 Nitpick comments (6)
docs/examples/debug.tsx (1)
47-47: 按钮缺少type="button"属性这个按钮没有指定
type属性，在<form>标签内默认会是type="submit"，可能导致意外的表单提交行为。🔎 建议修复
- <button onClick={() => remove(1)}>删除索引 1</button> + <button type="button" onClick={() => remove(1)}>删除索引 1</button>src/hooks/useForm.ts (4)
202-222: 建议删除注释掉的代码这些注释掉的代码是旧的批处理逻辑的残留，会影响代码可读性。建议在 PR 合并前删除这些注释。
🔎 建议修复
private notifyWatch = (namePath: InternalNamePath[] = []) => { - // // No need to cost perf when nothing need to watch - // if (this.watchList.length) { - // const values = this.getFieldsValue(); - // const allValues = this.getFieldsValue(true); - - // this.watchList.forEach(callback => { - // callback(values, allValues, namePath); - // }); - // } this.watcherCenter.notify(namePath); }; - - // private notifyWatchNamePathList: InternalNamePath[] = []; - // private batchNotifyWatch = (namePath: InternalNamePath) => { - // this.notifyWatchNamePathList.push(namePath); - // this.batch('notifyWatch', () => { - // this.notifyWatch(this.notifyWatchNamePathList); - // this.notifyWatchNamePathList = []; - // }); - // };
655-656: 同样建议删除注释此处也有类似的注释残留。
🔎 建议修复
- // this.batchNotifyWatch(namePath); this.notifyWatch([namePath]);
696-697: 同样建议删除注释🔎 建议修复
- // this.batchNotifyWatch(namePath); this.notifyWatch([namePath]);
763-764: 同样建议删除注释🔎 建议修复
- // this.batchNotifyWatch(namePath); this.notifyWatch([namePath]);src/hooks/useNotifyWatch.ts (1)
44-61: 批处理执行逻辑
taskId递增和比较机制确保了只有最新的批次会执行回调，避免了过时数据的问题。执行完成后清空namePathList也是正确的做法。一个小建议：可以考虑在
watcherList.size为 0 时提前返回，避免不必要的macroTask调度。🔎 可选优化：提前检查是否有监听器
public notify(namePath: InternalNamePath[]) { + // Skip if no watchers registered + if (!this.watcherList.size) { + return; + } + // Insert with deduplication namePath.forEach(path => { if (this.namePathList.every(exist => !matchNamePath(exist, path))) { this.namePathList.push(path); } }); this.doBatch(); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/examples/debug.tsx(1 hunks)package.json(1 hunks)src/BatchUpdate.tsx(0 hunks)src/FieldContext.ts(0 hunks)src/Form.tsx(1 hunks)src/hooks/useForm.ts(8 hunks)src/hooks/useNotifyWatch.ts(1 hunks)src/hooks/useWatch.ts(1 hunks)src/index.tsx(1 hunks)src/interface.ts(0 hunks)src/utils/valueUtil.ts(1 hunks)tests/common/index.ts(2 hunks)tests/dependencies.test.tsx(4 hunks)tests/index.test.tsx(5 hunks)tests/setupAfterEnv.ts(1 hunks)tests/useWatch.test.tsx(13 hunks)tests/validate.test.tsx(2 hunks)
💤 Files with no reviewable changes (3)
- src/interface.ts
- src/FieldContext.ts
- src/BatchUpdate.tsx
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/useWatch.test.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
tests/common/index.ts (1)
tests/common/timeout.ts (1)
timeout(3-7)
tests/validate.test.tsx (2)
tests/common/timeout.ts (1)
waitFakeTime(9-15)tests/common/index.ts (3)
matchError(49-77)changeValue(29-47)getInput(5-25)
src/hooks/useForm.ts (2)
src/hooks/useNotifyWatch.ts (1)
WatcherCenter(14-62)src/interface.ts (1)
InternalNamePath(6-6)
tests/dependencies.test.tsx (1)
tests/common/timeout.ts (1)
waitFakeTime(9-15)
tests/index.test.tsx (2)
tests/common/timeout.ts (1)
waitFakeTime(9-15)tests/common/index.ts (1)
matchError(49-77)
🔇 Additional comments (14)
src/utils/valueUtil.ts (1)
51-51: LGTM！文档改进清晰。参数说明更新准确地反映了
partialMatch的默认值行为，与函数签名（第 56 行）中的partialMatch = false一致。tests/setupAfterEnv.ts (1)
3-30: MessageChannel mock 实现正确。该 mock 正确实现了端口间的消息传递机制。
postMessage使用 10ms 延迟来模拟异步行为，这与测试套件中新引入的waitFakeTime工具和假计时器模式保持一致。src/index.tsx (2)
5-5: LGTM！导入路径更新正确。将
useForm的导入路径从'./useForm'更新为'./hooks/useForm'，与项目重组 hook 到src/hooks/目录的架构一致。
11-11: LGTM！导入路径更新正确。将
useWatch的导入路径更新为'./hooks/useWatch'，与项目的模块化重组保持一致。tests/common/index.ts (2)
27-27: 计时器检测模式实现巧妙。通过在模块作用域捕获原生
setTimeout，可以在测试中可靠地检测 Jest 是否已模拟计时器。这种模式使changeValue函数能够同时支持真实计时器和假计时器。
29-46: LGTM！条件计时器处理正确。
changeValue函数正确检测模拟计时器并相应地使用jest.advanceTimersByTime(1000)或实际的timeout()。这确保了测试在两种计时器模式下都能正常工作。tests/validate.test.tsx (1)
290-318: LGTM！假计时器迁移正确。"form context" 测试已正确迁移以使用假计时器：
- 在测试开始时设置
jest.useFakeTimers()- 用
waitFakeTime()替换timeout()调用- 在测试结束时正确清理
jest.useRealTimers()这确保了确定性的定时行为并与测试套件中更广泛的假计时器模式保持一致。
tests/dependencies.test.tsx (1)
102-155: LGTM！假计时器迁移正确。"should work when field is dirty" 测试已正确更新：
- 在测试开始时设置假计时器
- 用
waitFakeTime()替换timeout()以获得确定性的异步行为- 在测试结束时清理假计时器
这与测试套件中更广泛的假计时器迁移保持一致，并确保验证流程的可预测时序。
src/hooks/useWatch.ts (1)
3-12: LGTM！导入路径更新正确。所有导入路径已正确更新以反映文件从
src/移动到src/hooks/目录：
- FieldContext：
'./FieldContext'→'../FieldContext'- interface：
'./interface'→'../interface'- typeUtil：
'./utils/typeUtil'→'../utils/typeUtil'- valueUtil：
'./utils/valueUtil'→'../utils/valueUtil'这些更改纯粹是路径调整，没有功能性更改。
src/Form.tsx (1)
11-11: LGTM！导入路径更新正确。将
useForm的导入路径从'./useForm'更新为'./hooks/useForm'，与项目重组 hooks 到src/hooks/目录的架构一致。tests/index.test.tsx (1)
313-353: 测试改进：使用假定时器处理宏任务批处理这些更改正确地将提交测试适配为使用假定时器，以配合新的
WatcherCenter宏任务批处理机制。jest.useFakeTimers()和jest.useRealTimers()的配对使用确保了定时器状态在测试后正确恢复。src/hooks/useForm.ts (1)
78-84: WatcherCenter 集成实现正确使用
WatcherCenter替代之前的批处理逻辑是合理的架构改进。通过宏任务延迟通知，可以有效避免Form.List操作过程中产生的中间状态触发useWatch多次回调的问题。src/hooks/useNotifyWatch.ts (2)
33-42: 路径去重逻辑正确使用
matchNamePath进行去重可以正确处理嵌套路径的情况，避免重复通知。doBatch在每次notify调用时都会被触发，但通过taskId机制确保只有最后一次调度的回调会真正执行。
5-12: 使用 MessageChannel 实现宏任务批处理效果最优setTimeout(cb, 0) 在嵌套超过5层后会产生至少 4ms 的延迟，这使得 setTimeout 不是队列宏任务的最可靠方式。相比之下，MessageChannel 规避了这个限制，总是比 setTimeout 更快执行且耗时更少。这个选择对解决 Form.List 操作中的中间状态问题至关重要。
关于浏览器兼容性：MessageChannel 在 IE 10 及以上版本中支持，仅需关注 IE9 及更早版本的兼容性处理。MessageChannel 在跨浏览器中的可用性自 2015 年 9 月起已广泛支持，现代浏览器的兼容性不成问题。
tests/common/index.ts
Outdated
| export function executeMicroTasks() { | ||
| jest.advanceTimersByTime(1000); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找测试中计时器推进的使用模式和持续时间
rg -n "advanceTimersByTime|waitFakeTime" tests/ -A 2 -B 2Repository: react-component/field-form
Length of output: 11308
🏁 Script executed:
# 查找 executeMicroTasks 的调用位置
rg -n "executeMicroTasks" tests/ -B 3 -A 3Repository: react-component/field-form
Length of output: 308
🏁 Script executed:
# 查看 executeMicroTasks 周围的完整上下文和其他辅助函数
cat -n tests/common/index.ts | head -120 | tail -30Repository: react-component/field-form
Length of output: 984
🏁 Script executed:
# 搜索整个测试目录看 executeMicroTasks 是否被导入或调用
rg -n "executeMicroTasks" .Repository: react-component/field-form
Length of output: 135
🏁 Script executed:
# 检查 tests/common/index.ts 的导出内容
cat -n tests/common/index.ts | grep "export"Repository: react-component/field-form
Length of output: 427
🏁 Script executed:
# 查看完整的 tests/common/index.ts 文件，特别是包含 act 和计时器相关的代码
cat -n tests/common/index.ts | head -50Repository: react-component/field-form
Length of output: 1695
补充 act() 包装以保持一致性。
executeMicroTasks() 函数缺少 act() 包装，与同一文件中 changeValue() 函数的做法（第 39 行）不一致。应改为：
export function executeMicroTasks() {
act(() => {
jest.advanceTimersByTime(1000);
});
}1000ms 的持续时间与现有测试模式一致（如 changeValue() 中的用法），适合处理 WatcherCenter 批处理和消息通道调度。但必须用 act() 包装来确保 React 的副作用和状态更新得到正确处理。
🤖 Prompt for AI Agents
In tests/common/index.ts around lines 108 to 110, the executeMicroTasks()
function should be changed to wrap the jest.advanceTimersByTime(1000) call in
React's act() to match changeValue() behavior; update the function so it calls
act(() => { jest.advanceTimersByTime(1000); }); keeping the 1000ms duration
unchanged.
2 participants
fix: ant-design/ant-design#56230
Form.List 的更新（如 remove）内部会触发多次状态变更。在当前实现中，useWatch 会在这些内部步骤中被同步触发，导致一次操作出现多次更新，并可能读到尚未稳定的中间态数据。
Summary by CodeRabbit
发布说明
文档
测试
变更
杂项
✏️ Tip: You can customize this high-level summary in your review settings.