chore: add delayFrame option to validateFields#777
Conversation
- Add delayFrame option to InternalValidateOptions interface - Conditionally call delayFrame when delayFrame option is true in Field.validateRules - Use delayFrame: true in triggerDependenciesUpdate to avoid useWatch rule timing issues - Adjust test files (jest.mock formatting cleanup)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, 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 enhances the form validation mechanism by introducing a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
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
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Walkthrough本次更改引入可选的 Changes
Sequence Diagram(s)sequenceDiagram
participant Trigger as "triggerDependenciesUpdate"
participant Validator as "validateFields"
participant Field as "Field.validate"
participant Rules as "Rule Evaluation"
participant Observer as "Observers"
Trigger->>Validator: validateFields(delayFrame: true)
Validator->>Field: 调用字段验证并传入 options
alt options.delayFrame === true
Field->>Field: await delayFrame()
end
Field->>Rules: 评估规则（同步或异步）
Rules->>Observer: 更新验证结果
Observer-->>Trigger: 通知 / 继续依赖传播
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟 Possibly related PRs
Suggested reviewers
诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 introduces a delayFrame option for validateFields to address timing issues with useWatch during dependency updates. The implementation is sound, but I've identified a potential regression. By making the frame delay conditional, it's removed from most validation paths, which could break existing forms that rely on this delay. I've suggested a modification to make this change non-breaking while still allowing for performance optimizations.
| if (showDelayFrame) { | ||
| await delayFrame(); | ||
| } |
There was a problem hiding this comment.
This change makes the delayFrame conditional. While this is useful for adding the new option, it changes the default behavior. Previously, await delayFrame() was called unconditionally, which helped prevent race conditions with useWatch in validation rules.
Now, the delay only occurs if delayFrame: true is explicitly passed. This removes the delay for standard validation triggers (like onChange) and manual form.validateFields() calls, which could be a regression for users relying on this behavior (e.g., calling validateFields immediately after a state change that useWatch depends on).
If the goal is to allow opting out of the delay for performance, rather than requiring an opt-in, I'd suggest keeping the delay by default to avoid a breaking change. This can be done by checking if showDelayFrame is not explicitly false.
This would maintain backward compatibility while still providing a path for performance optimization.
| if (showDelayFrame) { | |
| await delayFrame(); | |
| } | |
| if (showDelayFrame !== false) { | |
| await delayFrame(); | |
| } |
The jest.mock('../src/utils/delayUtil') was removed from multiple test files since it's no longer needed after the delayFrame implementation.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/index.test.tsx (1)
4-4:⚠️ Potential issue | 🔴 Critical缺失类型导入导致测试编译失败。
文件中大量使用
FormRef（15+ 处）和Meta（至少 1 处），但未导入这两个类型，直接触发编译错误。需在导入区添加这些类型。🔧 建议修复
import React from 'react'; import type { FormInstance } from '../src'; +import type { FormRef, Meta } from '../src/interface'; import Form, { Field, useForm } from '../src';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/index.test.tsx` at line 4, The tests fail to compile because the types FormRef and Meta are used but not imported; update the type import statement that currently imports FormInstance to also import FormRef and Meta (e.g., change the import line importing FormInstance to import type { FormInstance, FormRef, Meta } from the same module) so all referenced types (FormRef and Meta) are available to the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/index.test.tsx`:
- Line 4: The tests fail to compile because the types FormRef and Meta are used
but not imported; update the type import statement that currently imports
FormInstance to also import FormRef and Meta (e.g., change the import line
importing FormInstance to import type { FormInstance, FormRef, Meta } from the
same module) so all referenced types (FormRef and Meta) are available to the
tests.
ℹ️ Review info
Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c204b9f-8bcb-4d25-bfa6-1fa436ad2062
📒 Files selected for processing (6)
src/Field.tsxsrc/hooks/useForm.tssrc/interface.tstests/index.test.tsxtests/list.test.tsxtests/validate-warning.test.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #777 +/- ##
=======================================
Coverage 99.54% 99.54%
=======================================
Files 20 20
Lines 1324 1325 +1
Branches 329 309 -20
=======================================
+ Hits 1318 1319 +1
Misses 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary by CodeRabbit