-
Notifications
You must be signed in to change notification settings - Fork 268
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): resetFields 增加 namepath 参数,用于重置指定的字段 #2953
Conversation
Warning Rate limit exceeded@oasis-cloud has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 59 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. 📒 Files selected for processing (7)
变更概览演练此次变更主要涉及表单组件的重置字段功能增强。在 变更
可能相关的 PR
建议标签
建议审阅者
诗歌
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2953 +/- ##
=======================================
Coverage 84.19% 84.20%
=======================================
Files 267 267
Lines 18099 18114 +15
Branches 2661 2667 +6
=======================================
+ Hits 15238 15252 +14
- Misses 2856 2857 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/packages/form/useform.ts (1)
203-225
: 优化重置字段的实现当前实现存在以下需要注意的点:
- 性能优化:对于选择性重置,可以先过滤出相关的 fieldEntities 再进行遍历
- 测试覆盖:需要补充对
path in this.initialValues
分支的测试用例建议重构如下:
resetFields = (namePaths?: NamePath[]) => { if (namePaths) { namePaths.forEach((path) => { this.errors[path] = null - this.fieldEntities.forEach((entity: FormFieldEntity) => { - const name = entity.props.name - if (name === path) { + const targetEntities = this.fieldEntities.filter( + (entity) => entity.props.name === path + ) + targetEntities.forEach((entity) => { if (path in this.initialValues) { this.updateStore({ [path]: this.initialValues[path] }) } else { delete this.store[path] } entity.onStoreChange('reset') - } }) }) } else { const nextStore = merge({}, this.initialValues) this.updateStore(nextStore) this.fieldEntities.forEach((entity: FormFieldEntity) => { entity.onStoreChange('reset') }) } }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 211-211: src/packages/form/useform.ts#L211
Added line #L211 was not covered by testssrc/packages/form/__tests__/form.spec.tsx (2)
324-335
: 优化 Fragment 的使用footer 属性中的 Fragment 是不必要的,可以直接返回 div 元素。
footer={ - <> <div id="reset" onClick={() => { form.resetFields(['username']) }} > Reset </div> - </> }
316-367
: 补充更多测试场景当前测试用例仅覆盖了基本的重置功能,建议添加以下测试场景:
- 重置不存在于 initialValues 中的字段
- 同时重置多个字段
- 重置所有字段(不传参数)
需要我帮你生成这些测试用例吗?
🧰 Tools
🪛 Biome (1.9.4)
[error] 328-339: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/form/__tests__/form.spec.tsx
(1 hunks)src/packages/form/useform.taro.ts
(2 hunks)src/packages/form/useform.ts
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/form/__tests__/form.spec.tsx
[error] 328-339: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🪛 GitHub Check: codecov/patch
src/packages/form/useform.ts
[warning] 211-211: src/packages/form/useform.ts#L211
Added line #L211 was not covered by tests
🔇 Additional comments (2)
src/packages/form/useform.ts (1)
177-177
: 验证错误处理行为的变更注释掉
this.errors.length = 0
是为了支持选择性重置字段功能。请确认在以下场景中错误状态的处理是否符合预期:
- 重置单个字段时的错误状态
- 重置所有字段时的错误状态
✅ Verification successful
错误处理行为变更已验证正确
代码中移除
this.errors.length = 0
是合理的,现在的实现通过this.errors[path] = null
精确清除指定字段的错误状态,这与选择性重置字段的功能设计相符。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有其他地方清除了所有错误 rg -A 3 "this\.errors.*=.*\[\]"Length of output: 380
Script:
#!/bin/bash # 检查resetFields的实现 ast-grep --pattern 'resetFields($$$) { $$$ }'Length of output: 49
Script:
#!/bin/bash # 搜索resetFields方法的实现 rg -A 10 "resetFields.*\(" src/packages/form/Length of output: 6647
src/packages/form/useform.taro.ts (1)
Line range hint
177-225
: 代码变更与 useform.ts 一致此文件的变更与 useform.ts 完全相同,相关的优化建议也同样适用。
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.
可以 resolve 下 conflicts
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.
fix一下 单测冲突~
src/packages/form/useform.taro.ts
Outdated
entity.onStoreChange('reset') | ||
}) | ||
resetFields = (namePaths?: NamePath[]) => { | ||
if (namePaths) { |
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.
需要判断 namePaths 长度不为0,为0 时,应该全部重置。
补一下文档。 |
# Conflicts: # src/packages/form/__tests__/form.spec.tsx
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
增加 namepath 参数用于重置指定字段的内容
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
测试
改进
resetFields
方法,提供了更灵活的字段重置选项