Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace @rc-component/util/lib/raf with custom delayFrame utility - Export macroTask for test usage - Add delayUtil.ts with mockable delayFrame implementation - Update tests to use fake timers and waitFakeTime for better control - Add jest.mock for delayUtil in test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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! 此拉取请求旨在解决表单验证规则在依赖于 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
Activity
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 infoConfiguration 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)
Walkthrough在验证规则执行前引入帧延迟：新增 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #776 +/- ##
=======================================
Coverage 99.54% 99.54%
=======================================
Files 19 20 +1
Lines 1311 1324 +13
Branches 329 308 -21
=======================================
+ Hits 1305 1318 +13
Misses 6 6 ☔ 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: 2
🧹 Nitpick comments (1)
tests/common/timeout.ts (1)
14-14: 建议消除11这个魔法数字。建议抽成具名常量，避免后续维护时误解其语义。
♻️ 可选改动示例
export async function waitFakeTime(timeout: number = 10) { + const MACRO_TASK_FLUSH_MS = 11; await act(async () => { await new Promise<void>(resolve => { macroTask(resolve); - jest.advanceTimersByTime(11); + jest.advanceTimersByTime(MACRO_TASK_FLUSH_MS); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/common/timeout.ts` at line 14, Replace the magic number 11 used in jest.advanceTimersByTime(11) with a named constant to clarify meaning and ease maintenance: define a descriptive constant (e.g., TIMEOUT_MS or DEFAULT_TIMEOUT_MS) near the top of tests/common/timeout.ts (or export it if shared) and use that constant in place of the literal in the jest.advanceTimersByTime call to make the intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/__mocks__/src/utils/delayUtil.ts`:
- Line 2: 在 tests/__mocks__/src/utils/delayUtil.ts 的 mock 实现中移除调试日志
console.log('@@@')：打开该文件找到 console.log('@@@') 并删除该行（或改为无副作用的注释），确保 mock
只包含必要的导出/实现且不在 CI 输出中产生噪音，随后运行相关测试以确认未影响断言或行为。
In `@tests/dependencies.test.tsx`:
- Around line 279-303: The test intermittently asserts before async validation
settles; update the test that defines Demo (using InfoField,
Form.useForm/useWatch) to use fake timers and explicitly advance them or wait
after changeValue calls: call jest.useFakeTimers() at test start, then after
each changeValue(getInput(...)) call advance timers (jest.runAllTimers() or
jest.advanceTimersByTime(...)) inside an act/await helper so the validation
finishes before calling matchError; ensure to restore timers after the test.
---
Nitpick comments:
In `@tests/common/timeout.ts`:
- Line 14: Replace the magic number 11 used in jest.advanceTimersByTime(11) with
a named constant to clarify meaning and ease maintenance: define a descriptive
constant (e.g., TIMEOUT_MS or DEFAULT_TIMEOUT_MS) near the top of
tests/common/timeout.ts (or export it if shared) and use that constant in place
of the literal in the jest.advanceTimersByTime call to make the intent explicit.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/Field.tsxsrc/hooks/useNotifyWatch.tssrc/utils/delayUtil.tstests/__mocks__/src/utils/delayUtil.tstests/common/timeout.tstests/context.test.tsxtests/dependencies.test.tsxtests/index.test.tsxtests/list.test.tsxtests/validate-warning.test.tsxtests/validate.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/dependencies.test.tsx (1)
279-303:⚠️ Potential issue | 🟡 Minor新增依赖回归用例仍缺少显式等待，存在时序抖动风险。
Line 297 和 Line 301 之后直接断言，可能早于异步校验完成。建议该用例也使用 fake timers，并在每次输入后等待一次。
🧪 建议修复
it('error should be cleared when dependency field changes and rule becomes false', async () => { + jest.useFakeTimers(); + const Demo = () => { const [form] = Form.useForm(); const type = Form.useWatch('type', form); @@ const { container } = render(<Demo />); await changeValue(getInput(container, 1), ['bamboo', '']); + await waitFakeTime(); matchError(container, true); @@ // Change type to make rule true await changeValue(getInput(container), '1'); + await waitFakeTime(); matchError(container, false); });#!/bin/bash # 只读验证：检查该测试块是否包含 useFakeTimers 和等待次数 python - <<'PY' from pathlib import Path p = Path("tests/dependencies.test.tsx") txt = p.read_text(encoding="utf-8") title = "it('error should be cleared when dependency field changes and rule becomes false'" start = txt.find(title) if start == -1: print("test block not found") raise SystemExit(1) end = txt.find("\n });", start) block = txt[start:end] print("has useFakeTimers:", "jest.useFakeTimers()" in block) print("waitFakeTime count:", block.count("waitFakeTime(")) print("changeValue count:", block.count("changeValue(")) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/dependencies.test.tsx` around lines 279 - 303, The test "error should be cleared when dependency field changes and rule becomes false" risks flakiness because it asserts immediately after asynchronous validation; update the test to use fake timers (jest.useFakeTimers()) and await a waitFakeTime() (or advance timers) after each changeValue call so validation completes before assertions; specifically, add jest.useFakeTimers() at the start of the test and insert await waitFakeTime() (or equivalent timer advance) after changeValue(getInput(container, 1)... ) and after changeValue(getInput(container), '1') so that the asserts via matchError run only after async validation finishes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/__mocks__/src/utils/delayUtil.ts`:
- Around line 2-3: There are two identical "return Promise.resolve();"
statements in tests/__mocks__/src/utils/delayUtil.ts causing an unreachable
statement lint error; remove the duplicate so only a single "return
Promise.resolve();" remains (i.e., keep the first return branch and delete the
second unreachable one) within the mock function in delayUtil.ts.
---
Duplicate comments:
In `@tests/dependencies.test.tsx`:
- Around line 279-303: The test "error should be cleared when dependency field
changes and rule becomes false" risks flakiness because it asserts immediately
after asynchronous validation; update the test to use fake timers
(jest.useFakeTimers()) and await a waitFakeTime() (or advance timers) after each
changeValue call so validation completes before assertions; specifically, add
jest.useFakeTimers() at the start of the test and insert await waitFakeTime()
(or equivalent timer advance) after changeValue(getInput(container, 1)... ) and
after changeValue(getInput(container), '1') so that the asserts via matchError
run only after async validation finishes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/__mocks__/src/utils/delayUtil.tstests/dependencies.test.tsxtests/initialValue.test.tsxtests/useWatch.test.tsx
✅ Files skipped from review due to trivial changes (2)
- tests/useWatch.test.tsx
- tests/initialValue.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/common/timeout.ts`:
- Around line 10-22: The shared helper waitFakeTime in tests/common/timeout.ts
unconditionally advances fake timers twice by 11ms (total 22ms), which can
affect unrelated test timing; change waitFakeTime to accept an optional
parameter (e.g., extraAdvanceMs or advanceExtraFrames boolean) defaulting to
0/false and only call the additional act+jest.advanceTimersByTime when that
parameter is truthy, and update any callers that need the extra frame to pass
the flag so other tests keep their original timing; reference the waitFakeTime
helper and the two occurrences of act + jest.advanceTimersByTime blocks in this
file when making the change.
| await act(async () => { | ||
| await new Promise<void>(resolve => { | ||
| setTimeout(resolve, 11); | ||
| jest.advanceTimersByTime(11); | ||
| }); | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| await new Promise<void>(resolve => { | ||
| setTimeout(resolve, 11); | ||
| jest.advanceTimersByTime(11); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
waitFakeTime 在共享 helper 中无条件多推进 22ms，容易影响无关测试时序。
Line 10-22 的两次固定 11ms 推进会作用于所有调用方，不仅仅是 useWatch 场景；这可能提前触发其它定时逻辑，导致测试“误通过”或时序不稳定。建议把额外帧推进改为可配置参数，仅在需要的用例里显式开启。
可参考的最小改法
export async function waitFakeTime(timeout: number = 10) {
- await act(async () => {
- await new Promise<void>(resolve => {
- setTimeout(resolve, 11);
- jest.advanceTimersByTime(11);
- });
- });
-
- await act(async () => {
- await new Promise<void>(resolve => {
- setTimeout(resolve, 11);
- jest.advanceTimersByTime(11);
- });
- });
+export async function waitFakeTime(timeout: number = 10, preFrames: number = 0) {
+ for (let i = 0; i < preFrames; i += 1) {
+ await act(async () => {
+ await new Promise<void>(resolve => {
+ setTimeout(resolve, 11);
+ jest.advanceTimersByTime(11);
+ });
+ });
+ }
await act(async () => {
await Promise.resolve();
jest.advanceTimersByTime(timeout);
await Promise.resolve();
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/common/timeout.ts` around lines 10 - 22, The shared helper waitFakeTime
in tests/common/timeout.ts unconditionally advances fake timers twice by 11ms
(total 22ms), which can affect unrelated test timing; change waitFakeTime to
accept an optional parameter (e.g., extraAdvanceMs or advanceExtraFrames
boolean) defaulting to 0/false and only call the additional
act+jest.advanceTimersByTime when that parameter is truthy, and update any
callers that need the extra frame to pass the flag so other tests keep their
original timing; reference the waitFakeTime helper and the two occurrences of
act + jest.advanceTimersByTime blocks in this file when making the change.
|
不用 useWatch 也一样，这个跟 useWatch 没有关系 |
| import { changeValue, getInput } from './common'; | ||
| import timeout from './common/timeout'; | ||
|
|
||
| jest.mock('../src/utils/delayUtil'); |
There was a problem hiding this comment.
为啥要 mock，delayFrame 里面的 raf 不能在 test 中用吗，raf 有什么功能
There was a problem hiding this comment.
raf 见评论描述，测试里 mock 是因为 raf 需要 adv timer。而测试里有很多测试都是从 rc-form 里过来的，所以用的是真实的 timeout 导致测试会卡死。所以对部分老测试使用 mock 来保持时序，其他比较新的测试写法 fakeTimer 的不动也自己过了。
2 participants
鸡生蛋蛋生鸡的问题，
useWatch之前有问题会导致额外的 render，而正好这又触发了 validator 使得可以拿到最新的 watch 值。在validateRule里由于本来就是异步的，所以添加一个raf延迟在getRules之前以保证得到的是最新的规则。ref ant-design/ant-design#56962
Summary by CodeRabbit
发行说明
Bug 修复
Tests