-
-
Notifications
You must be signed in to change notification settings - Fork 275
fix: field value updates may fail multiple times #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改在 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MockBtnInput
participant FormField
User->>MockBtnInput: 点击按钮
MockBtnInput->>FormField: onChange("")
MockBtnInput->>FormField: onChange("A")
FormField->>FormField: 标记 touched/dirty,触发 meta 事件
FormField->>FormField: 获取最新 curValue
FormField->>FormField: 归一化、比较并更新值
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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
tests/field.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. src/Field.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Field.tsx (1)
608-622
: 引入curValue
合理,但可再压缩一次getValue
调用当前实现先在
getControlled
开头计算了const value = this.getValue();
,随后在触发器内再次执行this.getValue()
得到curValue
。
虽然两次调用都很轻量,但在高频输入场景仍是一次不必要的遍历(getFieldsValue(true)
会克隆整棵 store)。可以直接删除最前面的value
变量,改为在外层利用curValue
,从而避免一次冗余读取,逻辑上也更统一。- const value = this.getValue(); + // 延后到真正渲染子节点时再取值,避免后续再次读取其余修改(归一化及比较均改用最新值)完全解决了 #753 的时序问题,👍。
tests/field.test.tsx (1)
76-108
: 测试覆盖到位,但等待方式仍有潜在隐患
await act(async () => timeout())
依赖固定 10 ms 定时器,若后续实现改为批量更新或启用flushSync
,该时长可能不足/过多导致偶发失败。
建议改用 Testing Library 提供的waitFor
或findBy
系列 API 监听实际 DOM 变动,而不是硬编码时间。- await act(async () => { - await timeout(); - }); + await waitFor(() => + expect(form.current?.getFieldValue('input')).toBe('A'), + );这样既能保证可靠性,也能让测试速度随实现自动收敛。
代码其余部分无问题,成功重现并验证修复效果。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Field.tsx
(2 hunks)tests/field.test.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/field.test.tsx (3)
src/interface.ts (1)
FormInstance
(254-276)tests/common/InfoField.tsx (1)
Input
(9-11)tests/common/timeout.ts (1)
timeout
(3-7)
fix #753
Summary by CodeRabbit