feat(setFieldError): simple api to update error message of single field#780
feat(setFieldError): simple api to update error message of single field#780guoyunhe wants to merge 2 commits intoreact-component:masterfrom
Conversation
|
@guoyunhe is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
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 skipped from review as they are similar to previous changes (1)
Walkthrough新增 Form 实例 API: Changes
Sequence Diagram(s)(无) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces the setFieldError method to the form store and instance, allowing users to manually set error messages for specific fields. The changes include updates to the documentation, interface definitions, and a new unit test. Feedback suggests that setFieldError should also mark the field as 'touched' to ensure the error is rendered by UI components that depend on the touched state.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/hooks/useForm.ts (1)
821-828: 实现简洁,但建议明确是否需要清除 warnings。当前实现仅透传
errors,通过setFields走已有的 observer/watch 流程,逻辑正确。不过注意与
setFieldValue(Line 809-819)对比:setFieldValue会显式清空errors: []和warnings: [],语义明确为"重置状态"。而setFieldError只设置 errors,不会清除同一字段上已存在的 warnings——这在大多数场景下是合理的(用户只想更新错误),但如果用户期望setFieldError(name, [])能"清空该字段所有校验状态",当前行为可能与预期不一致。建议在 README / JSDoc 中明确说明该方法仅影响
errors,不影响warnings、touched、validating等其他字段元数据,避免使用者误解。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useForm.ts` around lines 821 - 828, The setFieldError method only sets the errors array via setFields but does not clear other field metadata (warnings, touched, validating), which can surprise callers expecting setFieldError(name, []) to reset all validation state; update the documentation/JSDoc for the setFieldError function to explicitly state that it only affects the errors property and does not touch warnings, touched, validating, etc., and mention the contrasting behavior of setFieldValue (which clears errors and warnings) so users understand the difference and avoid misusing setFieldError.README.md (1)
143-143: 文档描述建议微调。"Set a field error list" 读起来略生硬,且未体现"按字段名设置"这一关键语义。建议改为更贴近其他条目风格的描述。
✏️ 建议的文案调整
-| setFieldError | Set a field error list | (name: [NamePath](`#namepath`), errors: string[]) => void | +| setFieldError | Set error messages of a field | (name: [NamePath](`#namepath`), errors: string[]) => void |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 143, Update the README entry for the setFieldError API so the description clearly conveys "set errors by field name" and matches the tone of other entries; replace "Set a field error list" with a concise phrase like "Set field error list by field name" (or the equivalent Chinese suggestion "按字段名设置字段错误列表") and ensure the signature line with setFieldError (name: [NamePath], errors: string[]) => void remains unchanged.tests/index.test.tsx (1)
916-940: 测试覆盖基础场景,建议补充边界用例。当前用例验证了"设置错误"与"清空错误"两条路径,足以覆盖主流程。可考虑补充以下场景以提高健壮性:
- 嵌套
NamePath(如['user', 'name'])下的setFieldError。- 多条错误信息(如
['err1', 'err2'])的设置与读取。- 调用
setFieldError后通过 UI 断言错误能被渲染出来(可参考setFields用例中的matchError),确保 observer 通知链路也被覆盖。- 验证
setFieldError不会意外修改value/touched/warnings。📝 建议追加的断言示例
expect(formRef.current.getFieldError('username')).toEqual(['Set single error']); + // setFieldError should not affect value or touched state + expect(formRef.current.getFieldValue('username')).toEqual('light'); + expect(formRef.current.isFieldTouched('username')).toBeFalsy();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/index.test.tsx` around lines 916 - 940, Add new tests that cover edge cases for setFieldError: (1) a test using a nested NamePath (e.g., formRef.current.setFieldError(['user','name'], ['nested error']) with a Field name of ['user','name']) and assert getFieldError(['user','name']) returns the error; (2) a test setting multiple errors (e.g., ['err1','err2']) and asserting getFieldError(...) returns both; (3) a test that after calling setFieldError the UI renders the error (use render with Field/Input and assert error text appears, similar to the setFields/matchError pattern); and (4) assertions that setFieldError does not mutate other field state by checking getFieldValue, getFieldTouched (or isFieldTouched), and any warnings/getFieldWarning for the same NamePath remain unchanged. Ensure tests use the same Form, Field, FormRef, setFieldError and getFieldError helpers as in the existing suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Line 143: Update the README entry for the setFieldError API so the description
clearly conveys "set errors by field name" and matches the tone of other
entries; replace "Set a field error list" with a concise phrase like "Set field
error list by field name" (or the equivalent Chinese suggestion "按字段名设置字段错误列表")
and ensure the signature line with setFieldError (name: [NamePath], errors:
string[]) => void remains unchanged.
In `@src/hooks/useForm.ts`:
- Around line 821-828: The setFieldError method only sets the errors array via
setFields but does not clear other field metadata (warnings, touched,
validating), which can surprise callers expecting setFieldError(name, []) to
reset all validation state; update the documentation/JSDoc for the setFieldError
function to explicitly state that it only affects the errors property and does
not touch warnings, touched, validating, etc., and mention the contrasting
behavior of setFieldValue (which clears errors and warnings) so users understand
the difference and avoid misusing setFieldError.
In `@tests/index.test.tsx`:
- Around line 916-940: Add new tests that cover edge cases for setFieldError:
(1) a test using a nested NamePath (e.g.,
formRef.current.setFieldError(['user','name'], ['nested error']) with a Field
name of ['user','name']) and assert getFieldError(['user','name']) returns the
error; (2) a test setting multiple errors (e.g., ['err1','err2']) and asserting
getFieldError(...) returns both; (3) a test that after calling setFieldError the
UI renders the error (use render with Field/Input and assert error text appears,
similar to the setFields/matchError pattern); and (4) assertions that
setFieldError does not mutate other field state by checking getFieldValue,
getFieldTouched (or isFieldTouched), and any warnings/getFieldWarning for the
same NamePath remain unchanged. Ensure tests use the same Form, Field, FormRef,
setFieldError and getFieldError helpers as in the existing suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5675eaad-7107-48e4-a9d1-eb73f8a982fe
📒 Files selected for processing (4)
README.mdsrc/hooks/useForm.tssrc/interface.tstests/index.test.tsx
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Summary by CodeRabbit
新功能
测试
文档