-
-
Notifications
You must be signed in to change notification settings - Fork 280
Allow passing ReactElement to errors/warnings and validation messages #767
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?
Allow passing ReactElement to errors/warnings and validation messages #767
Conversation
|
@Komoszek 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引入公开类型 Changes
Sequence Diagram(s)sequenceDiagram
participant App as 应用
participant Form as FormStore
participant Field as Field
participant Rule as ValidatorRule
rect rgb(230,245,255)
Note over App,Form: 用户触发验证/提交
App->>Form: validateFields()
Form->>Field: validateRules()
Field->>Rule: run validators
Rule-->>Field: 返回 RuleError { errors: Message[] }
Field-->>Form: resolve validatePromise with RuleError[]
end
rect rgb(235,255,230)
Note over Form,Field: 汇总并通知
Form->>Form: merge FieldError.errors (Message[])
Form->>App: notify onFieldsChange / onFinish with Message[]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 分钟
Possibly related PRs
Suggested reviewers
走过一遍该拉取请求引入了新的 诗歌
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/useForm.ts (2)
🔇 Additional comments (6)
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 @Komoszek, 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 significantly enhances the flexibility of message handling within the form validation system by allowing React elements to be used directly in error, warning, and general validation messages. This change involves a comprehensive update to the type definitions across the codebase, ensuring that the API correctly reflects this new capability and provides a more consistent developer experience. 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
|
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.
Code Review
This pull request does a great job of enabling ReactElement for error and warning messages, which significantly improves the flexibility of displaying validation feedback. The introduction of the Message type alias and its consistent application across the codebase is well-executed. The type definitions have been updated thoroughly in src/interface.ts, and the component logic in src/Field.tsx and src/useForm.ts has been adjusted accordingly.
I've found one minor issue where a type annotation was missed during the refactoring, which I've pointed out in a specific comment. Addressing this will ensure full type safety and consistency.
Overall, this is a valuable enhancement to the library.
| const mergedErrors: Message[] = []; | ||
| const mergedWarnings: Message[] = []; |
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.
While updating these types to Message[] is correct, a related type annotation in this file was missed and should also be updated to prevent potential runtime errors and ensure type consistency.
In the catch block for returnPromise within the validateFields method (around line 1025), the errors property in the type for the results parameter is still string[]. It should be Message[].
Suggested Change:
In src/useForm.ts around line 1025, please update the type signature:
// before
.catch((results: { name: InternalNamePath; errors: string[] }[]) => {
// after
.catch((results: { name: InternalNamePath; errors: Message[] }[]) => {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.
Fixed that by using FieldError
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/useForm.ts (1)
970-1001: 将外层 catch 的类型声明从string[]改为Message[]经验证,您的指出是正确的。类型链路中确实存在不一致:
- 内层 catch 处理 (第974-998行):
RuleError.errors定义为Message[],经过合并后的mergedErrors类型为Message[],通过Promise.reject()传递出去- 外层 catch 处理 (第1025行):错误声明为
errors: string[],但实际接收的是Message[]- ValidateErrorEntity (src/interface.ts:91):其
errorFields字段定义为{ name: InternalNamePath; errors: Message[] }[],与外层声明的string[]不符第1027行
errorList[0]?.errors?.[0]实际会得到Message(可为ReactElement),不仅仅是字符串。第1031行将errorList作为errorFields传入时,类型标注的错误会隐藏运行时的潜在问题。建议按原建议修改第1025行的类型注解:
- .catch((results: { name: InternalNamePath; errors: string[] }[]) => { + .catch((results: { name: InternalNamePath; errors: Message[] }[]) => {这样可以保证整个验证流程的类型一致性。
🧹 Nitpick comments (1)
src/interface.ts (1)
12-13:Message类型引入与全局替换基本一致,但属于 TS 公共 API 的类型变更优点与一致性:
- 新增
Message = string | ReactElement并在以下位置统一替换为Message[]:
Meta.errors/warningsFieldError.errors/warningsRuleError.errorsValidateErrorEntity.errorFields[].errorsFieldEntity.getErrors/getWarnings以及FormInstance.getFieldError/getFieldWarningValidatorRule.message、BaseRule.message、ValidateErrorEntity.message改为Message,再加上扩展后的ValidateMessage = string | (() => string) | ReactElement,整体上把“错误值”和“配置化消息”清晰区分开了。ValidateMessages增加了enum?: ValidateMessage,能匹配示例里对enum消息的配置。需要注意的地方:
- 由于
FormInstance.getFieldError/getFieldWarning、ValidateErrorEntity.errorFields等对外暴露的类型从string[]变为Message[],这对 TypeScript 使用者是一个显式的 breaking change(例如泛型约束里写死string[]的代码会编译失败)。建议在版本号与变更日志中明确标注这一点,方便下游升级。- 目前
Validator回调仍然声明为(error?: string) => void,这意味着如果用户希望在自定义validator中直接返回 ReactElement,需要绕过当前类型。考虑到本 PR 目标主要是让 message / validateMessages 支持 ReactElement,这点可以作为后续增强来评估。总体来说,这个文件里对
Message的引入和使用是一致且合理的,只需在发布时给下游明确升级预期即可。Also applies to: 17-18, 57-69, 91-96, 98-137, 270-292, 319-366
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/examples/validate-perf.tsx(2 hunks)src/Field.tsx(5 hunks)src/interface.ts(8 hunks)src/useForm.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
docs/examples/validate-perf.tsx (1)
src/interface.ts (1)
Message(12-12)
src/useForm.ts (2)
src/interface.ts (2)
NamePath(7-7)Message(12-12)src/utils/valueUtil.ts (1)
getNamePath(15-17)
src/Field.tsx (1)
src/interface.ts (2)
RuleError(135-138)Message(12-12)
🔇 Additional comments (3)
docs/examples/validate-perf.tsx (1)
7-7: 示例中 Message 引入与 onPasswordError 类型更新是合理的这里将
Message、ValidateMessages从@/interface引入,并把onPasswordError的errors参数更新为Message[],与Meta.errors的新类型保持一致,示例代码能同时覆盖字符串和 ReactElement 错误信息,整体没有类型或运行时风险。Also applies to: 37-39
src/useForm.ts (1)
19-20: 错误 / 警告返回值改为Message[]的实现是连贯的
Message类型引入,并在getFieldError/getFieldWarning返回Message[],与FormInstance和FieldEntity.getErrors/getWarnings的新签名一致。triggerOnFieldsChange中NameMap<Message[]>的缓存类型也与FieldError的更新保持同步,避免后续再出现string[]/Message[]混用问题。这部分逻辑和类型衔接良好,看不到明显问题。
Also applies to: 364-400, 868-900
src/Field.tsx (1)
13-14: Field 内部改用Message[]与新的错误模型一致
- 引入
Message,并将errors/warnings字段改为Message[],与RuleError.errors: Message[]和Meta.errors/warnings: Message[]的定义对齐。EMPTY_ERRORS/EMPTY_WARNINGS使用never[]作为共享空数组哨兵,在 TS 类型层面可以避免误修改,同时又能安全赋值给Message[]或RuleError[]。validatePromise与validateRules返回类型统一为Promise<RuleError[]>,validateOnly分支也保持一致。validateRules中通过warningOnly拆分到nextErrors/nextWarnings,最终写回this.errors/this.warnings,可同时承载字符串和 ReactElement。整体来看,Field 这一侧对新
Message类型的接入是自洽的,没有发现明显类型或运行时问题。Also applies to: 33-35, 141-147, 395-481, 503-523
|
@zombieJ what do you think? |
Currently
ReactElementcan only be passed tomessagein therulespassed to theField. This PR fixes types a bit, which in turn means thatReactElementcan also be passed to warnings/errors/validation messages andgetFieldError/getFieldsError/getFieldWarninghave correct return types.Summary by CodeRabbit
发布说明
新功能
重构
✏️ Tip: You can customize this high-level summary in your review settings.