Add Validator callback and provide prevValue in Parser callback #644
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改对 Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
src/InputNumber.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]_jest@29.7.0_@types[email protected]_ts-node@10.9.2_@types[email protected][email protected][email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. tests/formatter.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]_jest@29.7.0_@types[email protected]_ts-node@10.9.2_@types[email protected][email protected][email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learningssrc/InputNumber.tsx (2)🔇 Additional comments (9)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
src/InputNumber.tsx (2)
90-93: 需要更新formatter函数的所有实例以匹配新签名在测试文件和文档示例中，有些
formatter函数的使用没有更新为包含info参数的新签名。这可能导致不一致和潜在错误。请确保所有使用formatter的地方都已更新。
- 需要更新的文件包括：
tests/github.test.tsx,tests/formatter.test.tsx,docs/demo/formatter.tsx等。Analysis chain
批准更改并验证
formatter函数的使用。此更改符合 PR 目标，增强了验证和格式化功能。请确保组件中所有使用
formatter的地方都已更新，以传递新参数。运行以下脚本以验证
formatter函数的使用：Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证所有 `formatter` 函数调用都匹配新的签名。 # 测试：搜索函数使用。预期：只有新签名的出现。 rg --type typescript -A 5 $'formatter'Length of output: 72
Script:
#!/bin/bash # 描述：验证所有 `formatter` 函数调用都匹配新的签名。 # 测试：搜索函数使用。预期：只有新签名的出现。 rg --type ts -A 5 $'formatter'Length of output: 14346
88-88: 需要更新parser函数的所有用法以匹配新签名在多个文件中发现
parser函数的用法未更新为包含info参数的新签名。这可能导致不一致和运行时错误。请确保在以下文件中更新parser函数的用法：
tests/formatter.test.tsxtests/cursor.test.tsxdocs/demo/formatter.tsxdocs/demo/combination-key-format.tsx请确保所有实例都符合新的函数签名，以确保组件的正确功能。
Analysis chain
批准更改并验证
parser函数的使用。此更改符合 PR 目标，增强了验证和格式化功能。请确保组件中所有使用
parser的地方都已更新，以传递新参数。运行以下脚本以验证
parser函数的使用：Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证所有 `parser` 函数调用都匹配新的签名。 # 测试：搜索函数使用。预期：只有新签名的出现。 rg --type typescript -A 5 $'parser'Length of output: 69
Script:
#!/bin/bash # 描述：验证所有 `parser` 函数调用都匹配新的签名。 # 测试：搜索函数使用。预期：只有新签名的出现。 rg --type tsx -A 5 $'parser'Length of output: 55
Script:
#!/bin/bash # 描述：验证所有 `parser` 函数调用都匹配新的签名。 # 测试：搜索函数使用。预期：只有新签名的出现。 rg --glob '*.tsx' -A 5 $'parser'Length of output: 5000
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/InputNumber.tsx (7 hunks)
Additional comments not posted (1)
src/InputNumber.tsx (1)
95-95: 批准添加validator函数并验证其实现。此添加符合 PR 目标，防止输入不需要的字符。请确保
validator函数被正确调用，并按预期处理验证。运行以下脚本以验证
validator函数的实现：Verification successful
验证
validator函数的实现是正确的。
validator函数被正确地定义为可选属性，并在输入处理逻辑中正确调用。它确保只有通过验证的输入才会被进一步处理，从而增强了组件的健壮性。此实现符合其预期目的。
validator函数在src/InputNumber.tsx中被正确使用。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证 `validator` 函数是否正确实现。 # 测试：搜索函数实现。预期：函数应正确调用并处理验证。 rg --type typescript -A 5 $'validator'Length of output: 72
Script:
#!/bin/bash # 描述：验证 `validator` 函数是否正确实现。 # 测试：搜索函数实现。预期：函数应正确调用并处理验证。 rg --type ts -A 5 $'validator'Length of output: 921
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/InputNumber.tsx (7 hunks)
- tests/formatter.test.tsx (1 hunks)
🔇 Additional comments (4)
tests/formatter.test.tsx (2)
Line range hint
1-210: 文件总结这个测试文件的更改很小但很重要，它验证了新增的
prevValue参数功能。这与 PR 的目标一致，提高了对新特性的测试覆盖率。建议：
- 考虑添加更多测试用例，以全面覆盖
prevValue参数在各种情况下的行为。- 确保其他相关的测试文件也已更新，以反映这个新参数的添加。
- 更新文件顶部的注释或文档字符串，说明新增的
prevValue参数及其用途。运行以下脚本以确保所有相关的测试文件都已更新：
#!/bin/bash # 描述：检查其他测试文件是否也包含了 prevValue 参数的测试 # 测试：在测试文件中搜索 formatter 函数的使用。预期：相关的测试文件都应包含 prevValue 参数 rg --type typescript --type tsx 'formatter.*=.*\(.*,.*\{.*prevValue' -g 'tests/*'
166-166: 验证新增的 prevValue 参数这个更改为 formatter 函数调用添加了
prevValue参数。这与 PR 的目标一致，即在回调中提供先前的输入值。建议：
- 确保这个测试用例充分验证了
prevValue参数的正确性。- 考虑添加更多测试用例，以覆盖
prevValue在不同场景下的行为，例如连续输入时的值变化。为了确保
prevValue参数在整个代码库中得到正确实现，请运行以下脚本：src/InputNumber.tsx (2)
Line range hint
209-216: 确保在调用parser时正确传递prevValue。在
mergedParser函数中，prevValue的获取方式为String(prevValueRef.current ?? '')。请确认prevValueRef.current的值在预期范围内，且不会出现undefined或null的情况，以避免字符串转换时出现意外结果。
88-93: 请确认扩展后的parser和formatter接口不会引入破坏性变更。
parser和formatter函数的签名已修改，增加了一个额外的参数info，包含prevValue。这可能会影响所有现有使用这些函数的地方，请确保所有调用都已更新以匹配新的函数签名，防止潜在的运行时错误。您可以使用以下脚本查找代码库中所有调用
parser和formatter的位置：✅ Verification successful
确认无破坏性变更引入
代码库中仅在
src/InputNumber.tsx中调用了parser和formatter，且这些调用已包含新的info参数。确认扩展后的接口不会引入破坏性变更。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 查找所有使用 'parser' 的代码位置 rg --type typescript --type javascript 'parser\(' # 查找所有使用 'formatter' 的代码位置 rg --type typescript --type javascript 'formatter\('Length of output: 176
Script:
#!/bin/bash # 查找所有使用 'parser' 的代码位置 rg 'parser\(' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx' # 查找所有使用 'formatter' 的代码位置 rg 'formatter\(' --glob '*.ts' --glob '*.tsx' --glob '*.js' --glob '*.jsx'Length of output: 311
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #644 +/- ##
==========================================
+ Coverage 95.95% 96.05% +0.09%
==========================================
Files 6 6
Lines 297 304 +7
Branches 78 82 +4
==========================================
+ Hits 285 292 +7
Misses 12 12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
tests/validator.test.tsx (1)
5-32: 第一个测试用例"validator on direct input"看起来不错，但可以进一步改进。这个测试用例全面地覆盖了InputNumber组件的验证器功能，包括无效输入和有效数字输入的情况。然而，我们可以通过以下方式进一步增强测试：
添加更多的边界情况测试，例如：
- 输入最大值和最小值（如果组件有这些限制）
- 输入小数点和负数（如果组件支持）
- 输入非常大的数字
测试连续输入多个字符的情况，确保验证器在每次按键时都正确工作。
验证
onChange函数在无效输入时是否未被调用。考虑测试粘贴操作，因为用户可能会尝试粘贴无效内容。
您是否希望我为这些建议提供具体的测试代码示例？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/validator.test.tsx (1 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: bombillazo PR: react-component/input-number#644 File: src/InputNumber.tsx:173-174 Timestamp: 2024-09-29T06:18:11.993Z Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
🔇 Additional comments (2)
tests/validator.test.tsx (2)
1-3: 导入语句看起来很好。导入语句包含了所需的所有必要模块，包括KeyCode、InputNumber组件和测试工具。这些导入为测试文件提供了所需的功能。
1-84: 总体评价：测试文件结构良好，覆盖了关键功能，但有进一步改进的空间。这个测试文件为InputNumber组件的验证器和格式化器功能提供了全面的测试覆盖。它测试了各种用户输入场景、键盘导航和鼠标交互，这对于确保组件的可靠性和用户体验至关重要。
然而，实施先前建议的改进将进一步增强测试的全面性和稳健性：
- 添加更多边界情况和复杂输入场景的测试。
- 改进对无效输入的处理和测试。
- 增加对性能和用户体验相关方面的测试。
这些改进将有助于捕获潜在的边缘情况bug，并确保组件在各种使用场景下都能正常工作。建议在合并此PR之前考虑实施这些改进。
为了确保测试覆盖率的提高，请运行以下命令来检查当前的测试覆盖率，并在实施建议的改进后再次运行以比较结果：
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/InputNumber.tsx(7 hunks)
🧰 Additional context used
📓 Learnings (1)
src/InputNumber.tsx (3)
Learnt from: bombillazo
PR: react-component/input-number#644
File: src/InputNumber.tsx:393-395
Timestamp: 2024-11-12T06:11:49.241Z
Learning: The `InputNumber` component does not use error states or messages; it is designed to prevent the value from updating if validation fails without displaying additional feedback to the user.
Learnt from: bombillazo
PR: react-component/input-number#644
File: src/InputNumber.tsx:393-395
Timestamp: 2024-11-12T06:11:49.240Z
Learning: The `InputNumber` component does not use error states or messages; it is designed to prevent the value from updating if validation fails without displaying additional feedback to the user.
Learnt from: bombillazo
PR: react-component/input-number#644
File: src/InputNumber.tsx:173-174
Timestamp: 2024-11-12T06:11:49.240Z
Learning: In `src/InputNumber.tsx`, within the `InternalInputNumber` component, the state variables `prevValueRef` and `inputValueRef` are typed as `string | number` to maintain consistency with existing code.
🔇 Additional comments (1)
src/InputNumber.tsx (1)
394-397:
确保在验证失败时更新内部引用以维护状态一致性
在 collectInputValue 函数中，当输入未通过 validator 验证时，函数会提前返回，导致 prevValueRef 和 inputValueRef 未更新。这可能导致组件内部状态不一致。建议在验证之前更新这些引用，以确保状态的正确性。
应用以下代码修改：
collectInputValue = (inputStr: string) => {
+ prevValueRef.current = inputValueRef.current;
+ inputValueRef.current = inputStr;
+ setInternalInputValue(inputStr);
// 验证输入字符串
if (validator) {
if (!validator(inputStr)) return;
}Likely invalid or redundant comment.
| parser?: (displayValue: string | undefined, info: { prevValue: string }) => T; | ||
| /** Transform `value` to display value show in input */ | ||
| formatter?: (value: T | undefined, info: { userTyping: boolean; input: string }) => string; | ||
| formatter?: ( | ||
| value: T | undefined, | ||
| info: { userTyping: boolean; input: string; prevValue: string }, | ||
| ) => string; | ||
| /** Validate an input string before processing */ | ||
| validator?: (input: string) => boolean; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
注意：parser 和 formatter 方法签名的更改可能引入破坏性变更
在 InputNumberProps 接口中为 parser 和 formatter 方法添加新的参数 info，可能会导致现有使用这些属性的代码出现 TypeScript 类型错误，因为现有实现可能未预期新的 info 参数。建议将新的 info 参数设为可选，以保持向后兼容性。
应用以下代码修改：
- parser?: (displayValue: string | undefined, info: { prevValue: string }) => T;
+ parser?: (displayValue: string | undefined, info?: { prevValue: string }) => T;- formatter?: (
- value: T | undefined,
- info: { userTyping: boolean; input: string; prevValue: string },
- ) => string;
+ formatter?: (
+ value: T | undefined,
+ info?: { userTyping: boolean; input: string; prevValue: string },
+ ) => 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.
| parser?: (displayValue: string | undefined, info: { prevValue: string }) => T; | |
| /** Transform `value` to display value show in input */ | |
| formatter?: (value: T | undefined, info: { userTyping: boolean; input: string }) => string; | |
| formatter?: ( | |
| value: T | undefined, | |
| info: { userTyping: boolean; input: string; prevValue: string }, | |
| ) => string; | |
| /** Validate an input string before processing */ | |
| validator?: (input: string) => boolean; | |
| parser?: (displayValue: string | undefined, info?: { prevValue: string }) => T; | |
| /** Transform `value` to display value show in input */ | |
| formatter?: ( | |
| value: T | undefined, | |
| info?: { userTyping: boolean; input: string; prevValue: string }, | |
| ) => string; | |
| /** Validate an input string before processing */ | |
| validator?: (input: string) => boolean; |
This PR enhances the component by adding a new callback:
Validator, which prevents the input from changing if the value does not pass the validation. This is useful to prevent users from inputting unwanted characters in the input.The Parser callback has also been enhanced with a new
infosecond param that returns the previous value. This is useful for formatting or validation purposes when one is modifying the value and needs to compare the old and new values.Summary by CodeRabbit