-
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
fix(inputnumber): 增加 beforeChange,优化异步逻辑 #2950
base: next
Are you sure you want to change the base?
Conversation
概述演练这个拉取请求主要修改了 变更
可能相关的 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 #2950 +/- ##
==========================================
+ Coverage 84.03% 84.14% +0.10%
==========================================
Files 220 220
Lines 17917 17884 -33
Branches 2628 2624 -4
==========================================
- Hits 15057 15048 -9
+ Misses 2855 2831 -24
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: 2
🧹 Nitpick comments (4)
src/packages/inputnumber/demos/taro/demo8.tsx (2)
4-19
: 建议优化 useDebounce 钩子的类型安全性和清理逻辑建议进行以下改进:
- const timeoutId: any = useRef() + const timeoutId = useRef<NodeJS.Timeout>() const cleanup = useCallback(() => { if (timeoutId.current) { clearTimeout(timeoutId.current) } }, []) + useEffect(() => { + return cleanup + }, [cleanup]) return function (...args: T) { - if (timeoutId.current) { - clearTimeout(timeoutId.current) - } + cleanup() timeoutId.current = setTimeout(() => { func(...args) }, delay) }
35-41
: 建议优化异步演示的实现方式当前实现使用了硬编码的延迟时间和中文提示信息,建议:
- 将延迟时间抽取为可配置的常量
- 使用国际化文本支持多语言
+const DEBOUNCE_DELAY = 300 +const ASYNC_DELAY = 2000 +const messages = { + 'zh-CN': '异步演示 2 秒后更改', + 'en-US': 'Async demo, changing in 2 seconds' +} - const onChange = useDebounce((value: string | number) => { - toastShow('异步演示 2 秒后更改', 'loading') + const onChange = useDebounce((value: string | number) => { + toastShow(messages[locale], 'loading') setTimeout(() => { setInputValue(Number(value)) SetShow(false) - }, 2000) + }, ASYNC_DELAY) - }, 300) + }, DEBOUNCE_DELAY)src/packages/inputnumber/doc.en-US.md (2)
124-124
: 建议:更新废弃属性的文档说明建议在文档中明确说明
async
属性被废弃的原因,以及用户应该迁移到beforeChange
的具体步骤。这样可以帮助用户更好地理解和适应这个变更。建议添加如下说明:
- | ~~async~~`2.8.0` | Support for asynchronous modification | `boolean` | `false` | + | ~~async~~`2.8.0` | Support for asynchronous modification (已废弃。请使用 `beforeChange` 属性来处理异步验证逻辑) | `boolean` | `false` |
127-127
: 建议:补充 beforeChange 的使用示例新增的
beforeChange
属性文档说明比较简单,建议添加具体的使用示例,特别是异步验证的场景,以帮助用户更好地理解如何使用这个新特性。建议在属性说明后添加示例代码:
| beforeChange`2.8.0` | Callback function before the input value changes, return false to prevent input, support returning Promise | `(value: number \| string) => boolean \| Promise<boolean>` | `-` | + + Example: + ```tsx + // 同步验证 + <InputNumber + beforeChange={(value) => { + return value > 0; // 只允许输入正数 + }} + /> + + // 异步验证 + <InputNumber + beforeChange={async (value) => { + const response = await validateNumber(value); + return response.isValid; + }} + /> + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/packages/inputnumber/demos/h5/demo8.tsx
(1 hunks)src/packages/inputnumber/demos/taro/demo8.tsx
(2 hunks)src/packages/inputnumber/doc.en-US.md
(1 hunks)src/packages/inputnumber/doc.md
(1 hunks)src/packages/inputnumber/doc.taro.md
(1 hunks)src/packages/inputnumber/doc.zh-TW.md
(1 hunks)src/packages/inputnumber/inputnumber.taro.tsx
(7 hunks)src/packages/inputnumber/inputnumber.tsx
(7 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/inputnumber/inputnumber.tsx
[error] 206-206: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/inputnumber/inputnumber.taro.tsx
[error] 209-209: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (9)
src/packages/inputnumber/inputnumber.tsx (1)
31-31
: 添加 beforeChange 属性以支持异步校验新增的
beforeChange
属性增强了组件的可控性,允许在值改变前进行异步验证,确保输入的有效性。src/packages/inputnumber/inputnumber.taro.tsx (1)
33-33
: 添加 beforeChange 属性以支持异步校验新增的
beforeChange
属性使组件在值变化前可以进行异步验证,增强了组件的灵活性和可控性。src/packages/inputnumber/demos/h5/demo8.tsx (2)
6-19
: 引入 useDebounce 自定义 Hook 优化性能添加了
useDebounce
自定义 Hook,对onChange
方法进行了防抖处理,优化了组件的性能,避免了过于频繁的状态更新。
38-38
: 调整 min 属性以支持更大范围的输入将
InputNumber
组件的min
属性从-6
修改为-9999
,扩展了可输入的数值范围,更好地满足了业务需求。src/packages/inputnumber/demos/taro/demo8.tsx (1)
47-49
: 验证最小值限制的合理性当前设置的最小值为 -9999,这个范围可能过大。建议:
- 根据业务需求设置合理的最小值
- 考虑添加输入验证以防止潜在的数值溢出
src/packages/inputnumber/doc.md (1)
124-127
: 文档更新清晰完整文档变更合理地反映了组件的更新:
- 标记了 async 属性的废弃
- 新增了 beforeChange 回调函数
- 补充了 select 属性的版本信息
src/packages/inputnumber/doc.zh-TW.md (1)
116-119
: 文档翻译保持一致繁体中文版本的文档变更与简体中文版本保持一致,翻译准确。
src/packages/inputnumber/doc.taro.md (1)
117-119
: Taro 文档更新完整Taro 版本的文档变更与其他版本保持一致,变更内容完整。
src/packages/inputnumber/doc.en-US.md (1)
125-125
: 版本标注正确
select
属性的版本标注清晰地表明了该特性是在 2.7.0 版本引入的。
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/inputnumber/__tests__/inputnumber.spec.tsx (3)
Line range hint
12-32
: 异步处理的改进很好,建议进一步优化测试用例!异步包装和 act 的使用是正确的,这样可以确保状态更新完成后再进行断言。不过建议将相关的断言也包装在 await waitFor 中,以确保更好的可靠性。
await act(async () => { fireEvent.click(iconPlus) }) - expect(overlimit).not.toBeCalled() - expect(add).toHaveBeenCalled() - expect(change.mock.calls[0][0]).toBe(3) + await waitFor(() => { + expect(overlimit).not.toBeCalled() + expect(add).toHaveBeenCalled() + expect(change.mock.calls[0][0]).toBe(3) + })
Line range hint
35-55
: 建议与加号按钮测试保持一致的异步处理方式这个测试用例的异步改进方向是对的,建议也使用 waitFor 来包装断言,以保持与加号按钮测试的一致性。
await act(async () => { fireEvent.click(iconMinus) }) - expect(overlimit).not.toBeCalled() - expect(reduce).toBeCalled() - expect(change.mock.calls[0][0]).toBe(1) + await waitFor(() => { + expect(overlimit).not.toBeCalled() + expect(reduce).toBeCalled() + expect(change.mock.calls[0][0]).toBe(1) + })
174-184
: 建议增强超限输入测试的完整性虽然异步处理正确,但建议增加更多断言来全面测试超限情况:
- 验证输入值是否被正确限制
- 验证 overlimit 回调的参数
await act(async () => { fireEvent.input(input) }) - expect(overlimit).toBeCalled() + await waitFor(() => { + expect(overlimit).toHaveBeenCalledWith(200) + expect(input.value).toBe('100') + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/inputnumber/__tests__/inputnumber.spec.tsx
(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
src/packages/inputnumber/__tests__/inputnumber.spec.tsx (5)
2-2
: 导入 act 函数是正确的做法!从 @testing-library/react 导入 act 函数对于处理异步操作是必要的,这符合 React 测试的最佳实践。
Line range hint
57-78
: 异步处理正确,建议统一使用 waitFor异步改动符合预期,建议统一使用 waitFor 来处理断言,以确保测试的稳定性。
Line range hint
80-102
: 异步处理方式正确,保持一致性很重要异步改动符合预期,建议与其他测试用例保持一致的异步处理方式。
116-127
: readonly 属性的异步测试处理得当异步包装的添加是正确的,测试逻辑完整地验证了 readonly 属性的行为。
129-138
: 小数步进值的异步测试实现合理异步处理的添加确保了小数计算的准确性验证,测试逻辑完善。
🤔 这个变动的性质是?
🔗 相关 Issue
#2530
💡 需求背景和解决方案
async 属性和 onChange 的组合实现的用法不明确,实际场景中主要在 onChange 之前检查是不是可以更改 Input 中的值。基于这样的情况,增加了 beforeChange 方法。
Summary by CodeRabbit
新功能
beforeChange
属性,允许在输入值变更前进行异步验证-6
调整到-9999
文档更新
async
属性,并引入了更灵活的beforeChange
属性性能优化
测试改进