fix: restore load switcher after clearing search#657
Conversation
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough重写 OptionList 中 mergedExpandedKeys 的 useMemo 分支与依赖，并收窄 hasLoadDataFn 的 memo 比较为仅基于 searchValue；新增测试验证清空搜索值后 load switcher 的可见性与 loadData 调用语义。 Changes搜索与懒加载状态管理
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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/OptionList.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. tests/Select.loadData.spec.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the expanded keys logic in OptionList.tsx to correctly handle cases when clearing the search value, specifically when loadData is provided and treeDefaultExpandAll is false. It also simplifies the search value comparison logic. A corresponding test case has been added in Select.loadData.spec.tsx to ensure the load switcher is preserved after clearing the search value. There are no review comments to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/Select.loadData.spec.tsx (1)
47-69: ⚡ Quick win考虑增强测试覆盖：验证清空搜索后的实际加载行为
当前测试验证了清空搜索后加载切换器（load switcher）的存在性，这对于捕获回归问题已经足够。但是，测试没有验证点击该切换器是否真正触发
loadData回调，这是修复的完整功能验证的一部分。建议考虑添加以下测试步骤以提高覆盖率：
💡 扩展测试覆盖的建议
在 line 68 之后添加：
// 验证清空搜索后点击加载切换器能够触发 loadData fireEvent.click(container.querySelector('.rc-tree-select-tree-switcher_close')!); await act(async () => { await Promise.resolve(); }); expect(loadData).toHaveBeenCalledTimes(1);这将确保修复不仅恢复了切换器的显示，还恢复了其完整的加载功能。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Select.loadData.spec.tsx` around lines 47 - 69, Extend the existing test for TreeSelect to also assert that clicking the load switcher triggers the loadData callback: after the existing assertions, query the '.rc-tree-select-tree-switcher_close' element, fireEvent.click on it, wrap any async resolution in act(async () => { await Promise.resolve(); }), then assert expect(loadData).toHaveBeenCalledTimes(1); this ensures the TreeSelect loadData handler (passed as loadData prop) is actually invoked when the switcher is clicked.src/OptionList.tsx (1)
322-327: ⚡ Quick win确认 hasLoadDataFn 依赖项的必要性
hasLoadDataFn的useMemo依赖项包含[searchValue, treeExpandedKeys || expandedKeys]，但自定义比较函数仅检查searchValue是否变化，完全忽略第二个依赖项。如果第二个依赖项不影响重新计算逻辑，建议从依赖数组中移除它以提高代码清晰度：
♻️ 简化依赖项的建议
const hasLoadDataFn = useMemo( () => (searchValue ? false : true), - [searchValue, treeExpandedKeys || expandedKeys], + [searchValue], ([preSearchValue], [nextSearchValue]) => preSearchValue !== nextSearchValue, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OptionList.tsx` around lines 322 - 327, The useMemo for hasLoadDataFn declares dependencies [searchValue, treeExpandedKeys || expandedKeys] but the custom comparator only compares searchValue, so remove the unused second dependency and the now-unnecessary eslint-disable; update the useMemo call for hasLoadDataFn to depend solely on searchValue (and keep the comparator that compares previous vs next searchValue) or alternatively expand the comparator to include treeExpandedKeys/expandedKeys if those should trigger recomputation—locate the hasLoadDataFn useMemo and either remove "treeExpandedKeys || expandedKeys" from the deps array and delete the eslint-disable-next-line, or adjust the comparator to compare that second value as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/OptionList.tsx`:
- Around line 322-327: The useMemo for hasLoadDataFn declares dependencies
[searchValue, treeExpandedKeys || expandedKeys] but the custom comparator only
compares searchValue, so remove the unused second dependency and the
now-unnecessary eslint-disable; update the useMemo call for hasLoadDataFn to
depend solely on searchValue (and keep the comparator that compares previous vs
next searchValue) or alternatively expand the comparator to include
treeExpandedKeys/expandedKeys if those should trigger recomputation—locate the
hasLoadDataFn useMemo and either remove "treeExpandedKeys || expandedKeys" from
the deps array and delete the eslint-disable-next-line, or adjust the comparator
to compare that second value as well.
In `@tests/Select.loadData.spec.tsx`:
- Around line 47-69: Extend the existing test for TreeSelect to also assert that
clicking the load switcher triggers the loadData callback: after the existing
assertions, query the '.rc-tree-select-tree-switcher_close' element,
fireEvent.click on it, wrap any async resolution in act(async () => { await
Promise.resolve(); }), then assert expect(loadData).toHaveBeenCalledTimes(1);
this ensures the TreeSelect loadData handler (passed as loadData prop) is
actually invoked when the switcher is clicked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f4cc9b1-8253-4fd9-9dc1-b57a770822dc
📒 Files selected for processing (2)
src/OptionList.tsxtests/Select.loadData.spec.tsx
| if (searchExpandedKeys && loadData && !treeDefaultExpandAll) { | ||
| return expandedKeys || []; | ||
| } | ||
| return expandedKeys; |
There was a problem hiding this comment.
| if (searchExpandedKeys && loadData && !treeDefaultExpandAll) { | |
| return expandedKeys || []; | |
| } | |
| return expandedKeys; | |
| return expandedKeys || []; |
这里和直接这样有啥区别？
There was a problem hiding this comment.
这里主要差异在 treeDefaultExpandAll 场景。
初始化时如果没有传 treeDefaultExpandedKeys，expandedKeys 是 undefined。直接 return expandedKeys || [] 会返回 []，下游 Tree 收到 expandedKeys={[]} 后会进入受控展开状态，defaultExpandAll 就不会生效，所以默认展开会失效。
2 participants
修复 ant-design 中反馈的 TreeSelect 异步加载问题：ant-design/ant-design#55675
当用户在 TreeSelect 中搜索后再清空搜索内容时，异步节点仍然会被渲染成 noop switcher，导致原本用于展开/触发
loadData的入口没有恢复，用户无法继续加载该节点。这个修改会在清空搜索后恢复异步节点正常的加载/展开 switcher，同时保持搜索过程中不触发
loadData的现有行为。Summary by CodeRabbit
Bug Fixes
Tests