fix: always change treeProps.expandedKeys to avoid wrong loadling data #576
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及 Changes
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 and nitpick comments (1)
src/OptionList.tsx (1)
Line range hint
1-324: 代码复杂度和可维护性建议虽然主要更改很小，但考虑到组件的复杂性，建议进行以下改进：
组件拆分：考虑将
OptionList组件拆分为更小的子组件，以提高可读性和可维护性。自定义钩子：可以将一些逻辑（如搜索、键盘事件处理）提取到自定义钩子中，以简化主组件。
类型定义：确保所有 props 和状态都有明确的类型定义，这将有助于提高代码的可靠性。
性能优化：检查是否有机会使用
useMemo或useCallback来优化性能，特别是在处理大型树结构时。测试覆盖：确保有足够的单元测试和集成测试来覆盖各种边缘情况和用户交互场景。
您是否需要关于如何实施这些改进的具体建议？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/OptionList.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
src/OptionList.tsx (3)
214-214: 改进：确保 expandedKeys 始终有有效值这个更改通过将
treeProps.expandedKeys设置为mergedExpandedKeys || []来确保expandedKeys始终有一个有效的值。优点：
- 防止了
expandedKeys可能为undefined的情况，提高了代码的健壮性。- 简化了逻辑，使代码更易于理解和维护。
- 避免了在树组件中因
expandedKeys未定义而可能出现的渲染问题。建议：
- 考虑添加一个简短的注释，解释为什么要使用这种方式来设置
expandedKeys，以便future维护者更好地理解这个决策。
Line range hint
1-324: 总结
主要更改（第214行）是一个有效的改进，确保了
expandedKeys的一致性和可靠性。建议验证这个更改对树展开行为的影响，确保在各种场景下都能正常工作。
考虑到组件的复杂性，提出了一些改进建议，包括组件拆分、使用自定义钩子、加强类型定义、性能优化和增加测试覆盖。
总体而言，这是一个积极的改变，提高了代码的健壮性。但鉴于组件的复杂性，建议在合并之前进行彻底的测试。
214-214: 验证：确保树展开行为的一致性这个更改可能会影响树的展开行为。请验证以下几点：
- 当
mergedExpandedKeys为空数组时，树的默认展开状态是否符合预期。- 在各种场景下（如搜索、初始加载等）测试树的展开行为，确保没有意外的副作用。
- 检查是否有任何依赖
expandedKeys为undefined的逻辑，如果有，可能需要相应调整。为了协助验证，您可以运行以下脚本来检查组件中是否有其他地方使用了
expandedKeys：这将帮助您确认是否需要在其他地方进行相应的调整。
✅ Verification successful
为了继续验证，请运行以下脚本以查找
expandedKeys的所有使用位置：
为了继续验证，请运行以下脚本以查找
expandedKeys的所有使用位置：
验证：确保
expandedKeys的一致性根据搜索结果，
expandedKeys在OptionList.tsx和TreeSelect.tsx中被正确使用，没有发现依赖undefined的逻辑。更改确保了expandedKeys总是一个数组，行为一致。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索 src 目录下所有使用 expandedKeys 的地方 rg --type typescript --type tsx 'expandedKeys' srcLength of output: 85
Script:
#!/bin/bash # 搜索 src 目录下所有使用 expandedKeys 的地方 rg --type ts --type tsx 'expandedKeys' srcLength of output: 70
Script:
#!/bin/bash # 搜索 src 目录下所有使用 expandedKeys 的地方，使用 glob 模式匹配 .ts 和 .tsx 文件 rg 'expandedKeys' --glob '*.ts' --glob '*.tsx' srcLength of output: 474
|
需要测试用例。 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/OptionList.tsx
Outdated
| if (mergedExpandedKeys) { | ||
| treeProps.expandedKeys = mergedExpandedKeys; | ||
| } | ||
| treeProps.expandedKeys = mergedExpandedKeys || [] |
Check notice
Code scanning / CodeQL
Semicolon insertion
|
用例挂了，看看 |
…ck avoid triggering all nodes load data
bbb169
force-pushed
the
fix/search-cause-all-load-data
branch
from
d1181f7
to
91d4cac
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/OptionList.tsx (2)
199-207: 优化了 loadData 的处理逻辑这个新增的
loadDataFun函数很好地优化了loadData的处理逻辑，可以避免在搜索时不必要的数据加载。这个改动应该能提高组件的性能。不过，为了提高代码的可读性，我建议添加一些注释来解释这个函数的具体作用和判断逻辑。
建议添加如下注释：
const loadDataFun = React.useMemo(() => { + // 优化 loadData 的处理逻辑，避免在搜索时不必要的数据加载 // should not pass `loadData` when expandedKeys is not changed if (!searchValue && searchExpandedKeys?.length && !mergedExpandedKeys) { return null } + // 在搜索时不加载数据，其他情况下返回原始的 loadData 函数 return searchValue ? null : (loadData as any) }, [searchValue])
发现多个
loadData的使用位置除了
src/OptionList.tsx，loadData还在以下文件中被使用：
examples/dynamic.tsxsrc/TreeSelect.tsxsrc/LegacyContext.tsx请检查这些使用是否会受到此次更改的影响，以确保没有引入新的问题。
🔗 Analysis chain
Line range hint
199-250: 总体评价：有效解决了 TreeSelect 组件的加载问题这些更改很好地解决了 PR 目标中描述的问题，即在搜索结果展开时避免触发不必要的数据加载。通过引入
loadDataFun并在 Tree 组件中应用它，代码现在能够更智能地处理loadData的逻辑。这个优化应该能够：
- 提高组件在搜索场景下的性能
- 避免在失去焦点和清除搜索值时出现错误的加载行为
- 使组件的行为更符合用户的预期
建议在合并这个 PR 之前，确保按照 @afc163 的建议添加相应的测试用例，以验证这些更改的有效性并防止未来的回归。
为了进一步验证这些更改的影响，建议运行以下脚本来检查是否有其他地方使用了
loadData：🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：检查是否有其他地方使用了 loadData # 预期结果：只有在 OptionList.tsx 中有 loadData 的使用 echo "Searching for loadData usage:" rg --type typescript 'loadData' echo "\nSearching for Tree component usage with loadData prop:" ast-grep --lang typescript --pattern 'Tree $props' | rg 'loadData'Length of output: 759
Script:
#!/bin/bash # 描述：检查是否有其他地方使用了 loadData # 预期结果：只有在 OptionList.tsx 中有 loadData 的使用 echo "搜索 loadData 的使用情况：" rg 'loadData' --glob '*.{ts,tsx}' echo "\n搜索 Tree 组件中使用 loadData 属性的位置：" ast-grep --lang typescript --pattern 'Tree <props>'Length of output: 933
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/OptionList.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/OptionList.tsx (1)
250-250: 正确应用了优化后的 loadData 函数将 Tree 组件的
loadData属性更新为loadDataFun是正确的做法。这确保了新的优化逻辑能够在组件中生效，有助于提高性能和用户体验。
|
用例还是挂了 |
用例我后续加哈 但是这个用例我本地跑时全过了的 能帮忙看看是不是流水线的问题呢 |
src/OptionList.tsx
Outdated
| } | ||
|
|
||
| return searchValue ? null : (loadData as any) | ||
| }, [searchValue]) |
There was a problem hiding this comment.
| }, [searchValue]) | |
| }, [searchValue, searchExpandedKeys, mergedExpandedKeys]); |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
tests/Select.SearchInput.spec.js (3)
80-81: 新的测试用例很有价值。这个新增的测试用例很好地验证了清除搜索时不会触发 loadData 的行为。代码实现看起来正确且覆盖了预期的场景。
建议在测试用例开始处添加一个简短的注释，解释这个测试的目的。例如：
// 验证清除搜索时不会触发 loadData这将有助于其他开发者更快地理解测试的意图。
Also applies to: 95-101, 104-106, 129-130
151-155: 额外的测试步骤增强了测试覆盖率。这些新增的步骤很好地验证了在搜索结果中与节点交互时的组件行为。特别是验证
called应该为 0 的断言，确保了loadData不会被不必要地触发。建议在期望断言前添加一个简短的注释，解释为什么
called应该为 0。例如：// 搜索并点击节点不应触发所有节点加载数据 expect(called).toBe(0);这将使测试的意图更加清晰。
Line range hint
1-216: 总体测试覆盖率良好，但结构可以优化。这个测试文件现在包含了多个测试用例，涵盖了 TreeSelect.SearchInput 组件的不同场景，这很好。然而，有一些方面可以进一步改进：
- 代码组织：考虑将相似的测试用例组合在一起，使用
describe块来组织相关的测试。- 减少重复：如前面提到的，可以提取共同的设置和函数到辅助方法中。
- 注释和文档：为每个测试用例添加简短的描述，说明它们测试的具体行为。
- 一致性：确保所有测试用例遵循相同的模式和风格。
建议重构测试文件的结构，可能的话，将其拆分为多个更小的、专注于特定功能的测试文件。例如：
describe('TreeSelect.SearchInput', () => { describe('Search behavior', () => { // 搜索相关的测试 }); describe('LoadData behavior', () => { // 加载数据相关的测试 }); describe('Interaction with expanded keys', () => { // 展开键相关的测试 }); });这样的结构将使测试更容易理解和维护。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/Select.SearchInput.spec.js (7 hunks)
🧰 Additional context used
🔇 Additional comments (2)
tests/Select.SearchInput.spec.js (2)
22-22: 测试断言简化得很好。这个改动使测试更加简洁和易读，同时保持了原有的功能验证。这是一个很好的改进。
49-49: 搜索函数的简化很棒。这个改动使测试代码更加简洁和易于理解，同时保持了原有的功能。这是一个很好的改进，有助于提高代码的可维护性。
tests/Select.SearchInput.spec.js
Outdated
| @@ -1,7 +1,7 @@ | |||
| /* eslint-disable no-undef */ | |||
| import React, { useState } from 'react'; | |||
| import { mount } from 'enzyme'; | |||
| import TreeSelect, { TreeNode } from '../src'; | |||
| import TreeSelect, { TreeNode, TreeSelectProps } from '../src'; | |||
There was a problem hiding this comment.
💡 Codebase verification
TreeSelectProps 未在测试用例中使用，建议移除导入。
- 文件: tests/Select.SearchInput.spec.js 第4行
🔗 Analysis chain
导入 TreeSelectProps 是个好主意。
导入 TreeSelectProps 可以增强类型检查，提高代码的可靠性。这是一个很好的改进。
请确保在测试用例中正确使用了 TreeSelectProps。可以运行以下命令来验证：
由于之前的命令未能识别 typescript 类型，生成以下脚本以重新验证 TreeSelectProps 的使用情况：
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 TreeSelectProps 的使用情况
rg --type typescript "TreeSelectProps" tests/Select.SearchInput.spec.js
Length of output: 106
Script:
#!/bin/bash
# 描述：验证 TreeSelectProps 的使用情况
rg "TreeSelectProps" tests/Select.SearchInput.spec.js
Length of output: 117
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/OptionList.tsx (1)
Line range hint
199-249: 总体代码修改评审这些修改主要集中在优化 TreeSelect 组件的数据加载行为，特别是解决了搜索结果展开所有节点并触发不必要加载行为的问题。总的来说，这些更改是朝着正确方向迈出的一步。
然而，还有一些额外的考虑点：
测试覆盖：确保为新的
loadDataFun逻辑添加单元测试，以验证其在各种情况下的行为。性能影响：虽然这些更改旨在优化性能，但建议进行性能测试，特别是在大型树结构和频繁搜索的情况下。
文档更新：考虑更新组件文档，说明这个新的优化行为，特别是在搜索和展开节点方面的变化。
向后兼容性：确保这些更改不会影响现有的使用方式，特别是对于依赖原有加载行为的用户。
建议在合并这些更改之前，仔细考虑上述几点，并可能进行一些额外的测试和文档工作。这将有助于确保更改的稳定性和可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/OptionList.tsx (2 hunks)
- tests/Select.SearchInput.spec.js (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Select.SearchInput.spec.js
🧰 Additional context used
🔇 Additional comments (1)
src/OptionList.tsx (1)
249-249: loadData 属性的优化修改将
Tree组件的loadData属性更改为loadDataFun是一个很好的优化。这个修改与新引入的loadDataFun逻辑一致，可以在某些情况下防止不必要的数据加载，从而提高性能。为了增加代码的可读性，建议在这行添加一个简短的注释，解释为什么使用
loadDataFun而不是直接使用loadData。例如：- loadData={loadDataFun} + loadData={loadDataFun} // 使用 loadDataFun 来优化数据加载逻辑这样可以帮助其他开发者更快地理解这个改动的目的。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #576 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 517 520 +3
Branches 151 153 +2
=========================================
+ Hits 517 520 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/OptionList.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/OptionList.tsx (2)
199-218: 建议：优化loadDataFun的条件检查逻辑之前的评论者已经指出，
loadDataFun的条件判断可能存在问题，该建议仍然适用。请参考之前的评论，优化条件检查以确保代码的正确性。
260-260: 确认：正确传递loadDataFun给Tree组件在第 260 行，将
loadDataFun作为loadData属性传递给Tree组件，确保了在搜索状态下正确地控制数据加载行为。
bbb169
force-pushed
the
fix/search-cause-all-load-data
branch
from
71f018a
to
8acc1c8
Compare
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/OptionList.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
src/OptionList.tsx (1)
260-260:loadDataFun在 Tree 组件中的使用正确在
Tree组件中使用loadDataFun作为loadData属性的值是正确的。这确保了只有在没有搜索值时才会传递loadData函数，符合loadDataFun的实现逻辑。
Co-authored-by: afc163 <[email protected]>
Co-authored-by: afc163 <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
fix: ant-design/ant-design#51080
这个bug是因为搜索的时候，默认把搜索到的都全部展开:
tree-select/src/OptionList.tsx
Line 127 in de83565
这个导致了在点击时触发了节点的默认加载行为：
https://github.com/react-component/tree/blob/00dc70b20ed0905c69ed7286910eddeb86093ca6/src/TreeNode.tsx#L268
按理来说搜索的时候就应该加载节点，但是这段代码使得它避开了搜索时的加载，而在点击节点的时候失焦，没有searchValue，所以就会触发节点的默认加载行为了：
tree-select/src/OptionList.tsx
Line 240 in de83565
另外最关键的是，点击节点失焦时没有重置expandKeys，导致里面的节点收了错误的信息：
tree-select/src/OptionList.tsx
Line 118 in de83565
此刻由于
treeExpandedKeys，expandedKeys都是undefined导致这行代码没有执行：
tree-select/src/OptionList.tsx
Line 215 in de83565
所以改掉这行就行
Summary by CodeRabbit
Summary by CodeRabbit
OptionList组件的loadData属性处理逻辑，确保在不同状态下的正确传递。loadDataFun，优化了组件性能。TreeSelect.SearchInput组件的测试用例，增强了可读性和可维护性。loadData。loadData。