feat: supplement maxCount logic for complicated cases #602
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改涉及多个组件的修改，主要集中在 Changes
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)tests/Select.warning.spec.js (3)
此处在使用 maxCount 与 showCheckedStrategy=SHOW_ALL、treeCheckStrictly=false 的组合时，测试能够完整捕获到预期的警告信息，能够有效提升对警告逻辑的回归测试覆盖率。
此处再一次针对 maxCount 与 showCheckedStrategy=SHOW_PARENT 的组合进行测试，能够进一步确保不同策略下触发警告的逻辑一致且功能正常。
此处通过设置 treeCheckStrictly=true 并与 showCheckedStrategy=SHOW_ALL 一起使用，明确了不会出现警告信息。该测试场景有效确保在满足条件时不会产生误报。 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 #602 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 561 598 +37
Branches 167 186 +19
=========================================
+ Hits 561 598 +37 ☔ View full report in Codecov by Sentry. |
![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 (2)
src/OptionList.tsx (1)
171-221: 建议优化nodeDisabled函数的可读性当前的
nodeDisabled函数逻辑较为复杂，嵌套深度较大，可能影响代码的可读性和维护性。建议将复杂的逻辑拆分为多个小函数，或在关键部分添加注释，以提高代码的可读性。examples/mutiple-with-maxCount.tsx (1)
87-87: 考虑清理已注释的showCheckedStrategy属性在第 87 行，
showCheckedStrategy="SHOW_PARENT"被注释掉。若不再需要，建议删除该注释代码；若需保留，建议添加说明以解释注释原因。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
examples/mutiple-with-maxCount.tsx(2 hunks)src/OptionList.tsx(3 hunks)src/TreeSelect.tsx(2 hunks)src/TreeSelectContext.ts(2 hunks)
🔇 Additional comments (4)
src/TreeSelect.tsx (2)
628-629: 确保在上下文中正确传递 maxCount 和 showCheckedStrategy
将 maxCount 和 mergedShowCheckedStrategy 添加到 treeSelectContext 中，确保子组件能够访问并使用这些新添加的配置属性。
646-646: 在依赖项数组中包含 mergedShowCheckedStrategy
在 useMemo 的依赖项数组中添加 mergedShowCheckedStrategy，确保当该策略更改时，treeSelectContext 能够正确地重新计算和更新。
src/TreeSelectContext.ts (2)
4-4: 引入 CheckedStrategy 类型
从 './utils/strategyUtil' 导入 CheckedStrategy，以便在接口中使用该类型，增强类型检查和代码可读性。
20-21: 在 TreeSelectContextProps 接口中添加 maxCount 和 showCheckedStrategy 属性
为 TreeSelectContextProps 接口新增可选的 maxCount 和 showCheckedStrategy 属性，使上下文配置更灵活，支持最大选择数量和选择显示策略的定制。
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
tests/Select.maxCount.spec.tsx (1)
449-470: 建议将代码注释统一为英文为了保持代码库的一致性和方便全球开发者理解，建议将测试代码中的中文注释翻译为英文。
Also applies to: 493-502
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
examples/mutiple-with-maxCount.tsx(2 hunks)src/OptionList.tsx(3 hunks)src/TreeSelect.tsx(3 hunks)src/TreeSelectContext.ts(2 hunks)tests/Select.maxCount.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/TreeSelectContext.ts
- examples/mutiple-with-maxCount.tsx
🧰 Additional context used
🪛 eslint
src/OptionList.tsx
[error] 178-178: 'fieldNames' is already declared in the upper scope on line 46 column 5.
(@typescript-eslint/no-shadow)
🔇 Additional comments (1)
tests/Select.maxCount.spec.tsx (1)
375-503: 测试用例正确地覆盖了复杂场景
新增的测试用例有效地验证了在复杂树结构下 maxCount 的行为，逻辑清晰，符合预期。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/OptionList.tsx (1)
164-176: 建议优化缓存重置逻辑当前实现在
checkedKeys或maxCount变化时会清空整个缓存。考虑以下优化建议：
- 可以只清除受影响的缓存项，而不是清空整个 Map
- 考虑添加缓存大小限制，避免在大型树结构中内存占用过大
const resetCache = React.useCallback(() => { - disabledCacheRef.current.clear(); + // 只清除受影响的缓存项 + const affectedPrefix = `${checkedKeys.join(',')}-${maxCount}`; + for (const key of disabledCacheRef.current.keys()) { + if (key.includes(affectedPrefix)) { + disabledCacheRef.current.delete(key); + } + } lastCheckedKeysRef.current = [...checkedKeys]; lastMaxCountRef.current = maxCount; }, [checkedKeys, maxCount]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/OptionList.tsx(3 hunks)src/TreeSelectContext.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/TreeSelectContext.ts
🔇 Additional comments (2)
src/OptionList.tsx (2)
12-16: 导入和上下文变更看起来不错！
新增的导入和上下文值很好地支持了 maxCount 和 showCheckedStrategy 的功能实现。
Also applies to: 53-54
192-226:
性能优化警告：需要处理非虚拟列表场景
根据之前 @zombieJ 的评论，当前实现在未启用虚拟列表时可能会导致性能雪崩。建议：
- 在非虚拟列表场景下，实现节点懒加载机制
- 考虑使用 worker 线程处理复杂计算
- 实现节点计算结果的批量更新
const nodeDisabled = useEvent((node: DataNode) => {
+ // 非虚拟列表场景下的性能优化
+ if (!virtual) {
+ const batchSize = 100;
+ const throttleMs = 16;
+
+ // 实现节点计算的批量处理逻辑
+ // TODO: 添加具体实现
+ }
// 原有逻辑保持不变
const nodeValue = node[fieldNames.value];
// ...
});
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/TreeSelect.tsx (2)
Line range hint
89-89: 建议为 maxCount 属性添加 JSDoc 注释为了提高代码的可维护性和使用体验，建议添加 JSDoc 注释来说明 maxCount 属性的用途、限制条件和使用示例。
建议添加如下注释：
+ /** + * 最大可选择的节点数量 + * @default undefined + */ maxCount?: number;
Line range hint
391-476: 建议重构 onOptionSelect 函数以提高可维护性当前的
onOptionSelect函数职责过重，包含了选择逻辑、树形结构处理和 maxCount 限制等多个功能。建议将其拆分为更小的、职责单一的函数。建议按照以下方式重构：
const canSelectMore = (newValue: SafeKey) => { if (!mergedMultiple || maxCount === undefined) { return true; } const newNodeCount = getNodeCount(newValue, keyEntities); return calculateSelectedCount + newNodeCount <= maxCount; }; const handleTreeConduction = (newRawValues: SafeKey[], selected: boolean) => { const { missingRawValues, existRawValues } = splitRawValues(newRawValues); const keyList = existRawValues.map(val => valueEntities.get(val).key); const { checkedKeys } = selected ? conductCheck(keyList, true, keyEntities) : conductCheck(keyList, { checked: false, halfCheckedKeys: rawHalfCheckedValues }, keyEntities); return [...missingRawValues, ...checkedKeys.map(key => keyEntities[key as SafeKey].node[mergedFieldNames.value])]; }; const onOptionSelect = (selectedKey: SafeKey, { selected, source }) => { const entity = keyEntities[selectedKey]; const node = entity?.node; const selectedValue = node?.[mergedFieldNames.value] ?? selectedKey; if (!mergedMultiple) { triggerChange([selectedValue], { selected: true, triggerValue: selectedValue }, 'option'); onSelect?.(selectedValue, fillLegacyProps(node)); return; } if (selected && !canSelectMore(selectedValue)) { return; } const newRawValues = selected ? [...rawValues, selectedValue] : rawCheckedValues.filter(v => v !== selectedValue); const finalValues = treeConduction ? handleTreeConduction(newRawValues, selected) : newRawValues; triggerChange(finalValues, { selected, triggerValue: selectedValue }, source || 'option'); if (selected) { onSelect?.(selectedValue, fillLegacyProps(node)); } else { onDeselect?.(selectedValue, fillLegacyProps(node)); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/OptionList.tsx(3 hunks)src/TreeSelect.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/OptionList.tsx
🔇 Additional comments (1)
src/TreeSelect.tsx (1)
Line range hint 4-4: 建议补充 maxCount 功能的测试用例
为确保 maxCount 功能在各种场景下的正确性，建议补充以下测试场景：
- 不同 showCheckedStrategy 策略下的 maxCount 限制
- 父子节点选择时的 maxCount 计数
- 取消选择时的状态恢复
- 边界条件测试（maxCount 为 0、1 等）
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/OptionList.tsx (3)
12-12: 引入的 FieldNames 在本文件中未被使用
根据静态分析提示，FieldNames 当前未用到。若确无后续引用，可考虑移除该 import。🧰 Tools
🪛 eslint
[error] 12-12: 'FieldNames' is defined but never used.
(@typescript-eslint/no-unused-vars)
15-15: formatStrategyValues 未被使用
和上一条相似，若无后续使用，建议删除 import，避免无用代码。🧰 Tools
🪛 eslint
[error] 15-15: 'formatStrategyValues' is defined but never used.
(@typescript-eslint/no-unused-vars)
163-165: 通过 disabledCacheRef 提升节点禁用检查的性能
缓存机制可避免在大规模树中反复计算。此实现能提高渲染效率，思路不错。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/mutiple-with-maxCount.tsx(2 hunks)src/OptionList.tsx(3 hunks)src/TreeSelect.tsx(4 hunks)src/TreeSelectContext.ts(2 hunks)src/utils/warningPropsUtil.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/mutiple-with-maxCount.tsx
🧰 Additional context used
🪛 eslint
src/OptionList.tsx
[error] 12-12: 'FieldNames' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 15-15: 'formatStrategyValues' is defined but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (13)
src/utils/warningPropsUtil.ts (4)
13-14: 解构新增属性 showCheckedStrategy、maxCount 的用途明确
这两个属性后续在文件中均有实际使用，逻辑一致性良好，无明显问题。
25-25: 无效对象检测逻辑看起来合理
使用 toArray(value).every(...) 确保 value 均为合法对象，有助于减少非标准结构传入的风险。
38-38: 空行变更
此处仅为空行，不影响功能，可忽略。
39-44: 对特定 showCheckedStrategy 提示不兼容 maxCount
在 SHOW_ALL 或 SHOW_PARENT 场景下禁止 maxCount，能有效避免策略冲突。此处警告信息清晰明确。
src/TreeSelectContext.ts (1)
19-23: 增加了 leftMaxCount、leafCountOnly、valueEntities 三个上下文字段
引入这些字段后，可在下游组件统一读取与“剩余可选数量”和“叶节点计数策略”有关的信息，设计思路清晰，可维护性提高。
src/OptionList.tsx (4)
51-53: 解构获取 leftMaxCount、leafCountOnly、valueEntities
这些属性通过 TreeSelectContext 提供，可配合节点选择逻辑在大规模树中正确应用 maxCount 和叶节点限制。实现思路合理。
166-172: 基于 leftMaxCount 的 useEffect 清理缓存
当可选择数量重新计算后，及时清理缓存可防止过时状态。逻辑完善，值得肯定。
173-195: getDisabledWithCache：综合判断节点子元素规模
通过 children 长度、已选节点等信息判断是否禁用，且采用缓存避免重复计算。整体实现对大树有较好扩展性。
197-216: nodeDisabled 函数内多分支判断可维持代码简洁性
对于已勾选节点直接 return false 并终止其余判断；当 leftMaxCount 不存在或大于零时再做判断。实现逻辑严谨，清晰易读。
src/TreeSelect.tsx (4)
411-421: mergedMaxCount 逻辑简化选项
仅在多选模式下且识别到特定策略时才生效，可减少无关场景的干扰，设计合理。
436-438: 在触发更新前校验是否超过 mergedMaxCount
此检查防止用户越过限制强行添加过多条目，保证组件行为符合 maxCount 规范。
632-635: leftMaxCount 的计算逻辑
基于当前已显示数量判断剩余可选数量，为后续子组件快速判断是否可选提供直接依据。
650-654: leafCountOnly 条件与 maxCount、treeCheckStrictly、treeCheckable 的联动
当启用 SHOW_CHILD 并禁用严格模式时，才采用叶节点计数，这能更准确地匹配业务需求。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/OptionList.tsx (3)
162-170: 缓存实现合理，建议优化内存使用缓存 disabled 状态的实现可以提高性能，但建议考虑以下优化：
- 在组件卸载时清理缓存
- 考虑设置缓存大小限制，避免内存持续增长
建议添加以下清理逻辑：
+ React.useEffect(() => { + return () => { + disabledCacheRef.current.clear(); + }; + }, []);
172-193: 建议增加输入验证
getDisabledWithCache函数的实现逻辑正确，但建议增加以下安全检查：
- 验证 value 是否为有效值
- 验证 entity 是否存在
- 处理 children 为 undefined 的情况
建议修改如下：
function getDisabledWithCache(node: DataNode) { + if (!node || !fieldNames.value) { + return false; + } const value = node[fieldNames.value]; if (!disabledCacheRef.current.has(value)) { const entity = valueEntities.get(value); + if (!entity) { + return false; + } - const isLeaf = (entity.children || []).length === 0; + const children = entity.children || []; + const isLeaf = children.length === 0;
195-215: 建议完善函数文档
nodeDisabled函数实现了复杂的禁用状态逻辑，建议添加详细的文档说明：
- 各个条件分支的判断依据
- 性能考虑说明
- 与 maxCount 的关系说明
建议添加如下注释：
+/** + * 计算节点是否应该被禁用 + * @param node - 要检查的节点 + * @returns {boolean} 是否禁用 + * + * 禁用逻辑： + * 1. 已选中的节点永远不禁用 + * 2. 没有 maxCount 限制时不禁用 + * 3. 剩余可选数量为 0 时禁用 + * 4. 叶子节点计数模式下，根据缓存计算禁用状态 + */ const nodeDisabled = useEvent((node: DataNode) => {tests/Select.maxCount.spec.tsx (1)
349-476: 建议增加更多测试场景当前的复杂场景测试覆盖了基本情况，建议增加以下测试场景：
- 动态变更 maxCount 值的情况
- 搜索过滤后的选择限制
- 异步加载子节点的场景
- 边界值测试（maxCount 为 1 的特殊情况）
建议添加如下测试用例：
it('should handle dynamic maxCount changes', () => { const { rerender } = render( <TreeSelect treeData={complexTreeData} treeCheckable multiple maxCount={3} /> ); // 测试动态改变 maxCount rerender( <TreeSelect treeData={complexTreeData} treeCheckable multiple maxCount={2} /> ); });src/TreeSelect.tsx (1)
436-438: 建议优化 maxCount 检查时机当前在
triggerChange中检查 maxCount 限制可能会导致不必要的计算。建议将检查提前到onOptionSelect中进行。建议重构为：
const onOptionSelect = React.useCallback( (selectedKey: SafeKey, { selected, source }: { selected: boolean; source: SelectSource }) => { + // Early check for maxCount + if (mergedMaxCount && selected && rawValues.length >= mergedMaxCount) { + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/OptionList.tsx(2 hunks)src/TreeSelect.tsx(4 hunks)tests/Select.maxCount.spec.tsx(1 hunks)
🔇 Additional comments (3)
src/OptionList.tsx (1)
50-52: 新增的上下文属性设计合理
新增的三个上下文属性很好地支持了 maxCount 的功能实现：
leftMaxCount: 用于追踪剩余可选数量leafCountOnly: 控制节点计数策略valueEntities: 提供节点关系访问
src/TreeSelect.tsx (2)
411-420: mergedMaxCount 计算逻辑清晰
mergedMaxCount 的计算考虑了多种场景：
- 多选模式
- 显示策略
- 严格检查模式
- 可选状态
实现逻辑清晰合理。
632-635: 上下文属性设计合理
新增的上下文属性设计合理：
leftMaxCount计算准确leafCountOnly条件判断完整valueEntities提供必要的数据支持
TreeSelect maxCount 功能优化
修复了 TreeSelect 组件在设置 maxCount 时对树形结构处理的问题，使其能够正确处理父子节点关系和树的传导规则。
主要改动
具体改进
相关问题
修复了以下场景的问题：
Summary by CodeRabbit
Summary by CodeRabbit
新功能
TreeSelect组件中引入了最大选择数量 (maxCount) 和剩余选择数量 (leftMaxCount) 属性。OptionList组件的节点选择逻辑，增强了对禁用状态的管理。bug 修复