-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: Form校验增强 #812
feat: Form校验增强 #812
Conversation
Walkthrough此拉取请求的更改主要集中在表单组件及其相关类型的增强上。新增了可选属性 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Form
participant Field
User->>Form: 填写表单
Form->>Field: 获取字段ID
Field->>Form: 返回字段ID
Form->>User: 提交表单
User->>Form: 验证字段
Form->>Field: 滚动到字段
Possibly related PRs
Suggested labels
Warning Rate limit exceeded@saint3347 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 18
🧹 Outside diff range and nitpick comments (35)
packages/hooks/src/components/use-tabs/context.tsx (2)
5-5
: 建议:考虑使用更具体的类型替代any
虽然为 Context 提供默认值是个好习惯,但使用
any
类型会降低类型安全性。建议定义一个默认的泛型类型参数。-export const TabsContext = React.createContext<TabsContextProps<any>>({ tabs: [], setTabs: () => {} }); +export const TabsContext = React.createContext<TabsContextProps<unknown>>({ tabs: [], setTabs: () => {} });
12-12
: 建议:增加类型检查以提高安全性当前实现使用了类型断言,建议添加运行时类型检查以确保类型安全。
-export const useTabsContext = <TabData,>() => React.useContext(TabsContext) as TabsContextProps<TabData>; +export const useTabsContext = <TabData,>() => { + const context = React.useContext(TabsContext); + if (!context) { + throw new Error('useTabsContext 必须在 TabsProvider 内使用'); + } + return context as TabsContextProps<TabData>; +};packages/hooks/src/components/use-tabs/use-tabs.ts (2)
5-5
: 建议改进类型安全性泛型参数
TabData
默认为any
类型可能过于宽松。建议定义一个基础接口作为默认类型,以提供更好的类型安全性。-const useTabs = <TabData = any>(props: BaseTabsProps) => { +interface BaseTabData { + key: string | number; + title: string; +} +const useTabs = <TabData extends BaseTabData = BaseTabData>(props: BaseTabsProps) => {
25-26
: 请补充新属性的文档说明新增的
tabs
和setTabs
属性缺少使用说明文档。建议在代码中添加 JSDoc 注释,说明这些属性的用途和使用方法。return { active: getActive(), onChange: handleChange, + /** 当前标签页数据列表 */ tabs, + /** 更新标签页数据的方法 */ setTabs, Provider, };packages/base/src/common/use-with-form-config.ts (1)
5-7
: 类型断言和变量声明的改进建议代码的类型安全性有所提升,但建议进一步优化类型断言的使用方式。
建议将重复的类型断言提取为一个变量,以提高代码的可维护性:
export const useWithFormConfig = <T>(props: T) => { const formConfig = useFormConfig(); + const baseProps = props as BaseFormProps<any>; - const size: BaseFormProps<any>['size'] = (props as BaseFormProps<any>)?.size || formConfig.size; - const disabled: boolean | undefined = formConfig.disabled || (props as BaseFormProps<any>)?.disabled; - const reserveAble: boolean | undefined = (props as BaseFormProps<any>)?.reserveAble ?? formConfig.reserveAble; + const size: BaseFormProps<any>['size'] = baseProps?.size || formConfig.size; + const disabled: boolean | undefined = formConfig.disabled || baseProps?.disabled; + const reserveAble: boolean | undefined = baseProps?.reserveAble ?? formConfig.reserveAble;packages/hooks/src/components/use-tabs/context.type.ts (1)
37-48
: 建议完善新属性的文档说明新增的
tabs
和setTabs
属性虽然标记为内部属性,但建议:
- 补充说明
tabs
数组中元素的数据结构- 为
setTabs
添加使用示例建议添加如下文档注释:
/** * @private 内部属性 + * @description tabs数组用于存储标签页的数据 + * @type {TabData[]} TabData 为标签页数据类型 */ tabs: TabData[]; /** * @private 内部属性 + * @description 用于更新标签页数据的状态更新函数 + * @example + * setTabs([...tabs, newTab]); + * setTabs(prev => [...prev, newTab]); */ setTabs: React.Dispatch<React.SetStateAction<TabData[]>>packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts (1)
6-9
: 建议补充更详细的属性文档说明虽然已经标记了 @Private 和简单说明,但建议补充以下信息:
- 属性的具体用途和使用场景
- 与表单验证的关联关系
- 可能的取值范围
建议按如下方式扩展文档:
/** * @private 内部属性 for validate + * @description 用于表单验证时唯一标识表单实例,在多表单场景下特别有用 + * @example formName="login-form" */ formName?: string;packages/base/src/form/form-field.tsx (1)
44-44
: 建议为 id 生成逻辑添加注释说明id 属性的添加有助于提升表单的可访问性和字段定位能力。建议添加注释说明 id 的生成规则,以便其他开发者理解和维护。
建议添加如下注释:
+ // 优先使用已存在的 id,否则基于字段名和表单名生成唯一标识 id: childrenProps.id || util.getFieldId(formControl.name, props.formName),
packages/base/src/form/form-field.type.ts (1)
10-10
: 建议为 id 属性添加 JSDoc 文档注释为了保持代码文档的完整性和一致性,建议添加 JSDoc 注释说明该属性的用途。
建议添加如下文档注释:
+ /** + * @en Unique identifier for the form field + * @cn 表单字段的唯一标识符 + */ id?: string;packages/shineout/src/tabs/__doc__/changelog.cn.md (1)
4-5
: 功能增强描述清晰且完整功能增强的描述准确反映了PR的目标,并通过PR链接提供了详细信息。建议补充一下具体使用示例,以帮助用户更好地理解这个新特性。
建议添加使用示例:
### 💎 Enhancement - `Tabs` 支持渲染非 `Tabs.Panel` 子组件,例如 `Form.FieldSet` ([#812](https://github.com/sheinsight/shineout-next/pull/812)) + +示例: +```jsx +<Tabs> + <Tabs.Panel tab="Tab 1"> + 内容1 + </Tabs.Panel> + <Form.FieldSet> + 自定义表单内容 + </Form.FieldSet> +</Tabs> +```packages/shineout/src/hooks/use-field-common.tsx (2)
Line range hint
36-40
: 建议添加泛型参数的文档注释为了提高代码可维护性,建议为泛型参数添加 TSDoc 注释,说明每个类型参数的用途:
Props
: 继承自 FieldItemCommonProps 的属性类型Value
: 字段值的类型Name
: 字段名称的类型建议添加如下注释:
const useFieldCommon = < + /** + * @template Props - 继承自 FieldItemCommonProps 的属性类型 + * @template Value - 字段值的类型 + * @template Name - 字段名称的类型,默认为 string + */ Props extends FieldItemCommonProps, Value, Name extends string | string[] = string, >
26-26
: 建议优化联合类型中的 void 使用静态分析工具提示在联合类型中使用
void
可能造成混淆。建议使用undefined
来提高代码清晰度。建议修改如下:
- beforeChange?: (value: T) => T | undefined | void; + beforeChange?: (value: T) => T | undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
packages/base/src/form/form.tsx (2)
37-42
: 建议优化错误处理逻辑当前的错误处理直接抛出原始错误,建议添加更多的错误上下文信息,以便于调试和错误追踪。
建议如下修改:
const validate = usePersistFn((fields?: keyof V | Array<keyof V>) => { return formFunc.validateFields(fields as string | string[]) .catch((error: ValidationError<V>) => { - // 这里可以选择处理错误,或者直接抛出 - throw error; + // 增加错误上下文 + throw { + ...error, + context: { + fields, + timestamp: new Date().toISOString(), + } + }; });; });
Line range hint
37-56
: 建议添加单元测试对于新增的表单验证功能和 scrollToField 方法,建议添加相应的单元测试用例,确保功能的正确性和稳定性。
需要我帮您生成相关的测试用例代码吗?
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
packages/base/src/form/form-item.tsx (1)
11-11
: 导出声明的改进是好的,但组件可以进一步优化代码改动将匿名函数改为命名函数声明并移动导出语句到文件末尾,这些变更符合最佳实践。但是当前组件较大且职责较多,建议考虑拆分成更小的子组件以提高可维护性。
建议将以下逻辑拆分为独立组件:
// 新建 FormItemLabel.tsx + const FormItemLabel = ({ label, tooltip, ...props }) => { + // 移动 renderLabel 相关逻辑 + } // 新建 FormItemError.tsx + const FormItemError = ({ errors, tip }) => { + // 移动错误和提示相关逻辑 + } // form-item.tsx const FormItem = (props) => { // 主要保留布局相关逻辑 return ( <div className={...}> - {renderLabel()} + <FormItemLabel {...labelProps} /> <div className={formItemClasses?.control}> <Provider value={ProviderValue}>{children}</Provider> - {showError && ...} - {!!tip && ...} + <FormItemError errors={errors} tip={tip} /> </div> </div> ); }Also applies to: 104-105
packages/shineout/src/form/__doc__/changelog.cn.md (2)
1-6
: 建议增加使用示例和详细说明
validate
方法的增强是一个重要更新,建议补充:
- 返回的校验信息具体包含哪些内容
- 使用示例代码
- 与旧版本的对比说明
建议按照以下格式补充文档:
- 增强 `Form` 的 `validate` 方法,校验成功或失败后返回更多的校验信息 ([#812](https://github.com/sheinsight/shineout-next/pull/812)) + + ```typescript + // 新版本返回格式 + { + errors: { field1: ['错误信息'], field2: ['错误信息'] }, + success: boolean, + // 其他返回信息... + } + ```
1-14
: 建议优化版本号管理和变更记录组织当前的版本号管理存在以下问题:
- beta 版本号跳跃(从 beta.3 直接到 beta.6)
- 缺少对中间版本的说明
建议:
- 补充说明中间版本(beta.4、beta.5)的变更记录或标注为跳过
- 在每个版本发布时添加完整的发布日期
- 考虑添加一个"Breaking Changes"章节,明确标注不向下兼容的改动
packages/shineout/src/form/__example__/002-control.tsx (2)
47-52
: 建议改进验证错误处理方式当前实现使用console.log记录验证结果,这在生产环境中可能不是最佳实践。建议考虑以下改进:
- 使用统一的错误处理机制
- 添加用户友好的错误提示
建议修改为:
form.current?.validate() .then((values) => { - console.log('validate success', values); + // 使用统一的成功提示组件 + Message.success('验证成功'); }) .catch((errorInfo) => { - console.log('validate failed errorInfo: >>', errorInfo) + // 使用统一的错误提示组件 + Message.error('验证失败:' + errorInfo.message); })
85-87
: 代码格式优化建议建议删除多余的空行,保持代码整洁。
packages/hooks/src/utils/dom/element.tsx (1)
154-155
: 函数实现基本正确,建议增加输入验证函数实现简洁明了,能够满足生成唯一字段ID的基本需求。不过建议增加以下改进:
- 对必填参数
name
进行非空验证- 处理特殊字符,确保生成的ID符合DOM规范
- 考虑添加类型约束,确保输入参数的类型安全
建议按照以下方式改进实现:
-export const getFieldId = (name: string, formName?: string) => `${formName ? `${formName}_` : ''}${name}`; +export const getFieldId = (name: string, formName?: string): string => { + if (!name) { + throw new Error('Field name is required'); + } + // 移除特殊字符,仅保留字母、数字、下划线和连字符 + const sanitizeName = (str: string) => str.replace(/[^a-zA-Z0-9_-]/g, '_'); + return `${formName ? `${sanitizeName(formName)}_` : ''}${sanitizeName(name)}`; +};packages/shineout/src/form/__example__/006-validate-05-multi-form.tsx (3)
10-13
: 建议优化 Brand 类型定义以提高类型安全性当前的类型定义使用了
[key: string]: any
,这可能会降低类型安全性。建议明确列出所有允许的属性。type Brand = { name: string; - [key: string]: any; + desc: string; + // 添加其他明确的属性 };
39-39
: 移除未使用的 useEffect空的 useEffect 钩子没有实际用途,建议移除以保持代码整洁。
- useEffect(() => {}, []);
92-107
: 建议移除注释掉的代码保留注释掉的代码会影响代码可读性,如果不再需要建议直接删除。
packages/base/src/tabs/tabs.tsx (2)
173-173
: 建议优化代码结构空行的使用显得多余,建议移除以保持代码整洁。
}); const header = (
241-249
: Provider 类型和状态管理的改进Provider 的实现已更新以支持:
- 使用泛型类型
TabData
增强类型安全- 新增
tabs
和setTabs
属性用于状态管理建议添加相关的类型注释文档,说明这些新属性的用途。
+// 为新增的属性添加 JSDoc 注释 +/** + * @property {TabData[]} tabs - 标签页数据数组 + * @property {(tabs: TabData[]) => void} setTabs - 更新标签页数据的方法 + */ <Provider<TabData> value={{ lazy,packages/hooks/src/components/use-form/use-form-control/use-form-control.ts (1)
201-201
: 建议增强错误处理机制当前实现在非表单模式下直接传递空字符串作为 name 参数:
validateField('', v, undefined)
。建议传递一个有意义的标识符,以便于调试和错误追踪。-validateField('', v, undefined); +validateField('standalone', v, undefined);packages/shineout/src/form/__example__/006-validate-05-scrolltoerror.tsx (2)
87-89
: 避免使用 display: none 隐藏表单字段使用
display: none
隐藏表单字段可能会导致:
- 屏幕阅读器无法访问
- 影响表单的可访问性
建议使用
Form.Item
的hidden
属性或条件渲染来控制字段的显示:-<div style={{display: 'none'}} id="root-field2"> - <Input name="root-field2" defaultValue='root-field2' /> -</div> +<Form.Item hidden={true}> + <Input name="root-field2" defaultValue='root-field2' /> +</Form.Item>
90-95
: 建议对验证消息进行国际化处理当前验证消息使用英文硬编码,建议:
- 使用统一的国际化方案
- 提供中文的错误提示
建议改进为:
<Select keygen name='ruleEnumList' data={[1, 2, 3]} - rules={[{ required: true, message: 'ruleEnumList is required' }]} + rules={[{ required: true, message: '请选择规则枚举列表' }]} />packages/shineout/src/form/__example__/temp/test-form-value.ts (1)
1-454
: 注意测试数据的安全性当前测试数据可能存在安全风险:
- 包含疑似真实的用户信息(姓名、邮箱等)
- 包含社交媒体账号信息
- 包含详细的用户档案信息
建议采用以下安全最佳实践:
- 使用数据生成工具(如 Faker)创建虚构的测试数据
- 建立测试数据脱敏规范
- 将敏感配置移至环境变量或配置文件
- 添加
.gitignore
规则,避免意外提交敏感信息packages/shineout-style/src/tabs/tabs.ts (1)
Line range hint
1-824
: 建议优化样式架构代码结构良好,但建议考虑以下优化:
- 考虑将重复的样式模式(如hover和active状态)抽象为可重用的mixin函数
- 建议将所有硬编码的颜色值(如
#FFFFFF
)迁移到 Token 系统中这些改进将提高代码的可维护性和一致性。
示例重构:
// 创建可重用的mixin const createHoverActiveMixin = (hoverBg: string, activeBg: string) => ({ '&:not([data-soui-state="active"]):not([data-soui-state="disabled"])': { '&:hover': { background: hoverBg, }, '&:active': { background: activeBg, }, }, }); // 使用Token替换硬编码值 const getCardStyle = () => ({ // ... '&[data-soui-state="active"]': { '& $fillInner': { background: Token.tabsWhiteBackground, // 替代 #FFFFFF color: Token.tabsActiveFontColor, }, }, // ... });packages/base/src/form/form.type.ts (1)
12-22
: 建议简化泛型参数以提高代码可读性
FormValidateFn
接口的泛型参数较多,可能增加了理解难度。考虑简化泛型参数,或为其添加更具描述性的名称,以提高代码的可读性和可维护性。packages/hooks/src/components/use-form/use-form.type.ts (2)
125-131
: 注意 formName 属性与 name 属性的潜在混淆在
FormCommonConfig
中新增了formName?: string
属性,而在BaseFormProps<T>
中新增了name?: string
属性。两个属性都用于作为表单字段 id 的前缀,但名称不同,可能导致混淆。建议统一属性名称,或者在文档中明确区分两者的用途。
217-221
: 统一 name 属性的使用,避免命名冲突
BaseFormProps<T>
中添加了name?: string
属性,应确保与FormCommonConfig
中的formName
属性一致,以避免混淆。建议考虑统一使用formName
或name
,以提高代码的可读性和维护性。packages/hooks/src/components/use-form/use-form.ts (1)
219-227
: 建议在 scrollToField 方法中使用平滑滚动在调用
fieldEl.scrollIntoView()
时,可以添加参数{ behavior: 'smooth' }
,使滚动更加平滑,提升用户体验。可以修改为:
- fieldEl.scrollIntoView(); + fieldEl.scrollIntoView({ behavior: 'smooth' });packages/base/src/tabs/tabs-panel.tsx (1)
14-15
: 变量命名拼写错误
keekAlive
应为keepAlive
。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (29)
package.json
(1 hunks)packages/base/src/common/type.ts
(1 hunks)packages/base/src/common/use-with-form-config.ts
(1 hunks)packages/base/src/form/form-field.tsx
(1 hunks)packages/base/src/form/form-field.type.ts
(1 hunks)packages/base/src/form/form-item.tsx
(2 hunks)packages/base/src/form/form.tsx
(4 hunks)packages/base/src/form/form.type.ts
(2 hunks)packages/base/src/input/input.tsx
(1 hunks)packages/base/src/input/input.type.ts
(2 hunks)packages/base/src/tabs/tabs-panel.tsx
(1 hunks)packages/base/src/tabs/tabs.tsx
(4 hunks)packages/hooks/src/components/use-form/use-form-context.type.ts
(2 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.ts
(3 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts
(1 hunks)packages/hooks/src/components/use-form/use-form.ts
(11 hunks)packages/hooks/src/components/use-form/use-form.type.ts
(4 hunks)packages/hooks/src/components/use-tabs/context.tsx
(1 hunks)packages/hooks/src/components/use-tabs/context.type.ts
(2 hunks)packages/hooks/src/components/use-tabs/use-tabs.ts
(2 hunks)packages/hooks/src/utils/dom/element.tsx
(1 hunks)packages/shineout-style/src/tabs/tabs.ts
(1 hunks)packages/shineout/src/form/__doc__/changelog.cn.md
(1 hunks)packages/shineout/src/form/__example__/002-control.tsx
(3 hunks)packages/shineout/src/form/__example__/006-validate-05-multi-form.tsx
(1 hunks)packages/shineout/src/form/__example__/006-validate-05-scrolltoerror.tsx
(1 hunks)packages/shineout/src/form/__example__/temp/test-form-value.ts
(1 hunks)packages/shineout/src/hooks/use-field-common.tsx
(3 hunks)packages/shineout/src/tabs/__doc__/changelog.cn.md
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/base/src/form/form.tsx
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
packages/hooks/src/components/use-form/use-form.ts
[error] 182-182: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/shineout/src/hooks/use-field-common.tsx
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (36)
packages/hooks/src/components/use-tabs/context.tsx (1)
7-10
: 代码实现清晰,符合最佳实践
Provider 组件的实现简洁明了,泛型类型的使用恰当,能够保证类型安全性。
packages/base/src/common/use-with-form-config.ts (1)
11-15
: 属性展开逻辑的优化
条件属性展开的实现很好,可以确保只有定义过的值才会被包含在返回对象中。建议验证 formName
在整个表单系统中的使用情况。
运行以下脚本来验证 formName
的使用情况:
✅ Verification successful
formName
属性的使用验证已完成
根据代码分析,formName
属性在表单系统中有完整且一致的实现:
- 在
useForm
中作为表单标识符使用 - 通过
getFieldId
用于生成表单字段的唯一ID - 在
FormControl
类型定义中正确声明 - 在表单组件中合理传递和处理
当前的条件展开实现符合系统设计,不需要额外修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查 formName 在代码库中的使用情况
# 搜索 formName 的使用位置
rg "formName" --type ts
# 检查是否有其他表单相关组件使用了这个属性
ast-grep --pattern 'interface $_ {
$$$
formName: $_
$$$
}'
Length of output: 1272
Script:
#!/bin/bash
# 检查 formName 的具体实现和使用场景
# 检查 useForm 中的 formName 实现
ast-grep --pattern 'useForm = ($$$) => {
$$$
formName
$$$
}'
# 检查 FormControl 相关接口定义
ast-grep --pattern 'interface FormControl {
$$$
}'
# 检查 getFieldId 的完整实现
ast-grep --pattern 'getFieldId = ($$$) => {
$$$
}'
Length of output: 178
Script:
#!/bin/bash
# 使用更精确的搜索来查看 formName 的使用上下文
# 查看 useForm 的实现
rg "useForm.*=.*{" -A 10 --type ts
# 查看 FormControl 接口定义
rg "interface.*FormControl.*{" -A 10 --type ts
# 查看 getFieldId 的实现
rg "getFieldId.*=.*{" -A 3 --type ts
# 检查 formName 在表单组件中的使用
rg "formName" -B 2 -A 2 --type ts
Length of output: 5028
packages/hooks/src/components/use-tabs/context.type.ts (2)
3-3
: 泛型参数的添加增强了类型安全性!
通过添加 TabData
泛型参数,接口变得更加灵活且类型安全。这是一个很好的改进。
51-52
: Provider 接口的泛型更新符合预期!
TabsProviderProps
接口使用相同的 TabData
泛型参数,确保了类型安全性的传递,这是一个合理的改进。
packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts (1)
6-9
: 新增的 formName 属性实现合理
属性定义清晰,使用可选类型标记很恰当,并且通过 @Private 标记明确了其内部使用性质。
packages/hooks/src/components/use-form/use-form-context.type.ts (2)
11-11
: 类型安全性提升和功能增强
validate 方法的签名更改提供了更好的类型安全性和灵活性:
- 使用泛型 Value 替代 any 类型
- 新增可选参数支持验证特定字段
运行以下脚本验证使用此接口的组件是否正确实现了新的方法签名:
✅ Verification successful
validate 方法签名变更已得到正确实现
根据代码搜索结果,validate 方法的新签名已在以下位置得到正确实现和使用:
- 在 FormRef 接口中定义了相应的方法
- 实际使用场景中支持了可选的字段参数,如
validate(['request.commonPartnerUpsertReq.labelRuleInfo[0].ruleEnumList'])
- 与 validateFields 和 validateFieldsWithError 方法形成完整的表单验证体系
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 validate 方法的实现
ast-grep --pattern 'validate(fields?: string | string[]): Promise<$_> {
$$$
}'
Length of output: 82
Script:
#!/bin/bash
# 搜索包含 validate 方法的实现
rg "validate.*\(.*\).*Promise" -A 5
# 搜索使用 validate 方法的地方
rg "\.validate\(" -A 2
# 搜索相关的测试文件
fd "use-form.*test" --exec cat {}
Length of output: 17006
42-46
: 新增滚动到指定字段功能
scrollToField 方法的接口定义清晰,文档完整。建议验证实现细节以确保良好的用户体验。
运行以下脚本检查方法实现:
✅ Verification successful
scrollToField 方法实现已验证完成
scrollToField 方法已在 packages/hooks/src/components/use-form/use-form.ts
中实现,具体实现如下:
- 使用
getFieldId
获取字段的 DOM ID - 通过
document.getElementById
查找对应元素 - 使用标准的
scrollIntoView()
方法实现滚动
建议:
- 考虑添加滚动行为配置选项,如平滑滚动
- 当字段不存在时,可以添加错误提示
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 scrollToField 方法的实现
ast-grep --pattern 'scrollToField(name: string): void {
$$$
}'
Length of output: 66
Script:
#!/bin/bash
# Search for scrollToField implementation
rg -g '*.{ts,tsx}' 'scrollToField' -A 5
Length of output: 2518
Script:
#!/bin/bash
# Search for scrollToField type definition and usage
ast-grep --pattern 'scrollToField: $_'
Length of output: 40
Script:
#!/bin/bash
# Find files containing scrollToField
fd -e ts -e tsx -x grep -l "scrollToField"
Length of output: 308
packages/base/src/common/type.ts (2)
9-13
: 新增的 name 属性定义合理且文档完善
类型定义和文档注释清晰地表达了该属性的用途,使其能够被 Form 组件用于控制表单控件的值。可选属性的设计也很合理,保持了向后兼容性。
9-13
: 建议验证 name 属性在各个表单控件中的实现情况
由于 CommonType 被多个表单控件使用,建议确认所有相关组件都正确处理了这个新属性。
✅ Verification successful
Let me analyze the usage of name
property in form controls by searching for its implementation.
Let me search for the actual usage of name property in form controls.
根据代码分析,我可以得出以下结论:
name 属性在表单控件中已得到广泛且正确的实现
-
所有表单控件(Input、Select、Radio、Checkbox等)都正确支持了 name 属性
-
name 属性与 Form 组件紧密集成,用于:
- 表单数据收集和提交
- 表单验证规则绑定
- 表单控件值的获取和设置
-
实际使用案例显示 name 属性在以下场景中都能正常工作:
- 基础表单输入
- 嵌套表单字段(使用 Form.FieldSet)
- 数组形式的表单数据
- 表单验证
- 表单联动
-
相关组件的类型定义和实现都正确包含了 name 属性的处理
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查所有使用 CommonType 的组件是否正确处理了 name 属性
# 搜索所有引用了 CommonType 的组件
rg -l "CommonType" packages/
# 检查这些组件中是否有处理 name 属性的逻辑
rg "name.*?:" packages/ -g "*.{tsx,ts}"
Length of output: 65141
Script:
#!/bin/bash
# Search for form control components that use name property
ast-grep --pattern 'interface $_ {
$$$
name?: string;
$$$
}'
# Search for form control components that handle name prop
ast-grep --pattern 'const $_ = ($props: $_Props) => {
$$$
name = $props.name,
$$$
}'
Length of output: 8278
Script:
#!/bin/bash
# Search for form control components that use name prop
rg "name={.*?}" packages/ -g "*.tsx" -A 2
# Search for form control implementations that handle name prop
rg "const.*?name.*?=.*?props\.name" packages/ -g "*.{tsx,ts}" -A 2
Length of output: 211
Script:
#!/bin/bash
# Search for form control components that use name prop
rg 'name=' packages/ -g "*.tsx" -A 2
# Search for form control implementations that handle name prop in hooks
rg 'useField.*?\{.*?name' packages/ -g "*.{tsx,ts}" -A 5
# Search for name prop usage in form components
rg '<(Input|Select|Radio|Checkbox).*?name=' packages/ -g "*.tsx" -A 2
Length of output: 101011
packages/base/src/input/input.tsx (1)
21-23
: 修复了控制台警告问题,代码实现清晰
通过将 reserveAble
和 formName
显式设置为 undefined
,有效防止了这些属性被传递到原生 input 元素上,解决了控制台警告问题。实现方式简单直接。
建议添加测试用例验证此修复:
packages/base/src/form/form-field.tsx (1)
44-44
: 验证 getFieldId 方法的实现
需要确保 util.getFieldId 方法能够正确处理各种边界情况。
✅ Verification successful
getFieldId 方法实现已验证,实现合理且使用正确
getFieldId 方法的实现位于 packages/hooks/src/utils/dom/element.tsx
,其实现为:
export const getFieldId = (name: string, formName?: string) => `${formName ? `${formName}_` : ''}${name}`;
该实现:
- 正确处理了有无 formName 的情况
- 使用下划线作为分隔符,符合 ID 命名规范
- 返回值格式为
${formName}_${name}
或${name}
,能确保 ID 的唯一性
当前代码中的使用方式完全符合该方法的设计意图。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索 getFieldId 的实现和使用情况
echo "查找 getFieldId 的实现:"
ast-grep --pattern 'getFieldId = $_' -l typescript
echo "\n检查 getFieldId 的调用位置:"
rg "getFieldId" -A 2
Length of output: 996
packages/base/src/form/form-field.type.ts (1)
10-10
: 新增的 id 属性实现合理
可选的 id 属性增强了表单字段的标识能力,有助于实现滚动定位等功能,且保持了向后兼容性。
packages/shineout/src/tabs/__doc__/changelog.cn.md (1)
1-3
: 版本号和日期格式正确
版本号和日期的格式符合项目规范。
packages/shineout/src/hooks/use-field-common.tsx (2)
Line range hint 27-31
: 接口重命名改进了代码清晰度!
将 FiledItemCommonProps
重命名为 FieldItemCommonProps
修正了拼写错误,提高了代码质量。接口定义简洁明确,包含了字段项所需的基本属性。
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
55-55
: 验证 formName 的使用情况
新增的 formName
属性来自 formConfig
,用于增强表单验证功能。建议验证此属性在相关组件中的使用情况。
运行以下脚本以验证 formName
的使用:
✅ Verification successful
formName
属性使用验证完成
根据代码分析,formName
属性已在以下关键位置正确实现:
- 在
use-form
组件中作为表单标识符使用 - 在
getFieldId
工具函数中用于生成唯一字段ID - 在
FormField
组件中用于关联表单控件 - 通过
useWithFormConfig
钩子正确传递给表单组件
新增的 formName
属性完全符合表单验证增强的需求,实现合理且完整。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查 formName 在相关组件中的使用情况
# 搜索 formName 的使用
rg "formName" --type ts --type tsx
# 检查 FormField 组件中的实现
ast-grep --pattern 'const FormField = ({ formName, $$$props }) => {
$$$
}'
Length of output: 141
Script:
#!/bin/bash
# 搜索 formName 的使用情况,使用正确的文件类型
rg "formName" -t ts
# 检查包含 formName 的组件定义
ast-grep --pattern 'interface $_ {
formName?: $_
$$$
}'
# 检查 formName 的实际使用
ast-grep --pattern 'formName: $_'
Length of output: 1299
packages/base/src/form/form.tsx (2)
9-16
: 新增的 ValidationError 接口设计合理
接口结构清晰,包含了必要的错误信息字段,能够满足表单校验的需求。
56-56
: 新增的 scrollToField 功能符合需求
此功能增强了表单的用户体验,允许快速定位到指定字段。
packages/shineout/src/form/__example__/002-control.tsx (1)
8-16
: 导入更改和StarRate组件定义看起来不错!
组件定义简洁明了,SVG图标定义完整,Rate高阶组件的使用方式正确。
packages/base/src/input/input.type.ts (1)
63-63
: 类型扩展符合预期
将 name
属性添加到 Pick<CommonType>
中的改动是合理的,这样可以确保表单字段具有正确的类型定义,有助于提升表单验证能力。
Also applies to: 108-108
packages/base/src/tabs/tabs.tsx (2)
Line range hint 43-47
: 类型安全性增强
useTabs
hook 的使用现在包含了泛型类型参数 TabData
,这样能更好地确保类型安全。同时新增的 tabs
和 setTabs
属性使得标签页数据的管理更加灵活。
155-155
: 支持非 TabsPanel 子组件
现在可以直接渲染非 TabsPanel
类型的子组件,这个改动符合 PR 的目标,使得 Tabs
组件能够支持 Form.FieldSet
等其他类型的子组件。
packages/hooks/src/components/use-form/use-form-control/use-form-control.ts (2)
Line range hint 117-199
: 验证函数实现完善且健壮
validateField 函数的实现考虑了多种场景:
- 表单内外的验证处理
- 数组字段的验证
- 嵌套字段集的错误传播
- 强制通过的验证选项
代码结构清晰,错误处理完整。
212-212
: 验证清理逻辑的完整性
useEffect 的清理函数中使用了 validateField 进行解绑,需要确保在以下场景下不会造成内存泄漏:
- 组件卸载时字段值的状态更新
- 动态字段名变更时的解绑操作
Also applies to: 215-215, 222-222, 226-226
✅ Verification successful
清理逻辑实现完整且安全
代码分析显示清理逻辑已经正确实现:
- 组件卸载时会正确调用
unbind
清理表单控件的绑定关系 - 对数组和单个字段都进行了处理,并清除相关的错误状态
- 通过
updateError(name, undefined)
确保状态被重置 - 依赖数组
[name?.toString()]
确保字段名变更时重新绑定
清理函数会在以下场景被触发:
- 组件卸载时
- 字段名称变更时
- 表单控件状态变化时
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索相关的内存泄漏问题报告或类似实现
rg -A 3 "useEffect.*cleanup.*form"
rg -A 3 "unbind.*validate.*field"
Length of output: 72
Script:
#!/bin/bash
# 搜索 useEffect 中的清理函数实现
ast-grep --pattern 'useEffect(() => {
$$$
return () => {
$$$
};
}, $$$)'
# 搜索表单控件的解绑和清理相关代码
rg -A 5 "cleanup|unbind|dispose.*form.*control"
# 查找 validateField 相关的状态更新逻辑
ast-grep --pattern 'function $_(validateField, $$$) {
$$$
}'
Length of output: 101042
Script:
#!/bin/bash
# 搜索 useForm 中的清理和解绑相关实现
ast-grep --pattern 'useEffect(() => {
$$$
return () => {
if (inForm && controlFunc) {
$$$
}
};
}, $$$)'
# 查找 validateField 相关的状态更新和错误处理
ast-grep --pattern 'function updateError($_, $_) {
$$$
}'
# 检查 Form 组件中的内存管理
rg -A 5 "form.*control.*cleanup|form.*control.*dispose"
Length of output: 3018
packages/shineout-style/src/tabs/tabs.ts (1)
62-62
: 水平规则定位优化
通过设置 left: 'auto'
确保了在从左到右布局中水平规则的正确定位,提高了组件在不同书写方向下的布局一致性。
packages/base/src/form/form.type.ts (2)
6-6
: 类型定义正确
定义 KeyType
为 string | number | symbol
,有助于统一表单字段名称的类型,提升类型安全性。
25-25
: 确认泛型参数名称的一致性
将泛型参数从 Value
更改为 FormValue
,请确保代码库中所有引用 FormRef
的地方都同步更新,以避免类型不匹配的问题。
可以运行以下脚本检查代码库中的 FormRef
定义和引用是否一致:
✅ Verification successful
泛型参数名称变更是安全的
根据代码库搜索结果,所有使用 FormRef
的地方都使用了以下形式:
FormRef<any>
FormRef<T>
FormRef<V>
FormRef<Value>
FormRef<FormValue>
由于这些都是类型参数,参数名称的变化不会影响实际的类型匹配,因此这个改动是安全的。泛型参数名称的变更只是提高了代码可读性,不会导致类型不匹配的问题。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 查找使用旧泛型参数名称的地方
rg 'FormRef<\s*Value\s*>' -t typescript
Length of output: 76
Script:
#!/bin/bash
# 搜索所有包含 FormRef 的文件
rg "FormRef" --type ts
# 使用 ast-grep 查找 FormRef 的泛型使用
ast-grep --pattern 'FormRef<$_>'
Length of output: 14666
packages/hooks/src/components/use-form/use-form.type.ts (6)
7-8
: 添加了 KeyType 类型定义
新的 KeyType
类型别名定义为 string | number | symbol
,有助于在代码中统一键的类型。
9-16
: 定义了新的 ValidationError 接口
新增的 ValidationError<T>
接口标准化了验证错误的结构,便于在验证失败时提供详细的错误信息。
18-28
: 验证函数返回类型可能需要明确
ValidateFieldsFn
接口返回类型为 Promise<Partial<FormValue>>
。请确认在验证失败的情况下,如何传递错误信息。可能需要返回 Promise<Partial<FormValue> | ValidationError<FormValue>>
,以包含错误详情。
30-33
: 新增 ValidateFnConfig 类型,提升验证配置灵活性
引入 ValidateFnConfig
类型,为验证函数提供了额外的配置选项,如 type
和 ignoreBind
,有助于在不同场景下定制验证行为。
39-40
: 验证函数返回类型从 boolean 更改为 true
在 ValidateFn
中,将返回类型从 Promise<boolean | FormError>
修改为 Promise<true | FormError>
。请确认这种限制是否必要,或者是否应保持为 boolean
以涵盖所有可能的布尔值。
228-228
: 确保验证函数返回类型的一致性
在 validateMap
中,验证函数的返回类型修改为 Promise<true | FormError>
。请确认所有相关的验证函数的返回类型都已同步更新,并且调用这些函数的地方都已适配新的返回类型。
packages/hooks/src/components/use-form/use-form.ts (2)
157-162
: 新增 getValue 方法,提升了获取表单值的便利性
新增的 getValue
方法使得获取指定字段值或整个表单值更加简洁便利。
416-419
: 修正变量名拼写错误
将 validateFiled
更正为 validateField
,提高了代码的可读性和准确性。
packages/base/src/tabs/tabs-panel.tsx (2)
24-24
: 检查 useEffect
的依赖数组
使用条件展开 ...(color ? [active] : [])
可能导致依赖不完整。建议明确包含所有依赖项,避免潜在的状态同步问题。
22-22
: 确认 color
属性的逻辑
在设置 color
属性时,使用了 props.color || (active === id ? color : undefined)
。请确认此逻辑是否与预期一致,避免在颜色设置上出现意外情况。
const { defaultActive, active = defaultActive, onChange } = props; | ||
const [activeTabs, setActiveTabs] = useState(active); | ||
const [tabs, setTabs] = useState<TabData[]>([]); |
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.
🛠️ Refactor suggestion
建议添加数据验证
tabs
状态的更新缺少数据验证。建议添加验证逻辑以确保数据的完整性和正确性。
- const [tabs, setTabs] = useState<TabData[]>([]);
+ const [tabs, setTabs] = useState<TabData[]>([]);
+ const validateAndSetTabs = (newTabs: TabData[]) => {
+ if (!Array.isArray(newTabs)) {
+ console.warn('tabs必须是一个数组');
+ return;
+ }
+ setTabs(newTabs);
+ };
Committable suggestion skipped: line range outside the PR's diff.
const validateFields = usePersistFn((fields: keyof V | Array<keyof V>) => { | ||
return formFunc.validateFields(fields as string | string[]).catch(() => {}); |
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.
validateFields 方法存在潜在问题
- 静态分析工具显示存在不可达代码
- 当前实现会静默捕获所有错误,这可能导致错误被忽略
建议修改为:
const validateFields = usePersistFn((fields: keyof V | Array<keyof V>) => {
- return formFunc.validateFields(fields as string | string[]).catch(() => {});
+ return formFunc.validateFields(fields as string | string[])
+ .catch((error) => {
+ console.warn('表单字段验证失败:', error);
+ return { valid: false, error };
+ });
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const validateFields = usePersistFn((fields: keyof V | Array<keyof V>) => { | |
return formFunc.validateFields(fields as string | string[]).catch(() => {}); | |
const validateFields = usePersistFn((fields: keyof V | Array<keyof V>) => { | |
return formFunc.validateFields(fields as string | string[]) | |
.catch((error) => { | |
console.warn('表单字段验证失败:', error); | |
return { valid: false, error }; | |
}); | |
}); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
<Form.Item label='Name' required> | ||
<StarRate name="rate" allowHalf defaultValue={3.5} onChange={v => { | ||
console.log('======================') | ||
console.log('StarRate onChange: >>', v) | ||
console.log('======================') | ||
}} /> | ||
</Form.Item> |
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.
存在重要问题需要修正
- 发现重复的表单项标签"Name",这可能会导致用户混淆
- onChange事件处理器中包含了调试用的console.log语句
建议按如下方式修改:
- <Form.Item label='Name' required>
+ <Form.Item label='评分' required>
<StarRate name="rate" allowHalf defaultValue={3.5} onChange={v => {
- console.log('======================')
- console.log('StarRate onChange: >>', v)
- console.log('======================')
+ // 处理评分变化
}} />
</Form.Item>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Form.Item label='Name' required> | |
<StarRate name="rate" allowHalf defaultValue={3.5} onChange={v => { | |
console.log('======================') | |
console.log('StarRate onChange: >>', v) | |
console.log('======================') | |
}} /> | |
</Form.Item> | |
<Form.Item label='评分' required> | |
<StarRate name="rate" allowHalf defaultValue={3.5} onChange={v => { | |
// 处理评分变化 | |
}} /> | |
</Form.Item> |
@@ -102,9 +102,10 @@ export interface SimpleInputProps | |||
showClear?: boolean; | |||
renderInput?: (inputEl: React.ReactElement) => React.ReactElement; | |||
hasSuffix?: boolean; | |||
name?: string; |
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.
移除重复的 name 属性声明
name
属性已经通过 Pick<CommonType, '... | 'name'>
继承,无需重复声明。重复声明可能导致类型冲突和维护问题。
建议应用以下修改:
- name?: string;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
name?: string; |
<Form.Item label={`brand-${value.name}`} required> | ||
<Input | ||
name={`brand-${value.name}`} | ||
rules={[{ required: true, message: 'shein-field1 is required' }]} |
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.
修正验证消息中的字段名称
验证消息中的字段名称 'shein-field1' 与实际字段名称不匹配,应该使用动态生成的名称。
-rules={[{ required: true, message: 'shein-field1 is required' }]}
+rules={[{ required: true, message: `brand-${value.name} is required` }]}
Committable suggestion skipped: line range outside the PR's diff.
/** | ||
* @en return form value | ||
* @cn 返回表单的值 | ||
*/ | ||
getValue: (name?: string) => any | Value; | ||
getValue: (name?: string) => any | FormValue; |
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.
🛠️ Refactor suggestion
建议将参数类型从 string
更改为 KeyType
getValue
方法的参数 name
类型为 string
,考虑将其修改为 KeyType
,以支持 number
和 symbol
类型的字段名称,提升方法的通用性。
建议的修改:
-getValue: (name?: string) => any | FormValue;
+getValue: (name?: KeyType) => any | FormValue;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
getValue: (name?: string) => any | FormValue; | |
getValue: (name?: KeyType) => any | FormValue; |
packages/base/src/form/form.type.ts
Outdated
/** | ||
* @en Scroll to the position of the specified field | ||
* @cn 滚动到指定字段的位置 | ||
*/ | ||
scrollToField: (name: string) => void; |
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.
🛠️ Refactor suggestion
建议统一字段名称类型以增强一致性
scrollToField
方法的参数 name
类型为 string
,建议修改为 KeyType
,以支持更多类型的字段名称,确保与其他方法一致。
建议的修改:
-scrollToField: (name: string) => void;
+scrollToField: (name: KeyType) => void;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/** | |
* @en Scroll to the position of the specified field | |
* @cn 滚动到指定字段的位置 | |
*/ | |
scrollToField: (name: string) => void; | |
/** | |
* @en Scroll to the position of the specified field | |
* @cn 滚动到指定字段的位置 | |
*/ | |
scrollToField: (name: KeyType) => void; |
validatorValue = fieldArray.reduce( | ||
(prev, cur) => ({ | ||
...prev, | ||
[cur]: getValue(cur), | ||
}), | ||
{}, | ||
); |
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.
🛠️ Refactor suggestion
建议避免在 reduce 累加器中使用展开运算符
在 reduce
中使用展开运算符会导致时间复杂度从 O(n) 增加到 O(n^2),对性能有较大影响。建议使用 forEach
或其他方法累加值。
可以将代码修改为:
-validatorValue = fieldArray.reduce(
- (prev, cur) => ({
- ...prev,
- [cur]: getValue(cur),
- }),
- {},
-);
+validatorValue = {};
+fieldArray.forEach((cur) => {
+ validatorValue[cur] = getValue(cur);
+});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
validatorValue = fieldArray.reduce( | |
(prev, cur) => ({ | |
...prev, | |
[cur]: getValue(cur), | |
}), | |
{}, | |
); | |
validatorValue = {}; | |
fieldArray.forEach((cur) => { | |
validatorValue[cur] = getValue(cur); | |
}); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 182-182: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
useLayoutEffect(() => { | ||
setTabs(prev => { | ||
const prevTab = prev.find(item => item.id === id) | ||
if(prevTab){ | ||
return prev.map(item => item.id === id ? { ...item, tab } : item) | ||
} | ||
return [...prev, { id, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } as TabData] | ||
}) | ||
}, [id, tab, color, ...(color ? [active] : []), props.disabled, props.jssStyle]) |
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.
🛠️ Refactor suggestion
建议使用 useEffect
并在组件卸载时清理 tabs
- 建议将
useLayoutEffect
替换为useEffect
,以避免阻塞浏览器绘制,提高性能。 - 同时,
useEffect
可以返回一个清理函数,在组件卸载时从tabs
中移除对应的 tab,避免内存泄漏和状态不一致。
const prevTab = prev.find(item => item.id === id) | ||
if(prevTab){ | ||
return prev.map(item => item.id === id ? { ...item, tab } : item) | ||
} | ||
return [...prev, { id, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } as TabData] |
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.
🛠️ Refactor suggestion
更新已有 tab 时应同步更新所有相关属性
当前仅更新了 tab
属性,建议同步更新 disabled
、jssStyle
、color
等属性,以确保 tab 数据的一致性。
c46e242
to
d34b21f
Compare
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: 9
🧹 Outside diff range and nitpick comments (37)
packages/shineout/src/form/__example__/001-base.tsx (2)
62-62
: 建议本地化提示信息在第62行,
removeConfirm='Are you sure to delete it ?'
提示信息为英文。为了提供一致的用户体验,建议使用中文提示或实现国际化。可修改为:
- removeConfirm='Are you sure to delete it ?' + removeConfirm='确定要删除吗?'
40-40
: 建议为邮箱输入添加格式验证在第40行的邮箱输入框中,缺少对邮箱格式的验证,可能导致用户输入无效的邮箱地址。
可使用
rules
属性添加邮箱格式验证,例如:- <Input name={'email'} clearable placeholder='please input email' /> + <Input + name={'email'} + clearable + placeholder='please input email' + rules={[{ type: 'email', message: '请输入有效的邮箱地址' }]} + />packages/shineout/src/form/__example__/temp/test-form-value.ts (1)
336-336
: 发现无效的测试邮箱格式在第336行,
contact
字段的邮箱格式无效。这可能会导致测试中的邮件发送失败。建议使用有效的测试邮箱格式,例如[email protected]
。packages/base/src/form/form-field-context.ts (3)
1-2
: 建议完善注释文档当前注释过于简单,建议补充更详细的说明,包括:
- Context 的用途和使用场景
- fieldId 的格式和生成规则
- separator 的使用场景和注意事项
建议修改为:
-// 把id放到context中 +/** + * FormField Context + * + * 用于在表单组件树中传递和共享字段标识信息: + * - fieldId: 用于唯一标识表单字段,支持嵌套结构 + * - separator: 用于在嵌套结构中分隔字段名 + * + * @example + * // 基础用法 + * const { fieldId } = useContext(FormFieldContext); + * + * // 嵌套字段 + * parentId + separator + fieldName + */
4-7
: 接口设计清晰,建议添加属性注释接口定义简洁明确,但建议为属性添加 JSDoc 注释以提供更好的类型提示。
interface FormFieldContextProps { + /** 表单字段唯一标识 */ fieldId: string; + /** 嵌套字段分隔符 */ separator: string; }
9-12
: Context 实现合理,建议考虑配置化分隔符Context 的默认值设置合理,但建议考虑将 separator 改为可配置项,方便用户自定义分隔符。
建议考虑以下几点优化:
- 将 separator 默认值迁移到配置文件或常量文件中
- 考虑添加 Provider 组件以简化 Context 的使用
- 导出 useFormField hook 以规范化 Context 的消费方式
packages/hooks/src/components/use-form/use-form-context.type.ts (1)
11-11
: validate 方法的类型定义改进将返回类型从
any
改为泛型Value
提高了类型安全性,同时新增的可选参数fields
支持了选择性校验,这些都是很好的改进。建议在注释中补充
fields
参数的使用说明。/** * @en Validate form * @cn 校验表单 + * @param fields - @en Fields to validate @cn 需要校验的字段 */
packages/base/src/tabs/tabs-panel.tsx (2)
17-22
: 建议增强 tab 更新逻辑的健壮性当更新已存在的 tab 时,建议:
- 使用
Object.assign
或展开运算符确保所有属性的完整更新- 添加对
prevTab
是否存在的类型守卫setTabs(prev => { const prevTab = prev.find(item => item.id === id) if(prevTab){ - return prev.map(item => item.id === id ? { ...item, tab } : item) + return prev.map(item => item.id === id ? { ...item, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } : item) } return [...prev, { id, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } as TabData] })
24-24
: 建议优化依赖数组的写法当前依赖数组使用展开运算符的写法不够直观,建议改写为更清晰的条件表达式:
- }, [id, tab, color, ...(color ? [active] : []), props.disabled, props.jssStyle]) + }, [id, tab, color, props.disabled, props.jssStyle, ...(color ? [active] : [])])packages/base/src/button/button-group.tsx (1)
29-29
: 新增 id 属性实现正确新增的 id 属性实现符合预期,能够支持表单校验增强的需求。建议为此新增属性添加相应的测试用例,以确保功能的稳定性。
建议在测试文件中添加如下测试用例:
it('should render with custom id', () => { const { container } = render( <Button.Group id="test-group"> <Button>按钮1</Button> <Button>按钮2</Button> </Button.Group> ); expect(container.querySelector('#test-group')).toBeInTheDocument(); });packages/base/src/switch/switch.tsx (1)
13-13
: 建议添加上下文存在性检查建议添加对
FormFieldContext
的存在性检查,以防止在Switch
组件在表单上下文之外使用时出现问题。- const { fieldId } = useContext(FormFieldContext); + const { fieldId = undefined } = useContext(FormFieldContext) || {};packages/shineout/src/hooks/use-field-common.tsx (2)
36-39
: 泛型参数约束合理泛型参数的约束设计合理,通过继承
FieldItemCommonProps
确保了类型安全。建议在类型声明中添加更详细的文档注释,以便其他开发者更好地理解和使用。建议添加如下文档注释:
+/** + * @template Props - 继承自 FieldItemCommonProps 的属性类型 + * @template Value - 字段值的类型 + * @template Name - 字段名称的类型,默认为 string + */ const useFieldCommon = < Props extends FieldItemCommonProps, Value, Name extends string | string[] = string, >
Line range hint
42-45
: 考虑优化 void 类型使用
beforeChange
回调函数的返回类型包含void
,这可能导致类型混淆。建议修改为:
- beforeChange?: (value: T) => T | undefined | void; + beforeChange?: (value: T) => T | undefined;这样可以使类型定义更加清晰和准确。
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
packages/base/src/textarea/simple-textarea.tsx (1)
28-28
: 上下文使用正确,建议添加默认值处理!使用
useContext
获取fieldId
的方式符合最佳实践。不过,建议添加默认值处理,以防止在FormFieldContext
不存在时出现问题。建议修改为:
- const { fieldId } = useContext(FormFieldContext); + const { fieldId = undefined } = useContext(FormFieldContext);packages/base/src/input/simple-input.tsx (1)
30-30
: 上下文使用正确,建议添加默认值处理!使用
useContext
获取fieldId
的方式正确,但建议考虑添加默认值处理,以防止在FormFieldContext
不存在时出现问题。建议修改为:
- const { fieldId } = useContext(FormFieldContext); + const { fieldId = undefined } = useContext(FormFieldContext);packages/base/src/checkbox/simple-checkbox.tsx (1)
38-38
: 建议增强无障碍访问性虽然添加
id
属性是个好的开始,但建议同时添加aria-label
或aria-labelledby
属性以提供更好的无障碍访问支持。建议添加如下属性:
id={fieldId} +aria-label={props.children?.toString()} +aria-checked={checked}packages/base/src/form/form.tsx (2)
37-42
: 代码格式需要优化
- 第42行末尾存在多余的分号
- 注释建议改为更明确的中文说明
建议修改为:
const validate = usePersistFn((fields?: keyof V | Array<keyof V>) => { return formFunc.validateFields(fields as string | string[]) .catch((error: ValidationError<V>) => { - // 这里可以选择处理错误,或者直接抛出 + // 捕获验证错误并向上抛出,供上层业务处理 throw error; - });; + }); });
Line range hint
37-56
: 建议优化错误处理机制目前
validate
和validateFields
的错误处理机制不一致,建议统一处理方式,提供更好的错误反馈。建议将错误处理统一为:
interface ValidationResult<V> { valid: boolean; values?: V; error?: ValidationError<V>; } const validate = usePersistFn((fields?: keyof V | Array<keyof V>): Promise<ValidationResult<V>> => { return formFunc.validateFields(fields as string | string[]) .then((values) => ({ valid: true, values })) .catch((error: ValidationError<V>) => ({ valid: false, error })); }); const validateFields = usePersistFn((fields: keyof V | Array<keyof V>): Promise<ValidationResult<V>> => { return validate(fields); });🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
packages/base/src/checkbox/checkbox-group.tsx (1)
94-94
: 建议增强无障碍性支持虽然已经添加了
fieldId
属性来提升可访问性,但建议进一步完善:
- 考虑添加
aria-label
或aria-labelledby
属性- 为复选框组添加
role="group"
属性建议应用以下改进:
- <div className={groupClass} style={style} id={fieldId}> + <div className={groupClass} style={style} id={fieldId} role="group" aria-label="checkbox-group"> - <div className={groupClass} style={style} id={fieldId}> + <div className={groupClass} style={style} id={fieldId} role="group" aria-label="checkbox-group">Also applies to: 100-100
packages/base/src/radio/radio-group.tsx (1)
130-132
: 建议增强无障碍性支持虽然已经正确添加了
fieldId
,但建议进一步完善无障碍性支持。建议添加
aria-labelledby
属性以提升屏幕阅读器的可用性:- <Button.Group jssStyle={jssStyle} className={groupClass} style={style} id={fieldId}> + <Button.Group + jssStyle={jssStyle} + className={groupClass} + style={style} + id={fieldId} + aria-labelledby={fieldId} + role="radiogroup"> - <div className={groupClass} style={style} id={fieldId}> + <div + className={groupClass} + style={style} + id={fieldId} + aria-labelledby={fieldId} + role="radiogroup">Also applies to: 136-139
packages/base/src/rate/rate.tsx (1)
122-122
: DOM属性添加合理,建议考虑可访问性增强将fieldId作为id属性添加到根元素是正确的做法,这样可以提高表单的可访问性。建议考虑以下几点增强:
- 可以考虑添加aria-label属性来提供更好的屏幕阅读器支持
- 考虑在hover状态下添加aria-valuenow属性来反映当前值
packages/base/src/editable-area/editable-area.tsx (2)
41-41
: 正确使用FormFieldContext获取fieldId通过useContext正确地获取fieldId,增强了组件的表单关联能力。建议添加一个默认值以防止在FormFieldContext之外使用时出现undefined。
- const { fieldId } = useContext(FormFieldContext); + const { fieldId = '' } = useContext(FormFieldContext);
Line range hint
1-190
: 建议优化组件的错误状态处理考虑到PR的主要目标是增强表单校验,建议改进以下几点:
- 错误状态展示可以更加细化,支持不同类型的验证错误
- 考虑添加验证状态的过渡动画
- 可以提供一个回调函数来通知父组件验证状态的变化
packages/base/src/transfer/transfer.tsx (1)
190-193
: 表单字段上下文集成实现合理通过
FormFieldContext
获取fieldId
并应用到根元素的实现方式符合最佳实践:
- 使用
useContext
钩子正确获取上下文- 将
fieldId
应用到根元素增强了表单的可访问性建议考虑添加以下增强:
- 当
fieldId
不存在时的优雅降级处理- 为组件添加
aria-labelledby
属性以进一步提高可访问性- <div className={rootClass} style={style} id={fieldId}> + <div + className={rootClass} + style={style} + id={fieldId} + aria-labelledby={fieldId ? `${fieldId}-label` : undefined} + >packages/hooks/src/components/use-form/use-form.type.ts (1)
18-33
: 建议完善 ValidateFieldsFn 的类型约束和文档
ValidateFieldsFn
的泛型参数设计合理,但有以下建议:
- 建议为
ValidateFnConfig
中的type
属性添加详细的 JSDoc 说明,特别是 'forcePass' 的具体使用场景- 考虑为
ValidateFieldsFn
的返回值添加更严格的类型约束export type ValidateFnConfig = { + /** + * @en When set to 'forcePass', the validation will pass regardless of the actual validation result + * @cn 当设置为 'forcePass' 时,无论实际校验结果如何都会通过校验 + */ type?: 'forcePass', ignoreBind?: boolean, }packages/base/src/upload/upload.tsx (1)
227-230
: 建议优化表单字段标识符的处理方式当前实现直接将 fieldId 设置为 div 的 id 属性,建议考虑以下几点改进:
- 添加空值检查,避免在 fieldId 为空时设置无意义的 id 属性
- 考虑将 fieldId 与组件特定标识符组合,以确保全局唯一性
建议按如下方式修改:
- const { fieldId } = useContext(FormFieldContext); + const { fieldId } = useContext(FormFieldContext); + const uploadFieldId = fieldId ? `${fieldId}-upload` : undefined; return ( <div - id={fieldId} + id={uploadFieldId} style={props.style}packages/base/src/date-picker/result.tsx (2)
3-3
: 建议优化导入语句的组织方式建议将相关的导入语句组织在一起:
- React相关的导入(React, useContext等)
- 内部组件和上下文的导入(FormFieldContext)
- 类型和工具函数的导入
这样可以提高代码的可读性和可维护性。
-import React, { useContext, useEffect, useRef, useState } from 'react'; +import React, { + useContext, + useEffect, + useRef, + useState +} from 'react'; + import classNames from 'classnames'; -import { FormFieldContext } from '../form/form-field-context'; +import { FormFieldContext } from '../form/form-field-context';Also applies to: 5-5
Line range hint
7-31
: 建议优化组件属性类型定义当前Input组件的属性定义较为冗长,建议:
- 将属性类型提取为独立的接口
- 按照功能对属性进行分组
- 考虑使用Pick或Omit从HTML input元素的属性类型中派生
这样可以提高代码的可维护性和可读性。
interface InputBaseProps { id?: string; value: string; onChange: (v: string) => void; } interface InputUIProps { className?: string; style?: React.CSSProperties; placeholder?: string; } interface InputBehaviorProps { open?: boolean; disabled?: boolean; readOnly?: boolean; maxLength?: number; } interface InputRefProps { onRef?: (ref: HTMLInputElement) => void; inputRef?: React.MutableRefObject<{ updateValue?: React.Dispatch<React.SetStateAction<string>>; } | undefined>; } interface InputEventProps { onMouseDown?: (e: React.MouseEvent) => void; onPaste?: (e: React.ClipboardEvent) => void; onFocus?: (e: React.FocusEvent) => void; onBlur?: (e: React.FocusEvent) => void; onClick?: (e?: React.MouseEvent) => void; } type InputProps = InputBaseProps & InputUIProps & InputBehaviorProps & InputRefProps & InputEventProps;packages/shineout/src/form/__example__/test-002-validate-scrolltoerror.tsx (3)
84-95
: 建议清理未使用的注释代码保留注释掉的测试代码会影响代码的可维护性,建议:
- 如果这些代码是测试用例,应移至专门的测试文件
- 如果这些代码已不再需要,应直接删除
96-233
: 建议优化表单结构和验证规则当前表单结构存在以下可优化点:
- 重复的验证规则可以抽取为常量
- Select 组件的选项数据应该使用常量或从接口获取
- 嵌套层级较深,建议考虑拆分组件
建议添加以下常量定义:
const REQUIRED_RULE = { required: true, message: '该字段为必填项' }; const SELECT_OPTIONS = [ { label: '选项1', value: 1 }, { label: '选项2', value: 2 }, { label: '选项3', value: 3 }, ];然后重构表单项:
<Form.Item label='ruleEnumList' required> <Select keygen name='ruleEnumList' - data={[1, 2, 3]} - rules={[{ required: true, message: 'ruleEnumList is required' }]} + data={SELECT_OPTIONS} + rules={[REQUIRED_RULE]} /> </Form.Item>
236-236
: 建议移除注释掉的调试代码建议删除注释掉的 JSON 输出代码,如需调试可使用 React DevTools 查看组件状态。
packages/hooks/src/components/use-form/use-form.ts (4)
70-70
: 表单 DOM 引用处理的改进建议当前实现中,
formDomRef
的类型是隐式推导的。建议明确声明类型以提高代码的可维护性。- const formDomRef = React.useRef<HTMLFormElement>(); + const formDomRef = React.useRef<HTMLFormElement | null>(null);Also applies to: 134-146
220-255
: scrollToField 实现的改进建议
- 建议添加防抖处理,避免频繁调用导致的性能问题。
- 建议添加滚动完成的回调函数。
const scrollToField = usePersistFn( - (name: string, scrollIntoViewOptions: ScrollIntoViewOptions = {}) => { + ( + name: string, + scrollIntoViewOptions: ScrollIntoViewOptions = {}, + callback?: () => void + ) => { // ... existing code ... if (element) { // ... existing code ... + if (callback) { + // 使用 requestAnimationFrame 确保在滚动完成后调用 + requestAnimationFrame(() => { + callback(); + }); + } } } );
443-446
: 解绑方法的类型安全性改进
validateField
参数名称已更正,但建议进一步增强类型安全性。- unbind: (n: string, reserveAble?: boolean, validateField?: ValidateFn, update?: UpdateFn) + unbind: (n: string, reserveAble?: boolean, validateField?: ValidateFn | null, update?: UpdateFn | null)
251-252
: 警告信息国际化建议建议将警告信息纳入统一的国际化系统管理。
- console.warn(`[shineout] fieldId: ${fieldId} not found`); + console.warn(t('form.fieldNotFound', { fieldId })); // 使用国际化函数 tpackages/base/src/tree-select/tree-select.tsx (1)
651-651
: 建议验证 fieldId 的可用性虽然添加
id
属性提升了表单的可访问性,但建议添加条件渲染以处理fieldId
为空的情况。建议修改为:
- id={fieldId} + id={fieldId || undefined}packages/base/src/select/select.tsx (1)
Line range hint
31-766
: 建议优化组件结构以提高可维护性该组件实现了丰富的功能,但建议考虑以下优化:
- 将复杂的条件逻辑抽取为命名函数,例如:
- if (multiple && !shouldFocus) { + const shouldFocusInput = () => multiple && !shouldFocus; + if (shouldFocusInput()) {
- 考虑将部分逻辑拆分为自定义 hooks,例如键盘导航逻辑。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (50)
package.json
(1 hunks)packages/base/src/button/button-group.tsx
(1 hunks)packages/base/src/button/button.type.ts
(1 hunks)packages/base/src/cascader/cascader.tsx
(3 hunks)packages/base/src/checkbox/checkbox-group.tsx
(3 hunks)packages/base/src/checkbox/simple-checkbox.tsx
(2 hunks)packages/base/src/common/type.ts
(1 hunks)packages/base/src/common/use-with-form-config.ts
(1 hunks)packages/base/src/date-picker/result.tsx
(5 hunks)packages/base/src/editable-area/editable-area.tsx
(3 hunks)packages/base/src/form/form-field-context.ts
(1 hunks)packages/base/src/form/form-field.tsx
(3 hunks)packages/base/src/form/form-item.tsx
(2 hunks)packages/base/src/form/form.tsx
(4 hunks)packages/base/src/form/form.type.ts
(2 hunks)packages/base/src/input/input.tsx
(1 hunks)packages/base/src/input/input.type.ts
(2 hunks)packages/base/src/input/simple-input.tsx
(3 hunks)packages/base/src/radio/radio-group.tsx
(2 hunks)packages/base/src/radio/simple-radio.tsx
(2 hunks)packages/base/src/rate/rate.tsx
(2 hunks)packages/base/src/select/result.tsx
(0 hunks)packages/base/src/select/select.tsx
(4 hunks)packages/base/src/slider/slider.tsx
(2 hunks)packages/base/src/switch/switch.tsx
(2 hunks)packages/base/src/tabs/tabs-panel.tsx
(1 hunks)packages/base/src/tabs/tabs.tsx
(4 hunks)packages/base/src/textarea/simple-textarea.tsx
(3 hunks)packages/base/src/transfer/transfer.tsx
(3 hunks)packages/base/src/tree-select/tree-select.tsx
(3 hunks)packages/base/src/tree/tree.tsx
(2 hunks)packages/base/src/upload/upload.tsx
(3 hunks)packages/hooks/src/components/use-form/use-form-context.type.ts
(2 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.ts
(3 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts
(1 hunks)packages/hooks/src/components/use-form/use-form.ts
(11 hunks)packages/hooks/src/components/use-form/use-form.type.ts
(4 hunks)packages/hooks/src/components/use-tabs/context.tsx
(1 hunks)packages/hooks/src/components/use-tabs/context.type.ts
(2 hunks)packages/hooks/src/components/use-tabs/use-tabs.ts
(2 hunks)packages/hooks/src/utils/dom/element.tsx
(1 hunks)packages/shineout-style/src/tabs/tabs.ts
(1 hunks)packages/shineout/src/form/__doc__/changelog.cn.md
(1 hunks)packages/shineout/src/form/__example__/001-base.tsx
(1 hunks)packages/shineout/src/form/__example__/002-control.tsx
(3 hunks)packages/shineout/src/form/__example__/006-validate-05-scrolltoerror.tsx
(1 hunks)packages/shineout/src/form/__example__/temp/test-form-value.ts
(1 hunks)packages/shineout/src/form/__example__/test-002-validate-scrolltoerror.tsx
(1 hunks)packages/shineout/src/hooks/use-field-common.tsx
(3 hunks)packages/shineout/src/tabs/__doc__/changelog.cn.md
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/base/src/select/result.tsx
🚧 Files skipped from review as they are similar to previous changes (20)
- package.json
- packages/base/src/common/type.ts
- packages/base/src/common/use-with-form-config.ts
- packages/base/src/form/form-field.tsx
- packages/base/src/form/form-item.tsx
- packages/base/src/form/form.type.ts
- packages/base/src/input/input.tsx
- packages/base/src/input/input.type.ts
- packages/base/src/tabs/tabs.tsx
- packages/hooks/src/components/use-form/use-form-control/use-form-control.ts
- packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts
- packages/hooks/src/components/use-tabs/context.tsx
- packages/hooks/src/components/use-tabs/context.type.ts
- packages/hooks/src/components/use-tabs/use-tabs.ts
- packages/hooks/src/utils/dom/element.tsx
- packages/shineout-style/src/tabs/tabs.ts
- packages/shineout/src/form/doc/changelog.cn.md
- packages/shineout/src/form/example/002-control.tsx
- packages/shineout/src/form/example/006-validate-05-scrolltoerror.tsx
- packages/shineout/src/tabs/doc/changelog.cn.md
🧰 Additional context used
🪛 Biome (1.9.4)
packages/base/src/form/form.tsx
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
packages/hooks/src/components/use-form/use-form.ts
[error] 183-183: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/shineout/src/hooks/use-field-common.tsx
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (56)
packages/shineout/src/form/__example__/001-base.tsx (1)
65-65
:
修正回调函数参数名称的拼写错误
在第65行的 onSuccess
回调函数中,参数 filem
可能是拼写错误,应为 file
。
请应用以下修改以修正参数名称:
- onSuccess={(_res, filem, data) => {
+ onSuccess={(_res, file, data) => {
Likely invalid or redundant comment.
packages/shineout/src/form/__example__/temp/test-form-value.ts (3)
1-454
: 重复此前的评论:建议添加类型定义和文档说明
1-454
: 重复此前的评论:需要确保测试数据的一致性
1-454
: 重复此前的评论:改进测试数据质量和安全性
packages/hooks/src/components/use-form/use-form-context.type.ts (1)
42-46
: 新增的 scrollToField 方法实现需要验证
方法签名和文档注释看起来没有问题,但建议验证实现是否正确处理了以下场景:
- 字段不存在时的错误处理
- 字段在视口外时的滚动行为
- 嵌套表单中的字段定位
✅ Verification successful
scrollToField 方法实现已完整验证
根据代码实现验证,该方法已经正确处理了所有关键场景:
-
字段不存在时有适当的错误处理:
- 检查 name 是否为空
- 找不到元素时会在控制台输出警告信息
-
滚动行为处理完善:
- 使用
scrollIntoView
实现平滑滚动 - 支持自定义滚动选项
- 会查找最近的可滚动容器作为滚动上下文
- 使用
-
嵌套表单支持:
- 通过
getFieldId
方法正确处理了表单名称和字段名的组合 - 支持多层级的字段路径
- 通过
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有相关错误处理代码
rg -A 5 "scrollToField.*{"
# 检查滚动行为的实现
ast-grep --pattern 'scrollIntoView($$$)'
Length of output: 167
Script:
#!/bin/bash
# 搜索 scrollToField 方法的实现
rg -A 10 "scrollToField.*\("
# 搜索包含 scrollIntoView 的相关代码
rg -A 5 "scrollIntoView"
# 查找可能的错误处理相关代码
rg -B 2 -A 5 "getFieldRef|getField.*name"
Length of output: 9835
packages/base/src/tabs/tabs-panel.tsx (2)
9-12
: 代码结构清晰,类型完善!
解构和上下文使用恰当,保持了良好的类型安全性。
16-24
: 建议优化性能和用户体验
考虑到该更新不需要同步渲染,建议使用 useEffect
替代 useLayoutEffect
,以避免阻塞视图更新。同时建议在组件卸载时清理 tabs 状态。
packages/base/src/radio/simple-radio.tsx (3)
3-3
: 导入声明看起来没有问题!
新增的 useContext
和 FormFieldContext
导入正确支持了表单字段集成功能。
Also applies to: 5-5
9-9
: Context 使用正确,建议验证字段 ID 的一致性
从 FormFieldContext 中提取 fieldId 的实现是正确的,但建议验证在其他表单组件中的一致性。
✅ Verification successful
表单组件中 fieldId 的使用模式一致且正确
从验证结果来看,所有表单相关组件(包括 Input、Radio、Checkbox、Select、TreeSelect、Upload 等)都采用了相同的方式从 FormFieldContext 中提取 fieldId,实现了统一且正确的表单字段集成模式。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他表单组件是否也正确使用了 fieldId
ast-grep --pattern 'const { fieldId } = useContext(FormFieldContext)'
Length of output: 1698
36-36
: 建议验证 ID 属性对现有功能的影响
向根 div 元素添加 id 属性是正确的做法,这有助于提高表单的可访问性。但建议验证:
- 是否会影响现有的样式规则
- 是否会影响任何依赖于 DOM 结构的测试用例
✅ Verification successful
添加 fieldId 属性是安全的
根据测试文件的分析,Radio 组件的测试用例主要关注组件的功能性测试(如点击、禁用状态、分组等),没有依赖于特定的 DOM ID。同时也未发现任何基于 ID 选择器的样式规则。因此,向根元素添加 fieldId 属性不会影响现有功能和样式。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有依赖于 Radio 组件 DOM 结构的测试
rg -A 5 "test\(.*Radio" --type=typescript
Length of output: 78
Script:
#!/bin/bash
# Search for Radio component tests
rg -A 5 "test\(.*Radio"
# Search for any CSS selectors targeting Radio component by ID
rg "#.*Radio" --type=css
# Check test files specifically
fd "test|spec" -e ts -e tsx -e js -e jsx --exec rg -l "Radio"
Length of output: 224
Script:
#!/bin/bash
# Check the content of radio test file
cat ./packages/shineout/src/radio/__test__/radio.spec.tsx
# Check if there are any style files related to Radio
fd -e css -e less -e scss -e sass | xargs rg "Radio"
Length of output: 18045
packages/base/src/switch/switch.tsx (2)
3-3
: 导入声明看起来没有问题!
新增的 useContext
和 FormFieldContext
导入声明位置适当,符合导入顺序规范。
Also applies to: 7-7
55-55
: 很好的可访问性改进!
通过添加 fieldId
作为按钮的 id
属性,增强了表单字段的可访问性,使得 Switch 组件能够与表单标签建立正确的关联关系。这个改动符合表单校验增强的目标。
packages/shineout/src/hooks/use-field-common.tsx (3)
Line range hint 27-31
: 接口命名修正和类型定义完善
接口重命名从 FiledItemCommonProps
到 FieldItemCommonProps
修正了拼写错误,提高了代码质量。
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
54-56
: 表单名称集成增强
新增的 formName
属性集成增强了表单字段的上下文管理。这与 PR 目标中提到的表单校验增强相符。
Line range hint 42-71
: 验证功能集成完善
表单验证相关的属性和方法集成完善,符合 PR 的主要目标。建议添加单元测试以确保验证功能的可靠性。
需要我帮您编写相关的单元测试用例吗?
✅ Verification successful
表单验证功能已有充分的测试覆盖
根据代码分析结果,use-field-common 的验证功能已经在多个组件中得到了完整的测试覆盖:
-
Upload 组件中包含了完整的验证测试用例,包括:
- 自定义验证器(customValidator)
- 文件大小验证
- 文件类型验证
- beforeChange 回调验证
-
Input 和 Select 等组件也都包含了相应的验证测试用例
现有的测试用例已经充分覆盖了验证功能的各个方面,无需添加额外的单元测试。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查相关测试文件是否存在和覆盖率
# 查找相关测试文件
fd -e test.tsx -e spec.tsx "use-field-common" packages/shineout/src/hooks/
# 检查测试中是否包含验证相关的用例
rg -l "describe.*validate|test.*validate|it.*validate" packages/shineout/src/hooks/
Length of output: 160
Script:
#!/bin/bash
# 扩大搜索范围,查找所有相关的测试文件
fd -e test.tsx -e spec.tsx . packages/shineout/src/
# 检查表单相关的测试文件
fd -e test.tsx -e spec.tsx "form|field" packages/shineout/src/
# 检查表单验证相关的测试用例
rg -l "validate|rules|beforeChange" packages/shineout/src/ -g "*.test.tsx" -g "*.spec.tsx"
# 检查 use-field-common 的使用情况
rg "useFieldCommon" packages/shineout/src/ -A 3
Length of output: 13581
Script:
#!/bin/bash
# 检查 use-field-common 相关的测试覆盖情况
rg "describe|it|test" packages/shineout/src/upload/__test__/upload.spec.tsx -A 3
# 检查表单验证相关的测试用例内容
rg "validate|rules|beforeChange" packages/shineout/src/upload/__test__/upload.spec.tsx -A 3
# 检查其他组件中的验证测试
rg "validate|rules|beforeChange" packages/shineout/src/input/__test__/input.spec.tsx -A 3
rg "validate|rules|beforeChange" packages/shineout/src/select/__test__/select.spec.tsx -A 3
Length of output: 19641
packages/base/src/textarea/simple-textarea.tsx (2)
3-3
: 导入声明看起来没有问题!
新增的 useContext
和 FormFieldContext
导入正确支持了表单字段上下文的集成。
Also applies to: 5-5
77-77
: fieldId 应用正确,建议验证一致性!
将 fieldId
作为 id
属性应用到根元素是正确的做法,这样可以增强表单的可访问性。
让我们验证其他表单组件是否也进行了类似的更改:
✅ Verification successful
fieldId 的使用已在所有表单组件中得到验证,实现一致!
通过代码分析可以确认:
- 所有表单组件都正确使用了
FormFieldContext
来获取fieldId
- 所有组件都将
fieldId
作为id
属性应用到其根元素上 - 实现方式保持一致,增强了整体表单的可访问性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查其他表单组件是否也使用了 fieldId
# 预期结果:其他表单组件(如 Input、Select 等)也应该使用 fieldId
# 搜索其他组件中的 FormFieldContext 使用情况
ast-grep --pattern 'const { fieldId } = useContext(FormFieldContext)'
# 搜索组件中 id={fieldId} 的使用情况
rg 'id=\{fieldId\}'
Length of output: 3146
packages/base/src/input/simple-input.tsx (2)
3-3
: 导入声明看起来没有问题!
新增的 useContext
和 FormFieldContext
导入符合组件需求,有助于实现表单字段上下文的集成。
Also applies to: 7-7
73-73
: 请验证 fieldId 的唯一性!
添加 id
属性有助于提高可访问性,但需要确保 fieldId
在页面中是唯一的,以避免可能的 DOM ID 冲突。
运行以下脚本来检查是否存在重复使用 fieldId 的情况:
✅ Verification successful
fieldId 的使用是安全的,无需担心重复ID问题
从代码分析结果可以看出:
- fieldId 是通过 FormFieldContext 上下文提供的,每个表单字段都从这个上下文获取唯一的 ID
- 在 form-field.tsx 中,FormFieldContext.Provider 为每个字段提供了独特的 formFieldId
- 通过 hooks/use-form.ts 中的 getFieldId 函数生成唯一的字段标识
- 所有表单组件(如 input、select、checkbox 等)都正确地使用了这个上下文机制
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否有其他组件也使用了相同的 fieldId 属性
# 搜索其他组件中是否有使用 id={fieldId} 的情况
rg "id={fieldId}"
# 搜索 FormFieldContext 的实现,查看 fieldId 的生成逻辑
ast-grep --pattern 'const FormFieldContext = createContext<$_>($_)'
Length of output: 188
Script:
#!/bin/bash
# 搜索使用 fieldId 作为 id 属性的情况
rg "id=\{fieldId\}"
# 搜索 fieldId 的定义和使用
rg "fieldId"
# 搜索 FormFieldContext 相关代码
rg "FormFieldContext"
# 搜索可能的 ID 生成逻辑
rg -i "generateid|genid|createid"
Length of output: 15789
packages/base/src/checkbox/simple-checkbox.tsx (2)
3-3
: 导入声明看起来没有问题!
新增的 useContext
和 FormFieldContext
导入符合 React 最佳实践,用于表单字段集成。
Also applies to: 5-5
11-11
: Context 使用正确,建议验证上下文提供
Context 的使用符合 React 模式。不过建议验证是否所有可能的父组件都正确提供了 FormFieldContext。
✅ Verification successful
FormFieldContext 使用正确且安全
FormFieldContext 的使用完全符合预期:
- FormFieldContext 由 FormField 组件提供,定义在
form-field-context.ts
中 - SimpleCheckbox 正确使用 useContext 获取 fieldId
- Checkbox 组件通过 SimpleCheckbox 正确继承了这个上下文
- 这种用法在整个代码库中保持一致,其他表单控件也都遵循相同模式
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有组件使用 Checkbox 但未提供 FormFieldContext
ast-grep --pattern 'import { Checkbox } from "@sheinx/base"
$$$
<Checkbox $$$>$$$</Checkbox>'
Length of output: 95
Script:
#!/bin/bash
# 1. 查找 FormFieldContext 的定义和提供位置
ast-grep --pattern 'const FormFieldContext = createContext'
# 2. 查找 SimpleCheckbox 的导入和使用
rg -l "SimpleCheckbox"
# 3. 查找 FormFieldContext.Provider 的使用
ast-grep --pattern '<FormFieldContext.Provider'
# 4. 检查是否有直接使用 SimpleCheckbox 的地方
ast-grep --pattern '<SimpleCheckbox'
Length of output: 304
Script:
#!/bin/bash
# 1. 检查 FormFieldContext 的完整定义和导出
rg "FormFieldContext" -A 5 -B 5
# 2. 查看 checkbox.tsx 中如何使用 SimpleCheckbox
cat packages/base/src/checkbox/checkbox.tsx
Length of output: 31797
packages/base/src/button/button.type.ts (1)
138-142
: 代码变更看起来不错!
新增的 id
属性定义清晰,文档完整。建议验证一下与表单上下文的集成是否正确实现。
packages/base/src/form/form.tsx (3)
9-16
: 类型定义完善且清晰
ValidationError
接口的定义结构合理,包含了必要的错误信息字段,便于错误处理和展示。
44-45
: 验证方法存在潜在问题
此处问题在之前的评审中已经指出,仍然有效:
- 静态分析显示存在不可达代码
- 静默捕获所有错误可能导致问题被忽略
建议采用之前建议的修复方案。
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
56-56
: 新增滚动方法符合需求
scrollToField
方法的添加符合PR目标,提升了表单的用户体验。
packages/base/src/checkbox/checkbox-group.tsx (3)
5-5
: 导入语句的组织合理
新增的 useContext
和 FormFieldContext
导入是为了支持表单字段上下文的集成,这符合组件增强的目标。
Also applies to: 8-8
17-17
: 正确使用表单字段上下文
通过 useContext
获取 fieldId
的方式符合 React Hooks 的最佳实践。
Line range hint 1-115
: 验证组件与表单集成的完整性
需要验证组件是否正确集成到表单系统中,特别是在处理表单验证和字段关联方面。
✅ Verification successful
表单集成验证已完成,实现符合预期
从代码分析结果来看,CheckboxGroup 组件已正确集成到表单系统中:
- FormFieldContext 的使用方式与其他表单组件(如 Select、Input、Radio 等)保持一致
- 通过 useWithFormConfig 和 FormFieldContext 实现了与表单系统的标准集成
- fieldId 的处理方式遵循了统一的表单字段标识规范
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证表单集成的完整性
# 检查其他组件是否也正确使用了 FormFieldContext
rg "FormFieldContext" --type typescript
# 检查是否有相关的测试用例
rg -l "checkbox.*test" --type typescript
Length of output: 152
Script:
#!/bin/bash
# 检查表单集成相关的代码
rg "FormFieldContext"
# 检查 checkbox 相关的测试文件
fd "checkbox.*test"
# 检查表单集成的实现
ast-grep --pattern 'import { FormFieldContext } from $$$'
Length of output: 4005
packages/base/src/radio/radio-group.tsx (3)
5-5
: 导入声明合理且必要
新增的导入声明与组件功能增强相符,useContext
和 FormFieldContext
的引入为表单字段上下文集成提供了必要支持。
Also applies to: 9-9
17-17
: 表单字段上下文集成实现恰当
通过 useContext(FormFieldContext)
获取 fieldId
的方式符合 React 最佳实践,有效地实现了与表单组件的联动。
Line range hint 1-141
: 验证组件与表单集成的完整性
需要验证组件是否正确集成到表单系统中。
✅ Verification successful
Let me gather more specific information about the form field context integration.
表单字段上下文集成验证完成,实现正确
Radio 组件已正确集成了表单字段上下文,具体体现在:
- FormFieldContext 在表单系统中定义完整,提供了必要的上下文属性
- Radio 组件与其他表单控件(如 Input、Select、Checkbox 等)采用了一致的 FormFieldContext 集成方式
- fieldId 的使用符合表单字段标识的规范要求
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证相关组件是否正确实现了表单字段上下文集成
# 检查其他组件是否也正确使用了 FormFieldContext
rg "FormFieldContext" --type ts --type tsx
# 检查是否有相关的测试用例
rg -l "test|spec" | rg "radio|form"
Length of output: 1518
Script:
#!/bin/bash
# 检查 FormFieldContext 的定义和使用
ast-grep --pattern 'export const FormFieldContext = $$$'
# 检查 radio 组件的测试用例中是否包含表单集成测试
rg -A 10 "Form.*Radio" packages/base/src/form/__test__/form.spec.tsx packages/shineout/src/radio/__test__/radio.spec.tsx
# 检查其他表单控件是否类似地使用了 FormFieldContext
ast-grep --pattern 'const { fieldId } = useContext(FormFieldContext)'
Length of output: 2197
packages/base/src/slider/slider.tsx (3)
3-3
: 新增表单上下文相关导入
导入 useContext
和 FormFieldContext
用于获取表单字段标识符,这符合表单校验增强的目标。
Also applies to: 7-7
14-14
: 从上下文中获取字段标识符
通过 useContext
获取 fieldId
,增强了组件与表单的集成能力。
91-91
: 为滑块组件添加字段标识符
将 fieldId
应用到滑块组件的根元素,提升了表单的可访问性和字段定位能力。建议验证在实际表单场景中的效果。
✅ Verification successful
Based on the shell script results, I can now generate the final response since I have sufficient information about the usage of fieldId
across form components.
表单组件 fieldId 应用验证完成
验证显示 fieldId
已在所有相关表单组件中正确实现:
- 所有组件都通过
FormFieldContext
获取fieldId
fieldId
统一应用于组件的根元素- 实现方式与滑块组件保持一致
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证其他表单组件是否也正确使用了 fieldId
# 检查其他组件中的 FormFieldContext 使用情况
rg "useContext.*FormFieldContext" -A 3
# 检查其他组件中的 fieldId 应用情况
rg "id=.*fieldId"
Length of output: 7193
packages/base/src/rate/rate.tsx (2)
2-2
: 导入变更正确!
React hooks的导入遵循了最佳实践,将useContext与其他React导入放在一起。
13-13
: Context使用正确,建议验证使用场景
FormFieldContext的使用符合React Hooks的规则。不过建议确认一下fieldId在父级Form组件中的传递情况。
✅ Verification successful
FormFieldContext使用完全正确且安全
FormFieldContext的实现和使用都符合预期:
- FormField组件正确提供了fieldId,通过name和formName生成
- Context的类型定义完整且清晰
- Provider确保了fieldId的传递
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查Form组件中是否正确提供了fieldId
ast-grep --pattern 'const FormFieldContext = createContext<{ fieldId?: string }>($$$)'
Length of output: 88
Script:
#!/bin/bash
# Search for FormFieldContext definition
ast-grep --pattern 'const FormFieldContext = createContext'
# Search for FormFieldContext.Provider usage
ast-grep --pattern 'FormFieldContext.Provider value={$_}'
# Search for files containing FormFieldContext
rg "FormFieldContext" -l
Length of output: 916
Script:
#!/bin/bash
# Check FormFieldContext definition in context file
cat packages/base/src/form/form-field-context.ts
# Check FormField component implementation
cat packages/base/src/form/form-field.tsx
Length of output: 2679
packages/base/src/editable-area/editable-area.tsx (2)
1-1
: 导入FormFieldContext以支持表单字段集成
新增的FormFieldContext导入有助于组件与表单上下文的集成,这与PR的表单校验增强目标相符。
Also applies to: 9-9
172-172
: 验证fieldId属性的可访问性实现
将fieldId添加到根div元素上增强了组件的可访问性,这对表单校验很有帮助。但需要确保这个实现符合WAI-ARIA规范。
✅ Verification successful
fieldId属性的实现符合WAI-ARIA规范和组件库标准
通过验证发现,fieldId的实现是组件库中表单控件的标准做法:
- FormFieldContext在form-field-context中统一定义和管理
- 所有表单相关组件(如Input、Select、Radio等)都一致地使用fieldId作为根元素的id属性
- 这种实现方式确保了表单控件的可访问性和一致性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他表单相关组件是否也实现了类似的fieldId绑定
ast-grep --pattern 'const { fieldId } = useContext(FormFieldContext)'
# 检查是否有其他组件使用了类似的id属性绑定
rg 'id={fieldId}'
Length of output: 1817
Script:
#!/bin/bash
# 检查组件中id属性的使用情况,使用转义的大括号
rg "id=\{fieldId\}"
# 检查FormFieldContext的定义和使用
ast-grep --pattern 'FormFieldContext'
Length of output: 5416
packages/base/src/transfer/transfer.tsx (1)
1-1
: 导入声明看起来不错!
新增的导入声明清晰地表明了组件与表单上下文的集成意图。
Also applies to: 10-10
packages/hooks/src/components/use-form/use-form.type.ts (3)
7-16
: 类型定义清晰且结构合理
新增的 KeyType
和 ValidationError
接口定义明确,能够很好地支持表单校验的增强功能。ValidationError
的结构设计合理,包含了必要的错误信息字段。
228-228
: validateMap 返回类型的改进增强了类型安全性
将 validateMap
的返回类型从 Promise<boolean | FormError>
改为 Promise<true | FormError>
是一个很好的改进,这样可以在编译时捕获更多潜在的错误。
126-131
: 验证 formName 和 name 属性的一致性
FormCommonConfig
和 BaseFormProps
都添加了用于表单字段 ID 前缀的属性,需要确保它们的使用场景不会产生冲突。
Also applies to: 217-221
✅ Verification successful
Let me gather more information about the usage of these properties to better understand their relationship.
Let me check one more aspect to understand how the form ID generation is implemented.
formName
属性使用验证完成
根据代码分析,formName
属性在表单组件中的使用是一致且无冲突的:
formName
作为可选属性,用于生成表单字段的唯一 ID- 通过
getFieldId
工具函数统一处理,格式为${formName}_${fieldName}
- 该属性从表单组件向下传递给表单字段组件,确保 ID 的一致性
- 主要用于支持 HTML 原生的
<label for="id">
功能
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 formName 和 name 属性的使用情况
rg -A 5 "formName[?:]|name[?:].*formName" --type ts
Length of output: 2888
Script:
#!/bin/bash
# Check the implementation of formName in form components
ast-grep --pattern 'interface $_ {
$$$
formName?: string;
$$$
}'
# Check how formName is used in form-related components
rg -A 3 "formName: " --type ts
Length of output: 945
Script:
#!/bin/bash
# Check the implementation and usage of getFieldId
rg -A 5 "getFieldId" --type ts
# Check form field id related code
ast-grep --pattern 'id={$_}'
Length of output: 1780
packages/base/src/tree/tree.tsx (2)
8-8
: 导入语句正确添加!
新增的 FormFieldContext 导入支持了表单字段上下文的集成,这与 PR 的目标保持一致。
221-223
: 验证表单字段集成的影响
FormFieldContext 的集成增强了组件的表单关联能力,但建议验证以下几点:
- 确保在嵌套的表单场景下 fieldId 的传递是否正确
- 验证在动态表单项中 Tree 组件的表现
✅ Verification successful
FormFieldContext 集成验证完成
FormFieldContext 的集成是标准的表单字段实现,用于提供表单字段的唯一标识。根据代码分析:
- FormFieldContext 提供了标准的字段 ID 和分隔符
- Tree 组件正确使用了 fieldId 作为根元素的 id 属性
- 这种集成方式与其他表单组件保持一致
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索项目中 Tree 组件在表单中的使用场景
rg -l "Tree.*Form|Form.*Tree" --type tsx
# 检查是否存在嵌套表单的使用场景
ast-grep --pattern 'Form {
$$$
Form {
$$$
Tree
$$$
}
$$$
}'
Length of output: 152
Script:
#!/bin/bash
# Search for Tree component usage in form contexts
rg -l "Tree.*Form|Form.*Tree"
# Check for nested form scenarios with Tree component
rg -A 5 "<Tree"
# Look for FormFieldContext usage
rg "FormFieldContext"
# Find form-related test files
fd "form|test" --type f
Length of output: 85731
Script:
#!/bin/bash
# 检查 FormFieldContext 的使用和实现
rg -A 5 "FormFieldContext" packages/base/src/form/form-field-context.ts
# 检查 Tree 组件在表单中的使用示例
rg -A 10 "<Form.*Tree|Tree.*Form" packages/shineout/src/form/__example__/
# 检查 Tree 组件的测试用例中与表单相关的部分
rg -A 5 "Form.*Tree|Tree.*Form" packages/shineout/src/tree/__test__/
Length of output: 421
packages/base/src/upload/upload.tsx (3)
2-2
: 导入 useContext 钩子函数
从 React 中导入 useContext 钩子函数是正确的做法,这为组件提供了访问上下文的能力。
12-12
: 引入表单字段上下文
正确引入了 FormFieldContext,这为上传组件提供了与表单字段的关联能力。
Line range hint 1-267
: 验证上传组件与表单集成的完整性
需要验证上传组件是否正确集成了所有必要的表单特性。
✅ Verification successful
上传组件已正确集成表单功能
通过代码分析可以确认:
- Upload组件通过
useWithFormConfig
hook正确集成了表单配置,与其他表单组件(如Input、Select等)保持一致的集成方式 - 正确使用了
FormFieldContext
来获取fieldId
,这是表单字段标识的关键属性 - 集成方式与其他17个核心表单组件保持一致,显示了良好的实现一致性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他表单相关组件是否也实现了类似的集成方式
# 搜索其他组件中的 FormFieldContext 使用情况
rg "useContext.*FormFieldContext" --type tsx
# 检查是否有相关的测试用例
rg "test.*fieldId.*upload" --type test
Length of output: 144
Script:
#!/bin/bash
# 检查其他组件中的 FormFieldContext 使用情况
rg "useContext.*FormFieldContext"
# 检查表单相关的集成代码
rg "useWithFormConfig"
# 检查表单字段上下文的定义
fd "form-field-context" --exec cat {}
Length of output: 4858
packages/hooks/src/components/use-form/use-form.ts (2)
23-24
: 新增的工具函数和类型定义增强了表单的功能
新增的 getFieldId
和 getClosestScrollContainer
工具函数,以及 ValidateFnConfig
和 ValidationError
类型定义,为表单组件提供了更完善的字段定位和验证能力。
Also applies to: 36-37
158-218
: 🛠️ Refactor suggestion
验证逻辑的改进建议
- 验证逻辑中的
reduce
操作存在性能问题,建议采用更高效的实现方式。 - 建议为验证错误添加更多上下文信息,如字段的标签名称。
建议添加字段标签信息:
reject({
message: firstError.message,
values: validatorValue as T,
- errorFields,
+ errorFields: errorFields.map(field => ({
+ ...field,
+ label: getFieldLabel(field.name), // 需要实现 getFieldLabel 方法
+ })),
});
🧰 Tools
🪛 Biome (1.9.4)
[error] 183-183: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/base/src/tree-select/tree-select.tsx (3)
1-1
: 导入声明合理且完整
新增的 FormFieldContext
导入与组件功能增强相符,且放置在其他内部依赖之后,符合导入顺序规范。
Also applies to: 26-26
648-648
: Context 使用方式正确
使用 useContext
获取 fieldId
的方式符合 React Hooks 的最佳实践。
648-651
: 验证组件在表单上下文外的使用场景
需要确认组件在没有 FormFieldContext
的情况下是否能正常工作。
✅ Verification successful
Let me analyze the results. The search shows that TreeSelect is used in many examples and test cases without being wrapped in a Form context. Let me verify if there's any special handling for when FormFieldContext is not available.
Let me check how the fieldId is used in the component and if there's any handling for when it's undefined.
Based on the search results, I can now generate the final response:
TreeSelect 组件在表单上下文外可以正常使用
从代码分析来看,fieldId 仅用于设置组件根元素的 id 属性。当组件在表单上下文外使用时,fieldId 将为 undefined,这不会影响组件的正常功能。这一点已通过示例代码得到验证,如 packages/base/src/tree-select/__example__/01-base.tsx
等多个示例中都展示了不依赖表单上下文的使用场景。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查组件是否在非表单场景中使用
# 搜索组件的直接使用场景
rg -l "TreeSelect" | xargs rg "<TreeSelect"
# 搜索是否存在没有 Form 包裹的使用场景
rg -l "TreeSelect" | xargs rg -B5 "<TreeSelect" | rg -v "Form|FormProvider"
Length of output: 35717
Script:
#!/bin/bash
# Check how FormFieldContext is used in the component
ast-grep --pattern 'const { fieldId } = useContext(FormFieldContext);'
# Check if there's any default/fallback handling
rg -B3 -A3 'FormFieldContext' packages/base/src/tree-select/tree-select.tsx
Length of output: 2170
Script:
#!/bin/bash
# Check how fieldId is used in the component
rg -B5 -A5 "id={fieldId}" packages/base/src/tree-select/tree-select.tsx
# Check if there are any other id assignments or fallbacks
ast-grep --pattern 'id={$_}'
Length of output: 203
Script:
#!/bin/bash
# Check how fieldId is used in the component with proper escaping
rg -B5 -A5 'id=\{fieldId\}' packages/base/src/tree-select/tree-select.tsx
# Check the component's props and interface
rg -B3 -A3 'interface TreeSelectProps' packages/base/src/tree-select/tree-select.tsx
Length of output: 465
packages/base/src/cascader/cascader.tsx (3)
26-26
: 导入声明符合规范!
新增的 FormFieldContext
导入遵循了正确的导入声明组织方式,有助于表单字段的集成。
34-34
: 上下文使用方式正确!
通过 useContext
获取 fieldId
的方式符合 React 的最佳实践,为表单字段提供了必要的上下文信息。
Line range hint 716-724
: 请验证字段标识符的使用场景
已正确添加 fieldId
到根元素,这有助于提升表单的可访问性和集成度。建议验证在以下场景中的使用:
- 表单验证
- 字段聚焦
- 错误状态展示
运行以下脚本以验证 fieldId 的使用场景:
✅ Verification successful
fieldId 的使用场景已得到验证
根据代码分析,fieldId 的使用完全符合预期,主要用于以下场景:
- 表单验证:通过 FormFieldContext 提供 fieldId,用于表单组件的唯一标识
- 焦点管理:与 onFocus/onBlur 事件配合使用,实现表单控件的焦点状态管理
- 错误状态展示:用于定位具体的表单字段,展示对应的验证错误信息
- 滚动定位:在 scrollToField 方法中用于定位具体的表单字段
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 fieldId 在表单验证和错误处理中的使用
# 检查表单验证相关代码
ast-grep --pattern 'validate($$$) {
$$$
fieldId
$$$
}'
# 检查错误处理相关代码
ast-grep --pattern 'error($$$) {
$$$
fieldId
$$$
}'
# 检查字段聚焦相关代码
ast-grep --pattern 'focus($$$) {
$$$
fieldId
$$$
}'
Length of output: 182
Script:
#!/bin/bash
# 搜索 fieldId 的定义和使用
rg "fieldId" -A 3 -B 3
# 搜索表单相关的组件和验证逻辑
ast-grep --pattern 'const $_ = useForm($$$)'
# 搜索与 id 属性相关的代码
ast-grep --pattern 'id={$_}'
# 搜索与焦点管理相关的代码
rg "onFocus|onBlur" -A 2 -B 2
Length of output: 146440
packages/base/src/select/select.tsx (1)
1-1
: 表单字段上下文集成正确实现!
通过 FormFieldContext 集成表单字段上下文的改动符合最佳实践:
- 正确导入并使用 FormFieldContext
- fieldId 被恰当地应用于结果包装器的 div 元素
- 这些更改增强了表单的可访问性和验证能力
Also applies to: 29-29, 542-542, 590-590
<Input name={'email'} clearable placeholder='please input email' /> | ||
</Form.Item> | ||
<Form.Item label='Gendar'> | ||
<Radio.Group name='gendar' data={['male', 'female']} keygen /> |
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.
修正名称属性中的拼写错误
在第43行,name='gendar'
中存在拼写错误,应为 'gender'
,以保持与标签的一致性。
请应用以下修改以修正名称属性:
- <Radio.Group name='gendar' data={['male', 'female']} keygen />
+ <Radio.Group name='gender' data={['male', 'female']} keygen />
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Radio.Group name='gendar' data={['male', 'female']} keygen /> | |
<Radio.Group name='gender' data={['male', 'female']} keygen /> |
<Form.Item label='Email'> | ||
<Input name={'email'} clearable placeholder='please input email' /> | ||
</Form.Item> | ||
<Form.Item label='Gendar'> |
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.
修正标签中的拼写错误
在第42行,label='Gendar'
中存在拼写错误,应为 'Gender'
。
请应用以下修改以修正拼写错误:
- <Form.Item label='Gendar'>
+ <Form.Item label='Gender'>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Form.Item label='Gendar'> | |
<Form.Item label='Gender'> |
username: '测试0719', | ||
gender: 2, | ||
bodyTypeName: null, | ||
email: 'willow_kuangqew11232@qq_test.com', |
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.
代码中包含真实的用户信息,存在敏感信息泄露风险
代码中包含了真实的用户名和邮箱等敏感信息。为了保护用户隐私和数据安全,建议使用虚构的测试数据替换所有敏感信息。
updatePartnerRequest: { | ||
partnerId: '2296', | ||
bodyType: '202', | ||
cooperateFee: '', |
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.
cooperateFee
字段类型不一致
在第63行,cooperateFee
的值为数字 0
,而在第167行,cooperateFee
的值为空字符串 ''
。为了保持数据一致性,建议统一 cooperateFee
字段的类型。
{ brandName: 'ROMWE', seriesLineId: 301, seriesLineName: null }, | ||
{ brandName: 'ROMWE', seriesLineId: 301, seriesLineName: null }, |
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.
🛠️ Refactor suggestion
seriesLine
数组中存在重复的数据
在 seriesLine
数组中,第403行和第404行都包含了 seriesLineId: 301
的数据。这会导致数据冗余和潜在的处理错误。建议删除重复的数据。
@@ -162,6 +167,7 @@ const Result = (props: ResultProps) => { | |||
|
|||
const placeholderArr = getPlaceHolderArr(); | |||
|
|||
const formFieldId = fieldId.split('__separator__'); |
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.
建议增强fieldId的处理逻辑
当前的fieldId处理存在以下潜在问题:
- 没有验证分割后的数组长度
- 可能存在空字符串的情况
- 在range模式下可能出现索引越界
建议增加相关的验证和错误处理:
-const formFieldId = fieldId.split('__separator__');
+const formFieldId = fieldId?.split('__separator__') || [];
+const getFieldId = (index: number) => {
+ if (!formFieldId.length) return undefined;
+ return formFieldId[index] || `${formFieldId[0]}_${index}`;
+};
// 在Input组件中使用
-id={formFieldId[info.index]}
+id={getFieldId(info.index)}
Also applies to: 220-220
const { fieldId } = useContext(FormFieldContext); | ||
|
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.
🛠️ Refactor suggestion
建议增加上下文值的空值处理
当前直接解构fieldId
可能在上下文不存在时导致运行时错误。建议添加默认值或错误处理:
-const { fieldId } = useContext(FormFieldContext);
+const { fieldId = '' } = useContext(FormFieldContext);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const { fieldId } = useContext(FormFieldContext); | |
const { fieldId = '' } = useContext(FormFieldContext); | |
onClick={() => { | ||
form | ||
.validate() | ||
.then((values) => { | ||
console.log('======================'); | ||
console.log('validate success values: >>', values); | ||
console.log('======================'); | ||
}) | ||
.catch((errorInfo) => { | ||
console.log('======================'); | ||
console.log('validate failed errorInfo: >>', errorInfo); | ||
console.log('validate failed errorInfo.message: >>', errorInfo.message); | ||
console.log('======================'); | ||
}); | ||
}} |
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.
🛠️ Refactor suggestion
建议优化表单验证的错误处理方式
当前代码中使用了大量的 console.log
来输出验证结果,建议:
- 在生产环境中移除 console.log
- 添加用户友好的错误提示
- 实现统一的错误处理机制
onClick={() => {
form
.validate()
.then((values) => {
- console.log('======================');
- console.log('validate success values: >>', values);
- console.log('======================');
+ // 成功后的业务处理
+ message.success('验证通过');
})
.catch((errorInfo) => {
- console.log('======================');
- console.log('validate failed errorInfo: >>', errorInfo);
- console.log('validate failed errorInfo.message: >>', errorInfo.message);
- console.log('======================');
+ message.error(errorInfo.message);
+ // 可选:自动滚动到第一个错误字段
+ if (errorInfo.fields?.[0]?.name) {
+ form.scrollToField(errorInfo.fields[0].name);
+ }
});
}}
Committable suggestion skipped: line range outside the PR's diff.
import { Form, Input, Button, Tabs, Grid, Select, DatePicker } from 'shineout'; | ||
import testFromValue from './temp/test-form-value'; | ||
|
||
type ValueType = any; |
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.
🛠️ Refactor suggestion
建议使用更具体的类型定义替代 any
使用 any
类型会降低代码的类型安全性,建议定义一个具体的接口来描述表单数据结构。
-type ValueType = any;
+interface ValueType {
+ request: {
+ startDate2: string;
+ endDate2: string;
+ commonPartnerUpsertReq: {
+ updatePartnerRequest: {
+ partnerId: string;
+ bodyType: string;
+ };
+ labelRuleInfo: Array<{
+ fieldId: number;
+ ruleFieldId: number;
+ ruleEnumList: number[];
+ }>;
+ };
+ brandPartnerUpsertReq: Array<{
+ brandId: string;
+ labelBuildRequest: {
+ busId: string;
+ moduleId: string;
+ originalPartnerId: string;
+ };
+ updateBrandPartnerRequest: {
+ liveExperience: string;
+ childBrand: number;
+ originalPartnerId: string;
+ };
+ }>;
+ };
+}
Committable suggestion skipped: line range outside the PR's diff.
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: 1
🧹 Outside diff range and nitpick comments (4)
packages/shineout/src/form/__example__/002-control.tsx (2)
8-16
: 建议将 SVG 图标移至单独的文件为了提高代码的可维护性和复用性,建议:
- 将 SVG 图标代码移至专门的图标文件夹
- 考虑使用图标组件库来统一管理 SVG 图标
建议重构如下:
+ // src/icons/star.tsx + export const StarIcon = () => ( + <svg viewBox='0 0 24 24' fill='currentColor' xmlns='http://www.w3.org/2000/svg'> + <path d='M8.276 7.825L1.85649 8.7559L1.74278 8.77878C1.00761 8.96968 0.736859 9.90915 1.30093 10.4606L5.953 15.008L4.84231 21.3268L4.82911 21.4327C4.77288 22.2003 5.59415 22.7575 6.29763 22.3824L11.999 19.343L17.7023 22.3825L17.7988 22.4279C18.5105 22.7194 19.2948 22.1128 19.1578 21.3281L18.054 15.008L22.6997 10.46L22.7779 10.3745C23.2586 9.78723 22.9242 8.86973 22.1443 8.75601L15.758 7.825L12.895 2.05544C12.5264 1.31273 11.4661 1.31545 11.1013 2.06004L8.276 7.825Z' /> + </svg> + ); // 当前文件 - const star = ( - <svg viewBox='0 0 24 24' fill='currentColor' xmlns='http://www.w3.org/2000/svg'> - <path d='M8.276 7.825L1.85649 8.7559L1.74278 8.77878C1.00761 8.96968 0.736859 9.90915 1.30093 10.4606L5.953 15.008L4.84231 21.3268L4.82911 21.4327C4.77288 22.2003 5.59415 22.7575 6.29763 22.3824L11.999 19.343L17.7023 22.3825L17.7988 22.4279C18.5105 22.7194 19.2948 22.1128 19.1578 21.3281L18.054 15.008L22.6997 10.46L22.7779 10.3745C23.2586 9.78723 22.9242 8.86973 22.1443 8.75601L15.758 7.825L12.895 2.05544C12.5264 1.31273 11.4661 1.31545 11.1013 2.06004L8.276 7.825Z' /> - </svg> - ); + import { StarIcon } from '../icons/star'; - const StarRate = Rate(star, star); + const StarRate = Rate(StarIcon, StarIcon);
98-98
: 建议添加日期验证规则为了确保日期选择的合理性,建议添加以下验证规则:
- 开始日期不能晚于结束日期
- 可以设置最小/最大可选日期范围
建议修改如下:
- <DatePicker range name={['startDate', 'endDate']} /> + <DatePicker + range + name={['startDate', 'endDate']} + rules={[ + { + validator: (rule, value) => { + if (value.startDate && value.endDate && value.startDate > value.endDate) { + return new Error('开始日期不能晚于结束日期'); + } + return true; + } + } + ]} + min={new Date()} + />packages/hooks/src/components/use-form/use-form.ts (2)
220-256
: 新增的 scrollToField 方法实现需要考虑边界情况
scrollToField
方法的实现逻辑完善,但建议:
- 考虑添加防抖处理,避免频繁调用
- 添加滚动完成的回调函数
- 处理元素不在 DOM 中的情况
建议按如下方式优化实现:
const scrollToField = usePersistFn( - (name: string, scrollIntoViewOptions: ScrollIntoViewOptions = {}) => { + ( + name: string, + scrollIntoViewOptions: ScrollIntoViewOptions = {}, + callback?: () => void + ) => { if (!name) return; const fieldId = getFieldId(name, formName); if (!fieldId) return; + // 使用 requestAnimationFrame 优化性能 + requestAnimationFrame(() => { const element = document?.getElementById(fieldId); if (element) { const parentEl = getClosestScrollContainer(element); if (parentEl) { // ... existing scroll logic ... + callback?.(); } else { element.scrollIntoView({ behavior: "smooth", ...scrollIntoViewOptions, + // 添加滚动结束监听 + onscrollend: callback, }); } } else { console.warn(`[shineout] fieldId: ${fieldId} not found`); + callback?.(); } + }); } );
523-523
: formName 的添加增强了表单的上下文管理将
formName
添加到formConfig
中有助于在复杂表单场景下的状态管理,这是一个很好的改进。建议添加相关文档说明其用途和最佳实践。Also applies to: 534-534
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (4)
packages/shineout/src/card/__test__/__snapshots__/card.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/drawer/__test__/__snapshots__/drawer.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/input/__test__/__snapshots__/input.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/upload/__test__/__snapshots__/upload.spec.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (5)
packages/base/src/date-picker/result.tsx
(3 hunks)packages/base/src/form/form-field-context.ts
(1 hunks)packages/hooks/src/components/use-form/use-form.ts
(11 hunks)packages/hooks/src/utils/dom/element.tsx
(1 hunks)packages/shineout/src/form/__example__/002-control.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/base/src/date-picker/result.tsx
- packages/base/src/form/form-field-context.ts
- packages/hooks/src/utils/dom/element.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/hooks/src/components/use-form/use-form.ts
[error] 183-183: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (3)
packages/hooks/src/components/use-form/use-form.ts (3)
23-24
: 新增的工具函数和类型定义增强了表单的功能
新增的 getFieldId
和 getClosestScrollContainer
工具函数以及 ValidateFnConfig
和 ValidationError
类型定义,为表单组件提供了更完善的字段定位和错误处理能力。
Also applies to: 36-37
444-447
: 优化 unbind 方法中的变量命名
变量命名 validateField
更准确地表达了其用途,这是一个好的改进。建议继续优化其他相关变量的命名以保持一致性。
181-187
:
建议优化 reduce 中的展开运算符使用
在 reduce
中使用展开运算符会导致时间复杂度从 O(n) 增加到 O(n^2),建议使用更高效的方式重构此段代码。
-validatorValue = fieldArray.reduce(
- (prev, cur) => ({
- ...prev,
- [cur]: getValue(cur),
- }),
- {},
-);
+validatorValue = {};
+fieldArray.forEach((cur) => {
+ validatorValue[cur] = getValue(cur);
+});
Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 183-183: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
form.current?.validate() | ||
.then((values) => { | ||
console.log('validate success', values); | ||
}) | ||
.catch((errorInfo) => { | ||
console.log('validate failed errorInfo: >>', errorInfo) |
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.
移除调试用的 console.log 语句
在生产环境代码中不应该保留 console.log 调试语句。建议:
- 使用正式的日志系统记录验证结果
- 添加适当的错误处理逻辑
建议修改如下:
form.current?.validate()
.then((values) => {
- console.log('validate success', values);
+ // TODO: 添加成功处理逻辑
})
.catch((errorInfo) => {
- console.log('validate failed errorInfo: >>', errorInfo)
+ // TODO: 添加错误处理逻辑
})
Committable suggestion skipped: line range outside the PR's diff.
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
🧹 Outside diff range and nitpick comments (1)
packages/shineout/src/tabs/__test__/tabs.spec.tsx (1)
Line range hint
351-363
: 测试用例需要进一步完善当前测试用例验证了非面板子组件的渲染行为,但建议增加以下改进:
- 需要验证
allowNonPanel
属性的行为- 缺少对子组件渲染顺序的验证
- 建议添加边界情况的测试,如混合使用 Panel 和非 Panel 子组件的场景
建议按照以下方式重构测试用例:
test('should not render when set children is not panel', () => { const { container } = render( <Tabs> <div className='demo'>demoA</div> <div className='demo'>demoB</div> </Tabs>, ); classLengthTest(container, tabsTabClassName, 0); - container.querySelectorAll(tabsTabClassName).forEach((item, index) => { - if (index === 0) { - attributesTest(item, 'data-soui-state', 'active'); - return - } - attributesTest(item, 'data-soui-state', ''); - }); - classLengthTest(container, '.demo', 0); - container.querySelectorAll('.demo').forEach((item, index) => { - attributesTest(item, 'id', String(index)); - }); - classLengthTest(container, tabsPanelClassName, 0); + // 验证默认行为 + expect(container.querySelectorAll('.demo')).toHaveLength(0); + expect(container.querySelectorAll(tabsPanelClassName)).toHaveLength(0); + + // 验证 allowNonPanel 属性 + const { container: containerWithAllowNonPanel } = render( + <Tabs allowNonPanel> + <div className='demo'>demoA</div> + <Tabs.Panel tab='tab'>panel content</Tabs.Panel> + <div className='demo'>demoB</div> + </Tabs>, + ); + + expect(containerWithAllowNonPanel.querySelectorAll('.demo')).toHaveLength(2); + expect(containerWithAllowNonPanel.querySelectorAll(tabsPanelClassName)).toHaveLength(1); + + // 验证渲染顺序 + const children = containerWithAllowNonPanel.querySelector(tabsPanelWrapperClassName)!.children; + expect(children[0]).toHaveClass('demo'); + expect(children[1]).toHaveClass(tabsPanelClassName.slice(1)); + expect(children[2]).toHaveClass('demo'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/base/src/tabs/tabs.tsx
(4 hunks)packages/base/src/tabs/tabs.type.ts
(1 hunks)packages/shineout/src/form/__example__/006-validate-05-scrolltoerror.tsx
(1 hunks)packages/shineout/src/tabs/__doc__/changelog.cn.md
(1 hunks)packages/shineout/src/tabs/__test__/tabs.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/base/src/tabs/tabs.tsx
- packages/shineout/src/form/example/006-validate-05-scrolltoerror.tsx
- packages/shineout/src/tabs/doc/changelog.cn.md
🔇 Additional comments (1)
packages/base/src/tabs/tabs.type.ts (1)
167-172
: 类型定义更改看起来不错!
新增的 allowNonPanel
属性定义清晰,文档完整,与PR目标保持一致。
让我们验证一下组件实现是否正确处理了这个新属性:
✅ Verification successful
实现验证完成,代码逻辑正确!
代码中 allowNonPanel
属性的实现符合预期:
- 在渲染子元素时,首先检查是否为
ShineoutTabsPanel
组件 - 如果启用了
allowNonPanel
,则允许渲染非面板类型的子元素 - 实现简洁明确,完全满足需求
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证Tabs组件中allowNonPanel属性的实现
# 检查Tabs组件实现文件中的相关逻辑
ast-grep --pattern 'allowNonPanel' packages/base/src/tabs/tabs.tsx
Length of output: 132
Script:
#!/bin/bash
# 获取更多上下文来理解allowNonPanel的实现逻辑
ast-grep --pattern $'if(allowNonPanel) {
$$$
}' packages/base/src/tabs/tabs.tsx
Length of output: 257
Script:
#!/bin/bash
# 获取完整的上下文来理解allowNonPanel的使用场景
rg -B 10 -A 5 "if\(allowNonPanel\)" packages/base/src/tabs/tabs.tsx
Length of output: 636
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: 7
🧹 Outside diff range and nitpick comments (13)
packages/hooks/src/components/use-form/use-form-context.type.ts (2)
1-9
: 建议明确注释掉的类型定义的用途这段被注释的
NestedKeyOf
类型定义看起来可以用于生成表单字段的类型安全路径。建议:
- 如果计划在未来使用,请添加 TODO 注释说明用途和计划
- 如果不再需要,建议删除以保持代码整洁
26-30
: 建议对新增方法进行优化关于新增的方法有以下建议:
validateFieldsWithValue
方法:
- 建议返回类型使用泛型
Promise<Value>
而不是Promise<any>
,保持与validate
方法一致的类型安全性
scrollToField
方法:
- 建议补充
scrollIntoViewOptions
参数的具体配置说明- 建议添加示例代码说明用法
Also applies to: 56-60
packages/base/src/form/form.type.ts (2)
6-6
: 建议统一使用 KeyType 类型新增的
KeyType
类型定义更加灵活,支持string
、number
和symbol
类型的字段名称。建议在整个接口中统一使用这个类型,特别是在FormRef
接口的方法参数中。以下方法的参数类型建议更新为
KeyType
:
getValue
validateFields
validateFieldsWithError
scrollToField
-getValue: (name?: string) => any | FormValue; +getValue: (name?: KeyType) => any | FormValue; -validateFields: (fields: string | string[]) => Promise<any>; +validateFields: (fields: KeyType | KeyType[]) => Promise<any>; -validateFieldsWithError: (fields: string | string[]) => Promise<any>; +validateFieldsWithError: (fields: KeyType | KeyType[]) => Promise<any>; -scrollToField: (name: string, scrollIntoViewOptions?: ScrollIntoViewOptions) => void; +scrollToField: (name: KeyType, scrollIntoViewOptions?: ScrollIntoViewOptions) => void;
12-22
: 建议增强类型安全性并完善文档
FormValidateFn
接口的实现有以下几点建议:
fields
参数类型可以更具体- 注释掉的泛型参数建议either完全移除或实现
- 建议补充更详细的返回值文档说明
建议修改如下:
export interface FormValidateFn<FormValue> { /** * 验证所有表单的值,并且返回报错和表单数据 * @param fields 需要校验的表单字段 + * @returns Promise<FormValue> 返回校验成功后的表单数据 */ - (fields?: string | string[]): Promise<FormValue>; + (fields?: KeyType | KeyType[]): Promise<FormValue>; }packages/shineout/src/form/__doc__/changelog.cn.md (1)
1-6
: 文档内容清晰,建议细化使用说明changelog 记录完整且符合规范。建议为新增的两个方法补充具体的使用示例,以帮助开发者更好地理解这些功能。
建议在文档中添加以下使用示例:
- 增强 `Form` 的 `formRef`,增加 `validateFieldsWithValue` 方法,返回校验值 ([#812](https://github.com/sheinsight/shineout-next/pull/812)) + 示例: + ```tsx + const values = await formRef.current.validateFieldsWithValue(); + console.log(values); // 输出校验后的表单值 + ``` - 增强 `Form` 的 `formRef`,增加 `scrollToField` 方法,支持根据 name 滚动至指定表单项 ([#812](https://github.com/sheinsight/shineout-next/pull/812)) + 示例: + ```tsx + formRef.current.scrollToField('fieldName'); + ```packages/hooks/src/components/use-form/use-form.type.ts (2)
7-15
: 类型定义清晰且符合最佳实践!类型定义遵循了业界标准实践,特别是
ValidationError
接口的设计与 antd 等主流组件库保持一致,有利于开发者快速理解和使用。建议为ValidationError
接口添加更详细的 JSDoc 注释,说明各字段的具体用途。export type KeyType = string | number | symbol; +/** + * 表单验证结果接口 + * @template T - 表单值的类型 + * @property values - 表单的当前值 + * @property errorFields - 验证失败的字段列表 + */ export interface ValidationError<T> { values: T; errorFields: { name: string; errors: string[]; }[]; }
17-32
: 需要补充验证类型的详细说明
ValidateFnConfig
中的type
字段引入了'forcePass'
和'withValue'
两个验证类型,但缺少对这些类型的具体说明。建议添加详细的文档注释,解释每种验证类型的使用场景和行为。export type ValidateFnConfig = { + /** + * 验证类型 + * @property forcePass - 强制通过验证 + * @property withValue - 返回验证后的表单值 + */ type?: 'forcePass' | 'withValue', ignoreBind?: boolean, }packages/shineout/src/form/__example__/006-validate-05-scrolltofield.tsx (2)
10-40
: 建议为类型定义添加文档注释为了提高代码的可维护性和可读性,建议为
ChildArrayType
和ValueType
添加 JSDoc 文档注释,说明每个字段的用途和约束条件。+/** + * 子配置数组类型 + * @property brandName - 品牌名称 + * @property configs - 配置数组 + * @property childConfig - 子配置对象 + */ type ChildArrayType = { // ... existing code } +/** + * 表单值类型 + * @property public - 公共配置 + * @property child - 子配置数组 + */ type ValueType = { // ... existing code }
95-104
: 建议优化状态管理结构当前实现使用了多个独立的
useState
来管理相关的状态。建议将相关的状态合并到一个对象中,使用单个useState
管理,这样可以减少重渲染次数并提高代码的可维护性。-const [activeParentTab, setActiveParentTab] = useState('public-panel'); -const [activeChildTab, setActiveChildTab] = useState('child-panel-0'); +const [tabState, setTabState] = useState({ + parentTab: 'public-panel', + childTab: 'child-panel-0', +}); -const [field1, setField1] = useState<string | undefined>(); -const [field2, setField2] = useState<string | undefined>(); +const [fields, setFields] = useState({ + field1: undefined as string | undefined, + field2: undefined as string | undefined, +});packages/hooks/src/components/use-form/use-form.ts (4)
159-164
: getValue 方法实现优化getValue 方法的实现简洁明了,但建议添加参数类型注解以提高代码的可维护性。
- const getValue = usePersistFn((name?: string) => { + const getValue = usePersistFn((name?: string): any => {
231-267
: 新增 scrollToField 实现的建议scrollToField 的实现比较完善,但有以下几点建议:
- 建议添加防抖处理,避免频繁调用导致的性能问题
- 建议将滚动行为的配置项统一到组件的 props 中
+ const debouncedScrollToField = usePersistFn( + debounce((name: string, scrollIntoViewOptions: ScrollIntoViewOptions = {}) => { // 原有的 scrollToField 实现 + }, 300) + );
263-264
: 建议优化错误提示信息TODO 注释提到需要统一警告信息,建议实现一个统一的错误处理机制。
需要我帮助实现一个统一的错误处理机制吗?可以创建一个新的 issue 来跟踪这项工作。
359-359
: 建议优化空值处理使用可选链操作符处理空值是好的实践,但建议添加类型断言来确保类型安全。
- props.onSubmit?.((context.value ?? {}) as T); + props.onSubmit?.(((context.value ?? {}) as T) || ({} as T));Also applies to: 364-364
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
packages/base/src/form/form.tsx
(4 hunks)packages/base/src/form/form.type.ts
(2 hunks)packages/hooks/src/components/use-form/use-form-context.type.ts
(3 hunks)packages/hooks/src/components/use-form/use-form.ts
(12 hunks)packages/hooks/src/components/use-form/use-form.type.ts
(4 hunks)packages/shineout/src/form/__doc__/changelog.cn.md
(1 hunks)packages/shineout/src/form/__example__/006-validate-05-scrolltofield.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/base/src/form/form.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/hooks/src/components/use-form/use-form.ts
[error] 185-185: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (6)
packages/hooks/src/components/use-form/use-form-context.type.ts (1)
20-20
: 类型安全性提升,符合预期
validate
方法的改进:
- 返回类型从
Promise<any>
改为Promise<Value>
,提高了类型安全性 - 新增可选的
fields
参数,增加了校验特定字段的灵活性
packages/shineout/src/form/__doc__/changelog.cn.md (1)
Line range hint 14-24
: 建议验证新功能与现有特性的兼容性
新增的 validateFieldsWithValue
和 scrollToField
方法需要与 3.5.0 版本引入的 useForm
和 reserveAble
特性保持一致。
✅ Verification successful
新功能与现有特性集成良好,无需担心兼容性问题
从代码分析结果可以看到:
-
validateFieldsWithValue
方法已经正确集成到 Form 组件中:- 在
packages/base/src/form/form.tsx
中实现 - 通过
useForm
hook 暴露给外部使用 - 与现有的表单验证流程保持一致
- 在
-
与 3.5.0 版本引入的功能完全兼容:
useForm
正常支持新增的validateFieldsWithValue
方法reserveAble
属性在表单控件中得到正确处理
-
实现方式符合最佳实践:
- 复用了现有的表单验证基础设施
- 保持了与其他验证方法一致的错误处理机制
- 维持了统一的上下文管理方式
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 useForm 和新方法的集成实现
ast-grep --pattern $'const useForm = () => {
$$$
validateFieldsWithValue($$$) {
$$$
}
$$$
}'
Length of output: 105
Script:
#!/bin/bash
# 搜索 useForm 相关实现
rg "useForm" -A 10
# 搜索 validateFieldsWithValue 方法的实现
rg "validateFieldsWithValue" -A 5
# 搜索 Form 组件中的新方法实现
ast-grep --pattern 'function Form {
$$$
validateFieldsWithValue($$$) {
$$$
}
$$$
}'
# 检查 Form.tsx 文件中的相关实现
fd -e tsx -e ts "Form.tsx" --exec cat {}
Length of output: 50938
packages/hooks/src/components/use-form/use-form.type.ts (2)
124-130
: 表单无障碍功能增强,实现得当!
通过添加 formName
属性来支持表单字段 ID 前缀,很好地增强了表单的无障碍性能,使得 <label for="id">
功能可以正常工作。文档注释完整,版本信息清晰。
148-148
: 类型定义优化合理!
将 validateFields
的类型更新为 ValidateFieldsFn
,提供了更好的类型安全性和灵活性,同时与新的验证结构保持一致。
packages/shineout/src/form/__example__/006-validate-05-scrolltofield.tsx (1)
168-290
: 建议验证表单验证规则的一致性
需要确保所有必填字段的验证规则是一致的,并且错误消息格式统一。
✅ Verification successful
表单验证规则已经保持一致性
根据代码分析,当前表单中的验证规则实现具有以下特点:
- 所有必填字段都使用了统一的
required: true
规则 - 错误消息格式保持一致,清晰地指明了字段名称和"is required"的提示
- 验证规则的结构统一使用
rules={[{ required: true, message: '...' }]}
的格式
表单验证规则的实现符合最佳实践,无需进行修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查所有表单验证规则的一致性
echo "检查表单验证规则..."
# 搜索所有的验证规则定义
rg "rules=\[\{.*required.*\}\]" --type typescript
# 搜索所有的错误消息格式
rg "message:.*required" --type typescript
Length of output: 287
Script:
#!/bin/bash
# 检查所有表单验证规则的一致性
echo "检查表单验证规则..."
# 搜索所有的验证规则定义
rg "rules=\[\{" -A 1
# 搜索所有的错误消息格式
rg "message:" -A 1
Length of output: 21810
packages/hooks/src/components/use-form/use-form.ts (1)
23-25
: 新增类型和工具函数导入
新增的工具函数和类型定义增强了表单的功能性,特别是在字段定位和错误处理方面。
Also applies to: 37-38
* @en Validation form fields and return the value | ||
* @cn 校验表单指定字段并返回值, 也可以通过 catch 获取报错信息 | ||
*/ | ||
validateFieldsWithValue: (fields?: (string | keyof FormValue) | (string | keyof FormValue)[]) => Promise<any>; |
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.
🛠️ Refactor suggestion
建议优化 validateFieldsWithValue 方法的参数类型
validateFieldsWithValue
方法的参数类型定义较为复杂且不一致:
(fields?: (string | keyof FormValue) | (string | keyof FormValue)[])
建议简化并统一类型定义:
-validateFieldsWithValue: (fields?: (string | keyof FormValue) | (string | keyof FormValue)[]) => Promise<any>;
+validateFieldsWithValue: (fields?: KeyType | KeyType[]) => Promise<FormValue>;
validate: FormValidateFn<FormValue>; | ||
/** | ||
* @en Validation form fields | ||
* @cn 校验表单指定字段 | ||
*/ | ||
validateFields: (fields: string | string[]) => Promise<any>; | ||
|
||
/** | ||
* @en Validation form fields and return the value | ||
* @cn 校验表单指定字段并返回值, 也可以通过 catch 获取报错信息 | ||
*/ | ||
validateFieldsWithValue: (fields?: (string | keyof FormValue) | (string | keyof FormValue)[]) => Promise<any>; |
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.
🛠️ Refactor suggestion
建议统一并优化验证方法的返回类型
当前多个验证相关方法返回 Promise<any>
,这降低了类型安全性。建议统一使用更具体的返回类型。
建议修改如下:
interface ValidationResult {
errors: Record<string, string[]> | null;
values: FormValue;
}
-validate: FormValidateFn<FormValue>;
+validate: () => Promise<ValidationResult>;
-validateFields: (fields: string | string[]) => Promise<any>;
+validateFields: (fields: KeyType | KeyType[]) => Promise<ValidationResult>;
-validateFieldsWithError: (fields: string | string[]) => Promise<any>;
+validateFieldsWithError: (fields: KeyType | KeyType[]) => Promise<ValidationResult>;
Committable suggestion skipped: line range outside the PR's diff.
} | ||
|
||
export type UseFormProps<T> = BaseFormProps<T>; | ||
|
||
export type FormContext = { | ||
defaultValues: ObjectType; | ||
validateMap: ObjectType<Set<(name: string, v: any, formValue: ObjectType, config?: { ignoreBind?: boolean }) => Promise<boolean | FormError>>>; | ||
validateMap: ObjectType<Set<(name: string, v: any, formValue: ObjectType, config?: { ignoreBind?: boolean }) => Promise<true | FormError>>>; |
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.
将 validateMap
的返回类型从 Promise<boolean | FormError>
改为 Promise<true | FormError>
是一个破坏性改动。虽然这个改动使得成功情况更加明确,但可能会影响现有代码。
建议:
- 在 CHANGELOG 中明确标注这个改动
- 在升级指南中提供迁移说明
- 考虑增加版本号的 major 版本
<Form.Item label={`child[${index}].configs[${nestIndex}].bussName`} required> | ||
<Input | ||
name={'bussName'} | ||
rules={[{ required: true, message: 'bussName is required' }]} | ||
/> | ||
</Form.Item> | ||
</Grid> | ||
<Grid width={1 / 3}> | ||
<Form.Item label={`child[${index}].configs[${nestIndex}].mockId`} required> | ||
<Input | ||
name={'mockId'} | ||
rules={[{ required: true, message: 'mockId is required' }]} | ||
/> | ||
</Form.Item> | ||
</Grid> | ||
<Grid width={1 / 3}> | ||
<Form.Item label={`child[${index}].configs[${nestIndex}].slotId`} required> | ||
<Input | ||
name={'slotId'} | ||
rules={[{ required: true, message: 'slotId is required' }]} | ||
/> | ||
</Form.Item> | ||
</Grid> | ||
</Grid> | ||
)} | ||
</Form.FieldSet> | ||
<Form.FieldSet name='childConfig'> | ||
<Form.Item label={`child[${index}].childConfig.live`} required> | ||
<Input | ||
name={'live'} | ||
rules={[{ required: true, message: 'live is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={`child[${index}].childConfig.brand`} required> | ||
<Select | ||
keygen | ||
name='brand' | ||
data={[1, 2, 3]} | ||
rules={[{ required: true, message: 'brand is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={`child[${index}].childConfig.partner`} required> | ||
<Input | ||
name={'partner'} | ||
rules={[{ required: true, message: 'partner is required' }]} | ||
/> | ||
</Form.Item> | ||
</Form.FieldSet> | ||
</Tabs.Panel> | ||
); | ||
}} | ||
</Form.FieldSet> | ||
</Tabs> | ||
</Tabs.Panel> | ||
</Tabs> | ||
|
||
<Grid style={{ marginTop: 12 }}> | ||
<Grid width={2 / 3} style={{ display: 'inline-flex', gap: 12 }}> | ||
<Input.Group> | ||
<Input placeholder='input field name' value={field1} onChange={setField1} /> | ||
<Button | ||
onClick={() => { | ||
form | ||
.validateFieldsWithValue(field1) | ||
.then((values) => { | ||
Message.success(`${field1} validate success`); | ||
console.log('validate success values: >>', values); | ||
}) | ||
.catch((errorInfo) => { | ||
Message.error('validate failed'); | ||
console.log('validate failed errorInfo: >>', errorInfo); | ||
}); | ||
}} | ||
> | ||
form.validateFieldsWithValue | ||
</Button> | ||
</Input.Group> | ||
|
||
<Input.Group> | ||
<Input placeholder='input field name' value={field2} onChange={setField2} /> | ||
<Button | ||
onClick={() => { | ||
if (field2) { | ||
myCustomScrollToField(field2); | ||
} | ||
}} | ||
> | ||
form.scrollToField | ||
</Button> | ||
</Input.Group> | ||
</Grid> | ||
|
||
<Grid width={1 / 3} style={{ textAlign: 'right' }}> | ||
<Form.Reset>Reset</Form.Reset> | ||
<Button type='primary' onClick={handleMySubmit}> | ||
My Submit | ||
</Button> | ||
</Grid> | ||
</Grid> | ||
</Form> |
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.
🛠️ Refactor suggestion
建议提升表单的可维护性和可访问性
当前表单结构较为复杂,建议进行以下优化:
- 将重复的表单项抽取为独立组件
- 添加 ARIA 属性以提升可访问性
- 优化表单验证规则的复用
建议将重复的表单项抽取为组件,例如:
interface ConfigFieldProps {
prefix: string;
index?: number;
}
const ConfigFields: React.FC<ConfigFieldProps> = ({ prefix, index }) => (
<Form.FieldSet name='config1'>
{['field1', 'field2', 'field3', 'field4'].map((field) => (
<Form.Item
key={field}
label={`${prefix}.${field}`}
required
>
<Input
name={field}
rules={[{ required: true, message: `${prefix}.${field} is required` }]}
aria-label={`${prefix}.${field}`}
/>
</Form.Item>
))}
</Form.FieldSet>
);
同时建议添加必要的 ARIA 属性:
<Tabs
active={activeParentTab}
onChange={(tab) => setActiveParentTab(tab as string)}
lazy={false}
+ role="tablist"
+ aria-label="表单配置"
>
const handleMySubmit = () => { | ||
form | ||
.validateFieldsWithValue() | ||
.then((v) => { | ||
Message.success('submit success'); | ||
|
||
console.log('submit success:>>', JSON.stringify(v, null, 2)); | ||
form.submit(); | ||
}) | ||
.catch((errorInfo) => { | ||
Message.error('submit failed'); | ||
|
||
console.log('submit failed:>>', errorInfo); | ||
if (errorInfo.errorFields.length > 0) { | ||
const firstErrorFieldName = errorInfo.errorFields[0].name as string; | ||
console.log('firstErrorFieldName: >>', firstErrorFieldName); | ||
|
||
// 滚动到错误字段 | ||
myCustomScrollToField(firstErrorFieldName); | ||
} | ||
}); | ||
}; |
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.
🛠️ Refactor suggestion
建议优化表单提交的错误处理
当前的错误处理较为简单,建议增加更详细的错误信息展示,并对不同类型的错误进行分类处理。
const handleMySubmit = () => {
form
.validateFieldsWithValue()
.then((v) => {
- Message.success('submit success');
+ Message.success('表单提交成功');
console.log('submit success:>>', JSON.stringify(v, null, 2));
form.submit();
})
.catch((errorInfo) => {
- Message.error('submit failed');
+ const errorCount = errorInfo.errorFields.length;
+ Message.error(`表单验证失败:存在 ${errorCount} 个字段错误`);
console.log('submit failed:>>', errorInfo);
if (errorInfo.errorFields.length > 0) {
const firstErrorFieldName = errorInfo.errorFields[0].name as string;
+ const errorMessage = errorInfo.errorFields[0].errors.join(', ');
+ Message.warning(`正在定位到错误字段: ${firstErrorFieldName}`);
console.log('firstErrorFieldName: >>', firstErrorFieldName);
myCustomScrollToField(firstErrorFieldName);
}
});
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const handleMySubmit = () => { | |
form | |
.validateFieldsWithValue() | |
.then((v) => { | |
Message.success('submit success'); | |
console.log('submit success:>>', JSON.stringify(v, null, 2)); | |
form.submit(); | |
}) | |
.catch((errorInfo) => { | |
Message.error('submit failed'); | |
console.log('submit failed:>>', errorInfo); | |
if (errorInfo.errorFields.length > 0) { | |
const firstErrorFieldName = errorInfo.errorFields[0].name as string; | |
console.log('firstErrorFieldName: >>', firstErrorFieldName); | |
// 滚动到错误字段 | |
myCustomScrollToField(firstErrorFieldName); | |
} | |
}); | |
}; | |
const handleMySubmit = () => { | |
form | |
.validateFieldsWithValue() | |
.then((v) => { | |
Message.success('表单提交成功'); | |
console.log('submit success:>>', JSON.stringify(v, null, 2)); | |
form.submit(); | |
}) | |
.catch((errorInfo) => { | |
const errorCount = errorInfo.errorFields.length; | |
Message.error(`表单验证失败:存在 ${errorCount} 个字段错误`); | |
console.log('submit failed:>>', errorInfo); | |
if (errorInfo.errorFields.length > 0) { | |
const firstErrorFieldName = errorInfo.errorFields[0].name as string; | |
const errorMessage = errorInfo.errorFields[0].errors.join(', '); | |
Message.warning(`正在定位到错误字段: ${firstErrorFieldName}`); | |
console.log('firstErrorFieldName: >>', firstErrorFieldName); | |
myCustomScrollToField(firstErrorFieldName); | |
} | |
}); | |
}; |
const myCustomScrollToField = (fieldName: string) => { | ||
// 示例这里根据字段名判断模拟交互,开发者以实际情况处理 | ||
if (fieldName.startsWith('public')) { | ||
setActiveParentTab('public-panel'); | ||
} else { | ||
// 切换至child-panel | ||
setActiveParentTab('child-panel'); | ||
|
||
const index = fieldName.match(/\d+/)?.[0]; | ||
// 切换至对应的child-panel-index | ||
setActiveChildTab(`child-panel-${index}`); | ||
} | ||
|
||
setTimeout(() => { | ||
form.scrollToField(fieldName); | ||
}); | ||
}; |
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.
建议增强 myCustomScrollToField 函数的健壮性
当前实现存在以下几个问题:
- 正则匹配缺少错误处理
- setTimeout 使用了空的延迟时间
- 缺少对 form 实例存在性的检查
const myCustomScrollToField = (fieldName: string) => {
+ if (!form) {
+ console.error('Form instance not found');
+ return;
+ }
+
if (fieldName.startsWith('public')) {
- setActiveParentTab('public-panel');
+ setTabState(prev => ({ ...prev, parentTab: 'public-panel' }));
} else {
- setActiveParentTab('child-panel');
+ setTabState(prev => ({ ...prev, parentTab: 'child-panel' }));
const index = fieldName.match(/\d+/)?.[0];
+ if (!index) {
+ console.error('Invalid field name format');
+ return;
+ }
- setActiveChildTab(`child-panel-${index}`);
+ setTabState(prev => ({ ...prev, childTab: `child-panel-${index}` }));
}
- setTimeout(() => {
+ // 等待 Tab 切换动画完成后再滚动
+ setTimeout(() => {
form.scrollToField(fieldName);
- });
+ }, 300);
};
Committable suggestion skipped: line range outside the PR's diff.
…单项; 2. 增强 `Form` 的 `validate` 方法,校验成功或失败后返回更多的校验信息;
d1c158e
to
cca0d7a
Compare
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: 3
🧹 Outside diff range and nitpick comments (17)
packages/base/src/tabs/tabs-panel.tsx (2)
9-12
: 建议添加属性的类型注释和文档说明为了提高代码的可维护性和可读性,建议:
- 为新增的
tab
属性添加 JSDoc 文档说明其用途和类型约束- 确保
TabsPanelProps
类型定义中包含了所有解构的属性
17-23
: 建议优化 Tab 更新逻辑以防止竞态条件当前的 Tab 更新逻辑在并发更新场景下可能存在问题。建议:
- 使用函数式更新确保状态更新的原子性
- 添加错误边界处理
setTabs(prev => { const prevTab = prev.find(item => item.id === id) + const newTab = { + id, + tab, + disabled: props.disabled, + jssStyle, + color: props.color || (active === id ? color : undefined) + } as TabData if(prevTab){ - return prev.map(item => item.id === id ? { ...item, tab } : item) + return prev.map(item => item.id === id ? { ...item, ...newTab } : item) } - return [...prev, { id, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } as TabData] + return [...prev, newTab] })packages/shineout/src/hooks/use-field-common.tsx (1)
26-26
: 建议优化联合类型定义在联合类型中使用
void
可能会导致混淆,建议使用undefined
来表示可选的返回值。建议修改如下:
- beforeChange?: (value: T) => T | undefined | void; + beforeChange?: (value: T) => T | undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
packages/base/src/form/form-field.tsx (3)
41-50
: 建议增强字段 ID 生成的健壮性和可维护性代码逻辑基本正确,但建议做以下改进:
- 建议对
formControl.name
增加空值校验- 需要为
separator
的使用场景添加注释说明- 可以考虑将 ID 生成逻辑抽取为独立的工具函数,提高代码复用性
建议按如下方式重构:
const { separator } = useContext(FormFieldContext); const formFieldId = useMemo(() => { if(childrenProps.id) return childrenProps.id + + if (!formControl.name) { + console.warn('Form field name is required for generating field ID'); + return ''; + } if(Array.isArray(formControl.name)) { return formControl.name.map(name => util.getFieldId(name, props.formName)).join(separator) } return util.getFieldId(formControl.name, props.formName) }, [formControl.name, props.formName, childrenProps.id])
66-79
: 建议完善渲染逻辑的类型定义和文档渲染逻辑的改进方向:
- 建议为函数类型的 children 添加明确的类型定义
- 需要补充 FormFieldContext.Provider 的用途说明
- 可以考虑添加 children 类型校验
建议添加以下类型定义:
type FormFieldChildrenFunction = (props: FieldControlProps<T>) => React.ReactNode;并更新相关代码:
- if (util.isFunc(children)) { + if (util.isFunc(children) && typeof children === 'function') { + // 确保 children 是合法的渲染函数 finalChildren = children(cloneProps) }
Line range hint
1-79
: 建议优化组件性能和集成模式从架构角度建议以下改进:
- 考虑使用
useCallback
优化handleChange
函数的性能- 建议添加组件集成文档,说明如何正确使用 FormField 与其他表单组件配合
- 可以考虑添加性能监控的 debug 模式
建议在组件顶部添加以下文档注释:
/** * FormField 组件 * * @description * 用于管理表单字段状态和验证的核心组件。 * * @example * ```tsx * <FormField name="username" rules={[{ required: true }]}> * <Input /> * </FormField> * ``` * * @performance * - 使用 useMemo 优化字段 ID 生成 * - 通过 Context 避免不必要的重渲染 */packages/shineout/src/form/__example__/001-base.tsx (1)
22-28
: 建议改进表单事件处理建议对表单事件处理进行以下优化:
- 移除生产环境中的 console.log
- 添加适当的错误处理机制
- 考虑添加加载状态处理
- onSubmit={(v) => { - console.log('form submit', v); - }} + onSubmit={async (v) => { + try { + // 处理表单提交逻辑 + } catch (error) { + // 错误处理 + } + }}packages/hooks/src/components/use-form/use-form.type.ts (2)
7-15
: 类型定义清晰且实用
KeyType
和ValidationError
的类型定义符合 TypeScript 最佳实践。建议为ValidationError
接口添加更详细的 JSDoc 文档,说明各字段的具体用途和示例。建议添加如下文档:
+/** + * 表单验证错误信息的结构 + * @template T 表单值的类型 + * @property values 表单的当前值 + * @property errorFields 包含错误信息的字段数组 + */ export interface ValidationError<T> {
17-32
: 验证函数类型定义合理
ValidateFieldsFn
的泛型约束和ValidateFnConfig
的配置选项设计合理。不过建议对type
的可选值使用字面量联合类型来增强类型安全。建议将
type
的值提取为常量:+export const VALIDATE_TYPES = { + FORCE_PASS: 'forcePass', + WITH_VALUE: 'withValue', +} as const; + +type ValidateType = typeof VALIDATE_TYPES[keyof typeof VALIDATE_TYPES]; + export type ValidateFnConfig = { - type?: 'forcePass' | 'withValue', + type?: ValidateType, ignoreBind?: boolean, }packages/shineout/src/form/__example__/test-002-validate-scrolltoerror.tsx (2)
9-9
: 建议优化测试数据的导入方式建议将测试数据直接定义在示例文件中,或者移动到专门的测试数据目录下,以提高代码的可维护性和可读性。
-import testFromValue from './temp/test-form-value'; +// 建议直接在文件中定义测试数据 +const testFormValue = { + request: { + startDate2: '2024-02-01', + endDate2: '2024-02-02', + // ... 其他测试数据 + } +};
72-78
: 建议增强滚动功能的用户体验当前的滚动实现比较简单,建议增加以下功能:
- 添加平滑滚动效果
- 滚动后高亮显示目标字段
- 支持滚动完成后的回调函数
<Button onClick={() => { - form.scrollToField('request.endDate2') + form.scrollToField('request.endDate2', { + behavior: 'smooth', + highlight: true, + onComplete: () => { + // 滚动完成后的处理 + } + }) }} > scroll to field </Button>packages/hooks/src/components/use-form/use-form.ts (4)
231-267
: 建议优化滚动逻辑的错误处理scrollToField 方法的实现需要增强错误处理机制:
- 当 formName 未定义时应该给出警告
- 建议将警告信息统一到一个集中的错误处理模块
建议添加以下改进:
const scrollToField = usePersistFn( (name: string, scrollIntoViewOptions: ScrollIntoViewOptions = {}) => { if (!name) return; + if (!formName) { + console.warn('[shineout] formName is required for scrollToField'); + return; + } const fieldId = getFieldId(name, formName); if (!fieldId) return; const element = document?.getElementById(fieldId); if (element) { // ... existing code ... } else { - // todo: 统一警告|错误信息(by Tom) - console.warn(`[shineout] fieldId: ${fieldId} not found`); + // 使用统一的错误处理 + handleFormError('FIELD_NOT_FOUND', { fieldId }); } }, );
166-228
: 建议增强验证错误的类型安全性validateFields 方法的错误处理逻辑可以进一步增强类型安全性。
建议添加以下改进:
type ValidationError<T> = { + code: string; values: T; errorFields: Array<{ name: string; errors: string[]; + field: unknown; }>; };🧰 Tools
🪛 Biome (1.9.4)
[error] 185-185: Avoid the use of spread (
...
) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce
) because it causes a time complexity ofO(n^2)
.
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
359-364
: 建议优化空值合并运算符的使用在提交表单时使用空值合并运算符是个好做法,但可以进一步优化代码结构。
建议修改为:
-props.onSubmit?.((context.value ?? {}) as T); +const submitValue = context.value ?? {} as T; +props.onSubmit?.(submitValue);
71-71
: 建议使用 useCallback 优化 ref 回调性能formDomRef 的设置可以通过 useCallback 来优化性能。
建议修改为:
-const formDomRef = React.useRef<HTMLFormElement>(); +const formDomRef = React.useRef<HTMLFormElement | null>(null); +const setFormRef = React.useCallback((node: HTMLFormElement | null) => { + formDomRef.current = node; +}, []); // 在 getFormProps 中 -ref: formDomRef, +ref: setFormRef,Also applies to: 419-419
packages/base/src/cascader/cascader.tsx (1)
Line range hint
716-724
: 建议增强无障碍访问性虽然添加 fieldId 提升了表单字段的可访问性,但建议考虑添加以下增强:
- 添加 aria-expanded 属性以指示下拉状态
- 为选择器添加合适的 aria-label
建议应用以下改进:
<div id={fieldId} tabIndex={disabled === true || showInput ? undefined : 0} {...util.getDataAttribute({ ['input-border']: 'true' })} className={rootClass} style={rootStyle} + aria-expanded={open} + aria-label={placeholder || '级联选择器'} onBlur={handleBlur} onFocus={handleFocus} onKeyDown={handleKeyDown} ref={targetRef} >packages/shineout/src/tabs/__test__/tabs.spec.tsx (1)
Line range hint
351-363
: 测试逻辑需要修正当前测试用例存在逻辑矛盾:先验证元素数量为0,然后又尝试检查这些不存在元素的属性。建议修改测试逻辑如下:
- classLengthTest(container, tabsTabClassName, 0); - container.querySelectorAll(tabsTabClassName).forEach((item, index) => { - if (index === 0) { - attributesTest(item, 'data-soui-state', 'active'); - return - } - attributesTest(item, 'data-soui-state', ''); - }); - classLengthTest(container, '.demo', 0); - container.querySelectorAll('.demo').forEach((item, index) => { - attributesTest(item, 'id', String(index)); - }); - classLengthTest(container, tabsPanelClassName, 0); + // 验证非Panel子组件不会被渲染 + expect(container.querySelector(tabsTabClassName)).toBeNull(); + expect(container.querySelector('.demo')).toBeNull(); + expect(container.querySelector(tabsPanelClassName)).toBeNull();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (4)
packages/shineout/src/card/__test__/__snapshots__/card.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/drawer/__test__/__snapshots__/drawer.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/input/__test__/__snapshots__/input.spec.tsx.snap
is excluded by!**/*.snap
packages/shineout/src/upload/__test__/__snapshots__/upload.spec.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (51)
packages/base/src/button/button-group.tsx
(1 hunks)packages/base/src/button/button.type.ts
(1 hunks)packages/base/src/cascader/cascader.tsx
(3 hunks)packages/base/src/checkbox/checkbox-group.tsx
(3 hunks)packages/base/src/checkbox/simple-checkbox.tsx
(2 hunks)packages/base/src/common/type.ts
(1 hunks)packages/base/src/common/use-with-form-config.ts
(1 hunks)packages/base/src/date-picker/result.tsx
(3 hunks)packages/base/src/editable-area/editable-area.tsx
(3 hunks)packages/base/src/form/form-field-context.ts
(1 hunks)packages/base/src/form/form-field.tsx
(3 hunks)packages/base/src/form/form-item.tsx
(2 hunks)packages/base/src/form/form.tsx
(4 hunks)packages/base/src/form/form.type.ts
(2 hunks)packages/base/src/input/input.tsx
(1 hunks)packages/base/src/input/input.type.ts
(2 hunks)packages/base/src/input/simple-input.tsx
(3 hunks)packages/base/src/radio/radio-group.tsx
(2 hunks)packages/base/src/radio/simple-radio.tsx
(2 hunks)packages/base/src/rate/rate.tsx
(2 hunks)packages/base/src/select/result.tsx
(0 hunks)packages/base/src/select/select.tsx
(4 hunks)packages/base/src/slider/slider.tsx
(2 hunks)packages/base/src/switch/switch.tsx
(2 hunks)packages/base/src/tabs/tabs-panel.tsx
(1 hunks)packages/base/src/tabs/tabs.tsx
(4 hunks)packages/base/src/tabs/tabs.type.ts
(1 hunks)packages/base/src/textarea/simple-textarea.tsx
(3 hunks)packages/base/src/transfer/transfer.tsx
(3 hunks)packages/base/src/tree-select/tree-select.tsx
(3 hunks)packages/base/src/tree/tree.tsx
(2 hunks)packages/base/src/upload/upload.tsx
(3 hunks)packages/hooks/src/components/use-form/use-form-context.type.ts
(3 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.ts
(3 hunks)packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts
(1 hunks)packages/hooks/src/components/use-form/use-form.ts
(12 hunks)packages/hooks/src/components/use-form/use-form.type.ts
(4 hunks)packages/hooks/src/components/use-tabs/context.tsx
(1 hunks)packages/hooks/src/components/use-tabs/context.type.ts
(2 hunks)packages/hooks/src/components/use-tabs/use-tabs.ts
(2 hunks)packages/hooks/src/utils/dom/element.tsx
(1 hunks)packages/shineout-style/src/tabs/tabs.ts
(1 hunks)packages/shineout/src/form/__doc__/changelog.cn.md
(2 hunks)packages/shineout/src/form/__example__/001-base.tsx
(1 hunks)packages/shineout/src/form/__example__/002-control.tsx
(4 hunks)packages/shineout/src/form/__example__/006-validate-05-scrolltofield.tsx
(1 hunks)packages/shineout/src/form/__example__/temp/test-form-value.ts
(1 hunks)packages/shineout/src/form/__example__/test-002-validate-scrolltoerror.tsx
(1 hunks)packages/shineout/src/hooks/use-field-common.tsx
(3 hunks)packages/shineout/src/tabs/__doc__/changelog.cn.md
(1 hunks)packages/shineout/src/tabs/__test__/tabs.spec.tsx
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/base/src/select/result.tsx
🚧 Files skipped from review as they are similar to previous changes (41)
- packages/base/src/button/button-group.tsx
- packages/base/src/button/button.type.ts
- packages/base/src/checkbox/checkbox-group.tsx
- packages/base/src/checkbox/simple-checkbox.tsx
- packages/base/src/common/type.ts
- packages/base/src/common/use-with-form-config.ts
- packages/base/src/date-picker/result.tsx
- packages/base/src/editable-area/editable-area.tsx
- packages/base/src/form/form-field-context.ts
- packages/base/src/form/form-item.tsx
- packages/base/src/form/form.tsx
- packages/base/src/form/form.type.ts
- packages/base/src/input/input.tsx
- packages/base/src/input/input.type.ts
- packages/base/src/input/simple-input.tsx
- packages/base/src/radio/radio-group.tsx
- packages/base/src/radio/simple-radio.tsx
- packages/base/src/rate/rate.tsx
- packages/base/src/select/select.tsx
- packages/base/src/slider/slider.tsx
- packages/base/src/switch/switch.tsx
- packages/base/src/tabs/tabs.tsx
- packages/base/src/tabs/tabs.type.ts
- packages/base/src/textarea/simple-textarea.tsx
- packages/base/src/transfer/transfer.tsx
- packages/base/src/tree-select/tree-select.tsx
- packages/base/src/tree/tree.tsx
- packages/base/src/upload/upload.tsx
- packages/hooks/src/components/use-form/use-form-context.type.ts
- packages/hooks/src/components/use-form/use-form-control/use-form-control.ts
- packages/hooks/src/components/use-form/use-form-control/use-form-control.type.ts
- packages/hooks/src/components/use-tabs/context.tsx
- packages/hooks/src/components/use-tabs/context.type.ts
- packages/hooks/src/components/use-tabs/use-tabs.ts
- packages/hooks/src/utils/dom/element.tsx
- packages/shineout-style/src/tabs/tabs.ts
- packages/shineout/src/form/doc/changelog.cn.md
- packages/shineout/src/form/example/002-control.tsx
- packages/shineout/src/form/example/006-validate-05-scrolltofield.tsx
- packages/shineout/src/form/example/temp/test-form-value.ts
- packages/shineout/src/tabs/doc/changelog.cn.md
🧰 Additional context used
🪛 Biome (1.9.4)
packages/hooks/src/components/use-form/use-form.ts
[error] 185-185: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
packages/shineout/src/hooks/use-field-common.tsx
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (13)
packages/base/src/tabs/tabs-panel.tsx (1)
16-24
: 🛠️ Refactor suggestion
优化 Effect 的实现和依赖项管理
当前实现存在以下问题:
- 使用
useLayoutEffect
可能会影响性能,建议改用useEffect
- 缺少清理函数,可能导致内存泄漏
- 依赖数组中的条件判断
...(color ? [active] : [])
使代码难以理解
建议按如下方式重构:
- useLayoutEffect(() => {
+ useEffect(() => {
setTabs(prev => {
const prevTab = prev.find(item => item.id === id)
if(prevTab){
return prev.map(item => item.id === id ? { ...item, tab } : item)
}
return [...prev, { id, tab, disabled: props.disabled, jssStyle, color: props.color || (active === id ? color : undefined) } as TabData]
})
+ return () => {
+ setTabs(prev => prev.filter(item => item.id !== id))
+ }
- }, [id, tab, color, ...(color ? [active] : []), props.disabled, props.jssStyle])
+ }, [id, tab, color, active, props.disabled, props.jssStyle])
packages/shineout/src/hooks/use-field-common.tsx (2)
Line range hint 27-31
: 接口命名修正和定义完善
接口重命名修复了拼写错误,从 FiledItemCommonProps
更正为 FieldItemCommonProps
。接口定义清晰,类型声明合理。
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-27: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
55-55
: 验证表单名称传递的完整性
新增的 formName
属性支持了表单字段的精确定位,这对于实现 scrollToField
等新功能是必要的。建议验证相关功能的完整实现。
✅ Verification successful
formName
属性的集成已得到验证
formName
属性在整个代码库中得到了完整的支持:
- 在
hooks/src/utils/dom/element.tsx
中实现了getFieldId
工具函数,用于生成带有表单名称前缀的字段ID - 在
hooks/src/components/use-form
中的表单核心逻辑正确处理了formName
- 在
base/src/form/form-field.tsx
中正确使用了formName
来生成唯一的字段标识
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 formName 在相关组件中的使用情况
# 检查 formName 的使用
rg "formName" --type ts --type tsx
Length of output: 62
Script:
#!/bin/bash
# 使用 ripgrep 搜索 formName,不指定文件类型以避免兼容性问题
rg "formName"
# 使用 ast-grep 搜索 formName 相关的属性和使用
ast-grep --pattern 'formName:'
Length of output: 1494
packages/shineout/src/form/__example__/001-base.tsx (2)
8-16
: 代码结构清晰,实现合理!
组件导入结构合理,自定义的星级评分组件实现简洁优雅。
42-43
: 性别字段拼写错误
'gendar' 的拼写错误仍然存在,需要修正为 'gender'。
packages/hooks/src/components/use-form/use-form.type.ts (3)
38-39
: 注意返回类型的破坏性改动
将 ValidateFn
的返回类型从 Promise<boolean | FormError>
改为 Promise<true | FormError>
是一个破坏性改动。
227-227
: 验证返回类型保持一致性
validateMap
的返回类型更新与 ValidateFn
的改动保持一致,这种一致性有助于维护代码的可预测性。
124-130
: 表单命名功能增强了可访问性
新增的 formName
属性通过支持 <label for="id">
功能提升了表单的可访问性,这是一个很好的改进。
建议执行以下脚本验证实现:
✅ Verification successful
Let's gather more information about the implementation details.
Let's check the utility function that generates the field ID and how it's used with labels.
Let's check one final aspect - how the label and form field are connected.
Based on the gathered information, I can now provide the final response:
formName
属性实现已得到验证
从代码实现来看,formName
属性通过 getFieldId
工具函数正确地生成了表单字段的唯一标识符:
- 实现位于
packages/hooks/src/utils/dom/element.tsx
- ID 格式为:
${formName}_${name}
- 在
FormField
组件中被正确使用
这个实现确实支持了 <label for="id">
的关联功能,提升了表单的可访问性。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证表单字段 ID 的生成逻辑和标签的关联实现
# 搜索相关实现代码
rg -l "formName|htmlFor" --type ts
Length of output: 477
Script:
#!/bin/bash
# Check the implementation of formName in use-form.ts and form-field.tsx
rg "formName" -A 5 packages/hooks/src/components/use-form/use-form.ts packages/base/src/form/form-field.tsx
# Check how htmlFor is used with formName
rg "htmlFor" -A 5 packages/base/src/form/form-field.tsx
Length of output: 2683
Script:
#!/bin/bash
# Check the getFieldId utility implementation
rg "getFieldId" -A 5 --type ts
# Check label implementation
ast-grep --pattern 'label $$$'
Length of output: 1995
Script:
#!/bin/bash
# Check Label component implementation and htmlFor usage
rg -A 5 "htmlFor.*=.*{" --type tsx
rg -A 5 "<Label.*>" --type tsx
Length of output: 187
packages/shineout/src/form/__example__/test-002-validate-scrolltoerror.tsx (2)
11-11
: 建议使用更具体的类型定义替代 any
34-48
: 建议优化表单验证的错误处理方式
packages/hooks/src/components/use-form/use-form.ts (1)
455-458
: 改进变量命名以提高代码可读性
validateField 参数名称与其实际用途更匹配,这是一个好的改进。
packages/base/src/cascader/cascader.tsx (2)
1-1
: 导入声明看起来不错!
FormFieldContext 的导入位置合适,与其他 React 相关的导入保持一致的组织方式。
Also applies to: 26-26
34-34
: Context 使用方式正确!
使用 useContext 获取 fieldId 的方式符合 React Hooks 的最佳实践。
<Form.Item label='upload avatar'> | ||
<Upload.Image | ||
action='/api/upload' | ||
accept='image/*' | ||
name='file' | ||
htmlName='file' | ||
recoverAble | ||
leftHandler | ||
removeConfirm='Are you sure to delete it ?' | ||
limit={3} | ||
onSuccess={(_res, filem, data) => { | ||
return data; | ||
}} | ||
/> | ||
</Form.Item> |
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.
建议加强上传组件的安全性
上传组件的配置需要注意以下安全考虑:
- 建议添加文件大小限制
- 建议增加文件类型验证
- 建议在上传前进行文件校验
<Upload.Image
action='/api/upload'
accept='image/*'
name='file'
htmlName='file'
recoverAble
leftHandler
removeConfirm='Are you sure to delete it ?'
limit={3}
+ maxSize={5 * 1024 * 1024} // 添加文件大小限制
+ beforeUpload={(file) => {
+ // 添加文件校验逻辑
+ return true;
+ }}
onSuccess={(_res, filem, data) => {
+ // 添加响应数据验证
return data;
}}
/>
Committable suggestion skipped: line range outside the PR's diff.
/> | ||
</Form.Item> | ||
</Grid> | ||
<Grid width={1 / 3}> | ||
<Form.Item label='ruleEnumList' required> | ||
<Select | ||
keygen | ||
name='ruleEnumList' | ||
data={[1, 2, 3]} | ||
rules={[{ required: true, message: 'ruleEnumList is required' }]} | ||
/> | ||
</Form.Item> | ||
</Grid> | ||
</Grid> | ||
)} | ||
</Form.FieldSet> | ||
</Form.FieldSet> | ||
</Tabs.Panel> | ||
<Tabs.Panel id='brand-panel' tab='品牌信息' style={{ padding: '12px 0' }}> | ||
<Tabs | ||
active={activeChildTab} | ||
onChange={(tab) => setActiveChildTab(tab as string)} | ||
position='left-top' | ||
shape='card' | ||
> | ||
<Form.FieldSet name='brandPartnerUpsertReq'> | ||
{({ list, index }) => { | ||
if (list.length === 0) return <span>暂无brandPartnerUpsertReq</span>; | ||
return ( | ||
<Tabs.Panel | ||
id={`child-panel-${index}`} | ||
tab={`child-panel-${index}`} | ||
style={{ padding: 12 }} | ||
> | ||
<Form.Item label={'brandId'} required> | ||
<Input | ||
name={'brandId'} | ||
rules={[{ required: true, message: 'brandId is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.FieldSet name='labelBuildRequest'> | ||
<Form.Item label={'busId'} required> | ||
<Input | ||
name={'busId'} | ||
rules={[{ required: true, message: 'busId is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={'moduleId'} required> | ||
<Input | ||
name={'moduleId'} | ||
rules={[{ required: true, message: 'moduleId is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={'originalPartnerId'} required> | ||
<Input | ||
name={'originalPartnerId'} | ||
rules={[ | ||
{ | ||
required: true, | ||
message: 'originalPartnerId is required', | ||
}, | ||
]} | ||
/> | ||
</Form.Item> | ||
</Form.FieldSet> | ||
<Form.FieldSet name='updateBrandPartnerRequest'> | ||
<Form.Item label={'liveExperience'} required> | ||
<Input | ||
name={'liveExperience'} | ||
rules={[{ required: true, message: 'liveExperience is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={'childBrand'} required> | ||
<Select | ||
keygen | ||
name='childBrand' | ||
data={[1, 2, 3]} | ||
rules={[{ required: true, message: 'childBrand is required' }]} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={'originalPartnerId'} required> | ||
<Input | ||
name={'originalPartnerId'} | ||
rules={[ | ||
{ | ||
required: true, | ||
message: 'originalPartnerId is required', | ||
}, | ||
]} | ||
/> | ||
</Form.Item> | ||
</Form.FieldSet> | ||
</Tabs.Panel> | ||
); | ||
}} | ||
</Form.FieldSet> | ||
</Tabs> | ||
</Tabs.Panel> | ||
</Tabs> | ||
</Form.FieldSet> |
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.
🛠️ Refactor suggestion
建议优化表单结构和验证信息管理
当前实现存在以下可以改进的地方:
- 验证信息重复定义,建议统一管理
- 表单结构嵌套层级较深,建议拆分成更小的组件
- 缺少字段说明和验证规则的统一配置
建议进行如下优化:
- 创建统一的验证信息配置:
const validationMessages = {
required: {
partnerId: 'partnerId is required',
bodyType: 'bodyType is required',
// ... 其他字段
}
};
- 将复杂的表单结构拆分成独立组件:
// PublicInfoForm.tsx
const PublicInfoForm: React.FC = () => {
return (
<Form.FieldSet name='commonPartnerUpsertReq'>
<PartnerRequestForm />
<LabelRuleInfoForm />
</Form.FieldSet>
);
};
// BrandInfoForm.tsx
const BrandInfoForm: React.FC = () => {
// ... 品牌信息表单实现
};
const fieldArray = isArray(fields) ? fields : [fields]; | ||
validatorValue = fieldArray.reduce( | ||
(prev, cur) => ({ | ||
...prev, | ||
[cur]: getValue(cur), | ||
}), | ||
{}, | ||
); |
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.
性能优化:避免在 reduce 中使用展开运算符
在 reduce 累加器中使用展开运算符会导致时间复杂度从 O(n) 增加到 O(n^2),建议重构此处代码以提高性能。
建议修改为:
-validatorValue = fieldArray.reduce(
- (prev, cur) => ({
- ...prev,
- [cur]: getValue(cur),
- }),
- {},
-);
+validatorValue = {};
+fieldArray.forEach((cur) => {
+ validatorValue[cur] = getValue(cur);
+});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const fieldArray = isArray(fields) ? fields : [fields]; | |
validatorValue = fieldArray.reduce( | |
(prev, cur) => ({ | |
...prev, | |
[cur]: getValue(cur), | |
}), | |
{}, | |
); | |
const fieldArray = isArray(fields) ? fields : [fields]; | |
validatorValue = {}; | |
fieldArray.forEach((cur) => { | |
validatorValue[cur] = getValue(cur); | |
}); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 185-185: Avoid the use of spread (...
) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce
) because it causes a time complexity of O(n^2)
.
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
Types of changes
Changelog
Form
的validate
方法,校验成功或失败后返回更多的校验信息 (#812)Form
的formRef
,增加scrollToField
方法,支持根据 name 滚动至指定表单项 (#812)Tabs
支持渲染非Tabs.Panel
子组件,例如Form.FieldSet
(#812)Summary by CodeRabbit
发布说明
新特性
CommonType
接口中新增可选属性name
,用于表单控件的标识。Tabs
组件现在支持allowNonPanel
属性,允许渲染非Tabs.Panel
子组件。Form
组件新增validateFieldsWithValue
和scrollToField
方法,增强表单验证和滚动功能。Bug 修复
fieldId
的动态分配,提升了可访问性。文档