fix: antd-issue-51248 #541
Conversation
Walkthrough此次更改涉及 Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #541 +/- ##
==========================================
+ Coverage 99.67% 99.83% +0.16%
==========================================
Files 23 23
Lines 611 611
Branches 185 176 -9
==========================================
+ Hits 609 610 +1
+ Misses 2 1 -1 ☔ View full report in Codecov by Sentry. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![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 and nitpick comments (1)
src/hooks/useSearchConfig.ts (1)
Line range hint
26-30: 代码变更符合预期，但可以进一步优化这些更改很好地解决了 PR 中描述的问题。将
limit设置为false而不是删除它是一个好方法，可以保持对象结构的一致性。添加警告也有助于开发人员在开发过程中捕获潜在问题。为了提高代码的可读性和类型安全性，我建议将条件检查稍作修改：
-if ((searchConfig.limit as number) <= 0) { +if (typeof searchConfig.limit === 'number' && searchConfig.limit <= 0) { searchConfig.limit = false; if (process.env.NODE_ENV !== 'production') { warning(false, "'limit' of showSearch should be positive number or false."); } }这样可以避免使用类型断言，并更明确地表达意图。
npm run test ./tests/search.limit.spec.tsx |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
tests/search.limit.spec.tsx (1)
14-29: 选项定义结构合理，但可以稍作改进。选项的定义结构很好，创建了一个包含100个子选项的父选项'Asia'。这对于测试搜索功能和性能很有帮助。
建议：考虑使用
Array.from和箭头函数来简化子选项的创建，使代码更简洁：options[0].children = Array.from({ length: 100 }, (_, i) => ({ id: i, label: `label${i}`, value: `value${i}`, }));这样可以减少代码行数并提高可读性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/search.limit.spec.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
tests/search.limit.spec.tsx (2)
1-4: 导入语句看起来很好。导入语句包含了测试所需的所有必要模块，包括React、Cascader组件和Enzyme测试工具。没有未使用的导入。
7-13:doSearch辅助函数实现得很好。这个函数很好地封装了模拟搜索输入的逻辑，使用了Enzyme的
simulate方法来模拟输入变化。这将使测试用例更加清晰和可维护。
tests/search.limit.spec.tsx
Outdated
| it('limit', () => { | ||
| const wrapper = mount( | ||
| <Cascader | ||
| options={options} | ||
| open | ||
| showSearch={{ | ||
| limit: false, | ||
| }} | ||
| />, | ||
| ); | ||
|
|
||
| doSearch(wrapper, 'as'); | ||
| const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | ||
| expect(itemList).toHaveLength(itemList.length); | ||
| }); |
There was a problem hiding this comment.
测试用例可以更具体和描述性。
当前的测试用例结构正确，但可以进行以下改进：
-
测试描述应更具体，例如："当showSearch.limit设置为false时，应显示所有匹配项"。
-
断言可以更明确。目前的断言只是检查项目列表的长度与自身相等，这并不能真正验证功能。建议改为：
expect(itemList).toHaveLength(101); // 100个子项 + 1个父项
-
可以添加一个反例，测试当
limit设置为一个数值时的行为。 -
考虑测试搜索结果的内容，而不仅仅是数量。
这些改进将使测试更加健壮和有意义。
如果您需要，我可以帮助您重写这个测试用例。
| options={options} | ||
| open | ||
| showSearch={{ | ||
| limit: false, |
There was a problem hiding this comment.
| limit: false, | |
| limit: 0, |
There was a problem hiding this comment.
是不是用 0 比较好， pr修复的是<= 0的情况下，limit变成undefined然后又重新默认值50了，false本身是好的。
There was a problem hiding this comment.
定位的问题出在limit = false时会走进 if的判断，导致 limit被 delete，所以测试用例里是用的limit:false， 或者直接再加一层判断，规避上述场景，这样原本的逻辑没变，false也可以当 no limit处理
There was a problem hiding this comment.
如果误导你了不好意思，还是等大佬们建议。我评论的是测试用例用个 limit: 0,，因为之前limit: false的情况本来就是好的，是limit：0才出了问题, 感觉这里面的业务逻辑是 searchConfig.limit = 0; or searchConfig.limit = false; 可能都行。你改回了这个delete不是又导致了被默认赋值50的问题。
There was a problem hiding this comment.
测试用例我加上 limit:0/false/20 了，limit:false 是有问题的，实际使用依然会走进 if，大佬你可以试一下，官网 demo 里改成limit:false 依然是50 条
There was a problem hiding this comment.
delete之后会变成undefined所以重新赋值了，所以是不是给个默认复制 0 可以呢
There was a problem hiding this comment.
所以这里的逻辑 limit = false或者<=0都是展示全量数据，我最开始的修改进入判断后都是直接limit设为 false应该就可以的
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tests/search.limit.spec.tsx (2)
29-59: 建议改进前两个测试用例。这两个测试用例的结构正确，但有以下几点可以改进：
测试描述应更具体，例如：
- "当showSearch.limit设置为false时，应显示所有匹配项"
- "当showSearch.limit设置为0时，应使用默认限制50"
第一个测试用例（
limit: false）的断言不够有效。建议改为：expect(itemList).toHaveLength(101); // 100个子项 + 1个父项考虑添加对搜索结果内容的检查，而不仅仅是数量。例如，验证第一个和最后一个项目的标签。
这些改进将使测试更加健壮和有意义。
61-76: 第三个测试用例实现正确，建议小幅改进。这个测试用例正确地验证了自定义限制（20）的情况。为了进一步提高其可读性和明确性，建议：
使用更具描述性的测试名称，例如：
it('当showSearch.limit设置为20时，应只显示20个匹配项', () => { // ... 现有的测试代码 ... });考虑添加一个额外的断言来验证显示的项目是否正确，例如检查第一个和最后一个项目的标签。
这些小改动将使测试的目的更加清晰。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/hooks/useSearchConfig.ts (1 hunks)
- tests/search.limit.spec.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useSearchConfig.ts
🧰 Additional context used
🔇 Additional comments (2)
tests/search.limit.spec.tsx (2)
1-13: 导入和辅助函数实现正确。导入语句和
doSearch辅助函数的实现看起来都很合适，没有发现任何问题。
14-28: 选项数组创建正确。为测试目的创建的选项数组结构合理，包含一个父项"Asia"和100个子项。这提供了足够的数据来测试搜索限制功能。
|
Deployment failed with the following error: |
|
CI 挂了，奇怪。看起来不像是和这个 PR 有关的 重跑后好了，似乎是 bun 对 deps 的解析有点问题。偶尔会把底层的依赖强行提上来。 |
|
@dongbanban 
|
* fix: antd-issue-51248 * feat: add test for 51248 * feat: update test for limit * feat: update test for limit * feat: update test for limit * feat: update test for limit
fix ant-design/ant-design#51248
searchConfig.limit <= 0 时重新赋值为 false
Summary by CodeRabbit
false而不是被删除。false，将发出警告。Cascader.Search组件的测试套件，确保组件在不同限制条件下的行为符合预期。