-
-
Notifications
You must be signed in to change notification settings - Fork 262
feat: Support custom rendering #792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Walkthrough为 Menu 引入可自定义的 itemRender 渲染回调,并将其类型与上下文贯穿到 Menu、SubMenu、MenuItem、MenuItemGroup、Divider、类型定义、示例与测试中;示例与测试新增用例将 item 包裹为外链。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as 开发者
participant Menu as Menu
participant Context as MenuContext
participant Component as 子组件(Item/Group/SubMenu/Divider)
participant itemRender as itemRender 回调
participant Renderer as 渲染管线
Dev->>Menu: 提供 items 与 itemRender
Menu->>Context: 将 itemRender 注入上下文
Renderer->>Component: 构建 originNode / childList
alt itemRender 存在(prop 或 context)
Component->>itemRender: itemRender(originNode/childList, { item, keys, eventOpt })
itemRender-->>Component: customNode
Component-->>Renderer: 返回 customNode
else 无 itemRender
Component-->>Renderer: 返回 originNode
end
Renderer-->>Menu: 完整节点树
Menu-->>Dev: 渲染完成
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 注意可能需要额外关注:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-10-23T06:01:34.975ZApplied to files:
📚 Learning: 2025-10-23T07:19:07.493ZApplied to files:
🧬 Code graph analysis (2)src/SubMenu/index.tsx (1)
src/MenuItem.tsx (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/utils/nodeUtil.tsx (2)
55-56:prefixCls可能为空时生成异常类名当
prefixCls未传入时,当前会生成class="undefined-item-extra"。建议在渲染extra时判断prefixCls是否存在再拼接类名(见上方 diff 示例)。请确认是否存在调用
parseItems时未传入prefixCls的路径;必要时可为prefixCls提供默认值或在入口处断言。
27-29: 命名一致性:originNodevsoriginalNode接口使用
originalNode,实现中变量命名为originNode。建议统一命名,降低心智负担与搜索成本。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/examples/items.tsx(1 hunks)src/interface.ts(1 hunks)src/utils/nodeUtil.tsx(1 hunks)tests/MenuItem.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/MenuItem.spec.tsx (1)
docs/examples/items.tsx (1)
Menu(7-83)
src/utils/nodeUtil.tsx (1)
src/MenuItem.tsx (1)
render(40-63)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #792 +/- ##
==========================================
- Coverage 99.72% 99.19% -0.54%
==========================================
Files 26 26
Lines 727 743 +16
Branches 199 209 +10
==========================================
+ Hits 725 737 +12
- Misses 2 5 +3
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
02bad65 to
d45697e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Menu.tsx (1)
65-66: 命名一致性与语义约束建议:originalNode -> originNode,并在实现上确保不替换
- 命名建议:其他模块(如 utils/nodeUtil.tsx)普遍使用
originNode。为避免文档/类型注释不一致,建议统一为originNode。功能无差异。- 语义约束:
Overflow会“原样”渲染子节点为<ul>的直接子元素。请确保itemsRender的应用点在<li>内部(例如包裹标题/内容区域),而不是替换<li>本身;否则会造成无效 DOM(<ul>子元素必须是<li>)并破坏可访问性与样式选择器。这一点需在convertItemsToNodes/parseItems的实现中得到保证。可以按如下方式微调类型签名中的参数名(仅注释层面的统一,不影响运行时):
- itemsRender?: (originalNode: React.ReactNode, item: NonNullable<ItemType>) => React.ReactNode; + itemsRender?: (originNode: React.ReactNode, item: NonNullable<ItemType>) => React.ReactNode;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/examples/items.tsx(1 hunks)src/Menu.tsx(4 hunks)src/utils/nodeUtil.tsx(4 hunks)tests/MenuItem.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utils/nodeUtil.tsx
- docs/examples/items.tsx
- tests/MenuItem.spec.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Menu.tsx (2)
src/interface.ts (1)
ItemType(77-77)src/utils/nodeUtil.tsx (1)
parseItems(73-96)
🔇 Additional comments (2)
src/Menu.tsx (2)
248-251: 从 props 解构 itemsRender ✅按需解构后向解析链路透传,符合预期使用方式。
33-35: 验证通过:./SubMenu中存在SemanticName类型导出及默认导出SubMenu
- 在
src/SubMenu/index.tsx第 25 行已声明export type SemanticName = 'list' | 'listTitle'- 在同文件第 428 行已声明
export default SubMenu将 type-only 引入与默认导入分离是安全的,无需额外更改。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tests/MenuItem.spec.tsx (1)
232-265: 补充对 DOM 语义与交互不回归的断言(与先前建议一致)当前仅断言
<a>存在。为防回归,建议同时校验:
ul的直接子节点均为li;- 自定义渲染后键盘/点击交互仍可触发;
- 禁用态不受影响。
示例补充(可直接追加在本测试用例末尾):
const link = container.querySelector('a'); expect(link).toHaveAttribute('href', 'https://ant.design'); + + // 结构语义:ul 的直接子节点应为 li + const ul = container.querySelector('ul.rc-menu-root') || container.querySelector('ul'); + const directChildren = Array.from(ul!.children); + directChildren.forEach(node => expect(node.tagName.toLowerCase()).toBe('li')); + + // 可选:验证包装后键盘交互不回归(需要为某项传入 onClick) + // const secondLi = container.querySelectorAll('li.rc-menu-item')[1]; + // fireEvent.keyDown(secondLi, { which: KeyCode.ENTER, keyCode: KeyCode.ENTER }); + // expect(......).toHaveBeenCalled();src/utils/nodeUtil.tsx (2)
62-66: 避免在所有分支统一后置包装 originNode分支外层统一
return itemRender(originNode, opt)会让group/submenu/divider也被外层包裹,风险更大。建议移除该段,仅在 MenuItem 分支内应用(见上方修改):- if (typeof itemRender === 'function') { - return itemRender(originNode, opt); - } - return originNode; + return originNode;
52-59: 将 itemRender 限定在“内容层”,保持为最外层,避免破坏语义 当前对
originNode(实际是<li>)整体做itemRender后置包装,允许外层包裹/替换,可能导致ul > a > li或ul > div。建议仅对<li>内部内容进行定制:- else { - originNode = ( - <MergedMenuItem key={mergedKey} {...restProps} extra={extra}> - {label} - {(!!extra || extra === 0) && ( - <span className={`${prefixCls}-item-extra`}>{extra}</span> - )} - </MergedMenuItem> - ); - } + else { + // 仅对 li 的内容层应用 itemRender,确保最外层仍为 <li> + const content = ( + <> + {label} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )} + </> + ); + originNode = ( + <MergedMenuItem key={mergedKey} {...restProps} extra={extra}> + {typeof itemRender === 'function' ? itemRender(content, opt as any) : content} + </MergedMenuItem> + ); + }要点:
- 仅在 MenuItem 分支应用
itemRender;- 对
prefixCls加保护,避免渲染出undefined-item-extra。
🧹 Nitpick comments (3)
tests/Responsive.spec.tsx (2)
33-35: 用 Map 重置全局状态不错;补充类型声明以避免 TS 报错当前对
global.resizeProps的读写在 TS 下可能触发 “Property 'resizeProps' does not exist on type 'Global'” 的类型错误。建议补充全局声明,或将变量挂在globalThis上。可在测试入口或此文件顶部追加类型声明(二选一):
// 方案 A:全局声明 declare global { // 注意与 jest 声明合并 // eslint-disable-next-line no-var var resizeProps: Map<number, any> | undefined; } export {};或修改为:
- global.resizeProps = new Map<number, any>(); + (globalThis as any).resizeProps = new Map<number, any>();
125-127: 更明确地触发首个 ResizeObserver 回调使用可选链和下标访问是安全的,但
[0]语义略隐晦。建议解构以提升可读性并避免魔法数字。- getResizeProps()?.[0]?.onResize?.({}, document.createElement('div')); + const [containerRO] = getResizeProps() || []; + containerRO?.onResize?.({}, document.createElement('div'));docs/examples/items.tsx (1)
31-34: 示例命名易混淆:label 为 “SubMenu” 但该项并非 SubMenu该项没有
children且未声明type: 'group'/'divider',按实现会被当作普通item。为避免读者误解,建议更改为普通项的描述。- label: 'SubMenu', + label: 'Navigation Three',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
docs/examples/items.tsx(1 hunks)src/Menu.tsx(4 hunks)src/utils/commonUtil.ts(2 hunks)src/utils/nodeUtil.tsx(4 hunks)tests/MenuItem.spec.tsx(1 hunks)tests/Responsive.spec.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Menu.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/MenuItem.spec.tsx (1)
docs/examples/items.tsx (1)
Menu(7-88)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/utils/commonUtil.ts (2)
4-8: 统一 itemRender 的类型签名以匹配 parseItems当前签名为
(originNode: React.ReactNode) => React.ReactElement,而parseItems透传的是(originNode, item?: NonNullable<ItemType>) => React.ReactElement。为避免类型不一致导致的困惑或 TS 报错,建议在此处引入ItemType并统一签名(第二参数可选)。在选定行内修改签名:
export function parseChildren( children: React.ReactNode | undefined, keyPath: string[], - itemRender?: (originNode: React.ReactNode) => React.ReactElement, + itemRender?: (originNode: React.ReactNode, item?: NonNullable<ItemType>) => React.ReactElement, ) {并在文件顶部补充类型引入(需在选定行之外修改):
import type { ItemType } from '../interface';
25-27: 避免在 children 路径直接应用 itemRender,防止破坏 ul/li 语义在
parseChildren中直接对克隆后的子节点应用itemRender,会让最终 DOM 结构出现ul > <Wrapper> > li甚至ul > <Wrapper>的情况,破坏列表语义、样式选择器与可访问性。建议不要在此层进行外层包装或替换。短期修复(移除该层的 itemRender 调用,仅返回克隆节点):
- if (typeof itemRender === 'function') { - return itemRender(React.cloneElement(child, cloneProps)); - } return React.cloneElement(child, cloneProps);后续可通过在
MenuItem内部内容层应用itemRender(参考对 nodeUtil.tsx 的建议)。src/utils/nodeUtil.tsx (1)
79-80: 与 parseChildren 的签名保持一致;避免在 children 路径透传 itemRender
parseItems将itemRender继续传入parseChildren,而 children 路径的外层包装存在语义风险(见 commonUtil.ts 建议)。建议:即便保留parseItems的可选itemRender参数,也不要在parseChildren路径透传。在选定行内修改调用:
- itemRender?: (originNode: React.ReactNode, item?: NonNullable<ItemType>) => React.ReactElement, + itemRender?: (originNode: React.ReactNode, item?: NonNullable<ItemType>) => React.ReactElement, @@ - return parseChildren(childNodes, keyPath, itemRender); + return parseChildren(childNodes, keyPath);(配合 commonUtil.ts 的变更,children 路径不再应用
itemRender,结构更安全。)Also applies to: 95-96
src/Menu.tsx (1)
258-262: 修复 useMemo 依赖缺失 + 移除测量树 itemRender,避免双挂载与副作用问题:
- 当前依赖数组未包含
itemRender,当itemRender发生变化时childList不会重算,导致 UI 过期。[高风险]- 测量树(隐藏节点)也应用了
itemRender,会造成用户自定义 wrapper 被挂载两次,带来副作用与性能开销。一并修改为:
] = React.useMemo( () => [ - parseItems(children, items, EMPTY_LIST, _internalComponents, prefixCls, itemRender), - parseItems(children, items, EMPTY_LIST, {}, prefixCls, itemRender), + parseItems(children, items, EMPTY_LIST, _internalComponents, prefixCls, itemRender), + // 测量树不应用 itemRender,避免双渲染与副作用 + parseItems(children, items, EMPTY_LIST, {}, prefixCls), ], - [children, items, _internalComponents, prefixCls], + // 可见列表依赖 itemRender,确保变更时重算 + [children, items, _internalComponents, prefixCls, itemRender], );效果:
- 可见树随
itemRender变更而正确重算;- 隐藏测量树不再执行
itemRender,避免双挂载与副作用。
🧹 Nitpick comments (1)
src/utils/nodeUtil.tsx (1)
55-57: 避免生成undefined-item-extra类名当
prefixCls为空或未传入时,${prefixCls}-item-extra会变成undefined-item-extra。建议加防护。可结合上一个修复一起改为:
- {(!!extra || extra === 0) && ( - <span className={`${prefixCls}-item-extra`}>{extra}</span> - )} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/Menu.tsx(4 hunks)src/utils/commonUtil.ts(2 hunks)src/utils/nodeUtil.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/Menu.tsx (2)
src/interface.ts (1)
ItemType(77-77)src/utils/nodeUtil.tsx (1)
parseItems(73-96)
src/utils/nodeUtil.tsx (2)
src/interface.ts (1)
ItemType(77-77)src/utils/commonUtil.ts (1)
parseChildren(4-33)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/nodeUtil.tsx (1)
24-71: renderNodeWrapper 隐式 any 且闭包捕获 opt 不必要,建议直接下传 itemRender 保持类型与语义一致当前
renderNodeWrapper = node => ...会产生隐式 any(在严格模式会导致构建失败),且通过闭包捕获opt,屏蔽了子组件传入的第二个参数item。直接把itemRender透传给子组件更简单、也更符合类型约定。应用如下精简与类型友好修改:
- const renderNodeWrapper = node => { - return typeof itemRender === 'function' ? itemRender(node, opt as any) : node; - }; + // 直接透传 itemRender,避免隐式 any 与不必要的闭包<MergedMenuItemGroup key={mergedKey} {...restProps} - itemRender={renderNodeWrapper} + itemRender={itemRender} title={label} >return ( <MergedSubMenu key={mergedKey} {...restProps} - itemRender={renderNodeWrapper} + itemRender={itemRender} title={label} >- return <MergedDivider key={mergedKey} {...restProps} itemRender={renderNodeWrapper} />; + return <MergedDivider key={mergedKey} {...restProps} itemRender={itemRender} />;<MergedMenuItem key={mergedKey} {...restProps} extra={extra} - itemRender={renderNodeWrapper} + itemRender={itemRender} >备注:若希望进一步收紧为“仅内容层渲染”,可参考另一个可选建议。
♻️ Duplicate comments (2)
src/interface.ts (1)
9-9: API 契约不一致:itemRender缺少item参数,且与 PR 其他文件/文档不一致当前仅定义了单参数
(originNode) => ReactNode,而 PR 目标与文档示例均为(originNode, item) => ReactNode。这会导致类型与实现、用法和文档三者不一致,增加使用方困惑,也无法把条目元数据传递给自定义渲染逻辑。此外,建议补充 JSDoc 约束“必须保留可访问性与菜单交互语义(focus、键盘、ARIA 等)”,并明确应包含
originNode以避免破坏导航与选择逻辑。建议修改如下(并收窄第二参数类型,见下 diff):
interface ItemSharedProps { ref?: React.Ref<HTMLLIElement | null>; style?: React.CSSProperties; className?: string; - itemRender?: (originNode: React.ReactNode) => React.ReactNode; + /** + * 自定义渲染菜单条目的原始节点。 + * 必须保留可访问性与交互语义:不要移除可聚焦元素、键盘交互与 ARIA role; + * 返回值应包含传入的 originNode,否则可能破坏键盘导航与选择逻辑。 + */ + itemRender?: ( + originNode: React.ReactNode, + item: NonNullable<ItemType> + ) => React.ReactNode; }为确保全局一致性,请同步更新所有调用点(如 MenuItem、Divider、SubMenu、MenuItemGroup 等)传递第二参数。
请运行以下脚本检查仓库内调用签名是否一致(是否存在两参/单参混用):
#!/bin/bash set -euo pipefail echo "== 定位 itemRender 定义与调用 ==" rg -n -C2 --type ts --type tsx '\bitemRender\s*\(' || true echo "== 统计两参/一参调用(仅作参考,人工复核) ==" rg -n -P --type ts --type tsx 'itemRender\s*\(\s*[^,()]+\s*,\s*[^)]+\)' || true rg -n -P --type ts --type tsx 'itemRender\s*\(\s*[^,()]+\s*\)' || truesrc/utils/nodeUtil.tsx (1)
60-71: 可选:将 itemRender 限定在 MenuItem 的“内容层”,避免外层 li 被替换/包裹破坏语义与可访问性该建议与既有评论一致:把
render能力限制在内容区,保持最外层仍为<li>,避免破坏ul > li语义、ARIA、键盘导航与样式选择器。参考最小改动示例(只针对 MenuItem 分支,Group/SubMenu/Divider 不透传 itemRender):
- return ( - <MergedMenuItem - key={mergedKey} - {...restProps} - extra={extra} - itemRender={itemRender} - > - {label} - {(!!extra || extra === 0) && prefixCls && ( - <span className={`${prefixCls}-item-extra`}>{extra}</span> - )} - </MergedMenuItem> - ); + // 构造内容节点,供 itemRender 定制 + const content = ( + <> + {label} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )} + </> + ); + + return ( + <MergedMenuItem key={mergedKey} {...restProps} extra={extra}> + {typeof itemRender === 'function' ? itemRender(content, opt as any) : content} + </MergedMenuItem> + );若团队最终决定允许外层包裹/替换,建议至少在开发环境对非
<li>顶层返回进行 warning,并在文档中明确风险与规避手段。
🧹 Nitpick comments (7)
src/SubMenu/index.tsx (2)
410-411: 在 SubMenu 中对 “子列表” 进行变换可能遗漏条目元信息与可访问性约束若继续保留该能力,建议也传入第二参数(当前 SubMenu 的条目元数据
props或其子集),并在文档中明确“不得移除/破坏子项的可聚焦元素与 ARIA 语义”。- const childListNode = typeof itemRender === 'function' ? itemRender(childList) : childList; + const childListNode = + typeof itemRender === 'function' ? itemRender(childList, props as any) : childList;同时,建议在 InternalSubMenu 构造
listNode后增加条目级itemRender钩子(见下条评审),统一行为面。
334-369: (设计建议)在listNode级别支持与 MenuItem 一致的itemRender注入为达成“包裹整个 SubMenu 条目”的一致体验,建议在生成
listNode后、_internalRenderSubMenuItem之前插入与 MenuItem 同构的注入逻辑,并采用与 MenuItem 相同的“外层替换/内部注入”安全策略。示例(非精确 diff,仅思路):if (typeof props.itemRender === 'function') { const custom = props.itemRender(listNode, props as any); if (React.isValidElement(custom) && (custom.type === Overflow.Item || custom.type === 'li')) { listNode = custom; } else { listNode = React.cloneElement(listNode as React.ReactElement, undefined, custom); } }请同步在类型与文档中明确 SubMenu 的
itemRender语义与 MenuItem/Divider 对齐。src/MenuItemGroup.tsx (1)
66-67: 为回调补齐第二参数,并在文档中强调不可破坏子项可访问性建议最小改动如下:
- {typeof itemRender === 'function' ? itemRender(childList) : childList} + {typeof itemRender === 'function' ? itemRender(childList, props as any) : childList}并在 README/TS 类型注释中补充“不得移除/破坏子项的 focus、键盘与 ARIA 语义”的契约说明。
src/utils/nodeUtil.tsx (4)
73-73: 为 extra 的类名添加 prefixCls 保护,避免渲染出undefined-item-extra当未传递
prefixCls时会得到不期望的类名前缀。建议:
- {(!!extra || extra === 0) && <span className={`${prefixCls}-item-extra`}>{extra}</span>} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )}
89-90: parseItems 与 convertItemsToNodes 的 itemRender 签名不一致这里把第二个参数标为可选,而上游
convertItemsToNodes使用的是必填NonNullable<ItemType>。为保持一致性与类型安全,建议统一为非可选。建议:
-export function parseItems( +export function parseItems( children: React.ReactNode | undefined, items: ItemType[] | undefined, keyPath: string[], components: Components, prefixCls?: string, - itemRender?: (originNode: React.ReactNode, item?: NonNullable<ItemType>) => React.ReactElement, + itemRender?: (originNode: React.ReactNode, item: NonNullable<ItemType>) => React.ReactElement, ) {
80-80: 使用 .filter(Boolean) 简化可读性当前
.filter(opt => opt)可用更语义化写法替代,行为等价。- .filter(opt => opt); + .filter(Boolean);
13-14: 请统一itemRender签名,保持接口与实现一致
- tsconfig.json 中未启用
strict、noImplicitAny或strictFunctionTypes,当前默认配置下不存在隐式 any 或函数兼容性错误- 在
src/interface.ts(9 行)中,itemRender?: (originNode: React.ReactNode) => React.ReactNode- 在
src/Menu.tsx(162 行)及src/utils/nodeUtil.tsx(13-14、89-90 行)中,均定义为
(originNode: React.ReactNode, item: NonNullable<ItemType>) => React.ReactElementparseChildren定义在src/utils/commonUtil.ts,无需在此额外透传itemRender建议将接口声明改为双参数回调,或将实现调整为单参数,以消除潜在类型不一致风险。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/Divider.tsx(1 hunks)src/Menu.tsx(4 hunks)src/MenuItem.tsx(2 hunks)src/MenuItemGroup.tsx(2 hunks)src/SubMenu/index.tsx(2 hunks)src/interface.ts(1 hunks)src/utils/commonUtil.ts(1 hunks)src/utils/nodeUtil.tsx(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/utils/commonUtil.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Menu.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/Divider.tsx (2)
src/context/MenuContext.tsx (1)
MenuContext(66-66)src/context/PathContext.tsx (1)
useMeasure(15-17)
src/utils/nodeUtil.tsx (1)
src/interface.ts (1)
ItemType(78-78)
🔇 Additional comments (1)
src/utils/nodeUtil.tsx (1)
61-63: 确认 Divider 是否需要支持 itemRender对 Divider 开放外层包装的意义有限,还可能引入无效交互与语义问题。建议评估是否移除,或仅支持内容层渲染。
请确认产品设计是否要求 Divider 也支持自定义渲染;如不要求,建议去除以降低复杂度与风险。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/utils/nodeUtil.tsx (3)
36-41: 谨慎:对 Group 最外层应用 itemRender 可能破坏 DOM 语义/可访问性(重复提醒)在 Group 最外层传递 itemRender,若内部实现允许替换/包裹外层节点,可能导致
<ul>的直接子节点不再是<li>,从而破坏语义、样式选择器与键盘导航等。这与以<li>为交互根节点的约束冲突。建议在 Group 层不暴露外层 render,保持结构稳定,仅对内容层或标题提供更细粒度的 render(例如 titleRender)。最小化修改如下:
<MergedMenuItemGroup key={mergedKey} {...restProps} - itemRender={renderNodeWrapper} title={label} >如果团队坚持允许外包裹/替换,建议在开发环境对顶层非
<li>或未包含原节点时发出 warning,并在文档中明确风险与规避手段。
49-54: 谨慎:对 SubMenu 最外层应用 itemRender 存在同样语义风险(重复提醒)与 Group 相同,若 SubMenu 外层支持被替换/包裹,可能破坏
<li>作为直接子节点的语义与交互。建议移除最外层的 itemRender 传递,或改为仅对标题/内容层提供专用 render:
<MergedSubMenu key={mergedKey} {...restProps} - itemRender={renderNodeWrapper} title={label} >
66-75: 将 itemRender 限定在 MenuItem“内容层”,避免替换/包裹最外层(重复提醒) 当前将 itemRender 作为 prop 传入 MergedMenuItem,若其实现允许替换外层,会破坏
<ul>/<li>语义与交互。建议仅对内容进行定制,外层<li>固定由 MergedMenuItem 承载。最小改法(仅在内容层应用 render,且为 extra 的 className 加保护):
- <MergedMenuItem - key={mergedKey} - {...restProps} - extra={extra} - itemRender={renderNodeWrapper} - > - {label} - {(!!extra || extra === 0) && <span className={`${prefixCls}-item-extra`}>{extra}</span>} - </MergedMenuItem> + // 构造内容节点并仅在内容层应用 render,保持 <li> 作为最外层 + <MergedMenuItem key={mergedKey} {...restProps} extra={extra}> + {renderNodeWrapper( + <> + {label} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )} + </>, + )} + </MergedMenuItem>
🧹 Nitpick comments (3)
src/utils/nodeUtil.tsx (3)
24-26: 为 renderNodeWrapper 增加显式类型,避免隐式 any 并提高可读性目前参数与返回值均为隐式 any/unknown。建议标注为 ReactNode -> ReactNode。
- const renderNodeWrapper = node => { - return typeof itemRender === 'function' ? itemRender(node, opt as any) : node; - }; + const renderNodeWrapper = (node: React.ReactNode): React.ReactNode => { + return typeof itemRender === 'function' ? itemRender(node, opt as any) : node; + };
62-62: Divider 暴露 itemRender 的价值存疑分割线通常不承载交互与内容,开放 itemRender 可能引入非预期的结构替换与样式问题。建议先不暴露,保持最简化。
- return <MergedDivider key={mergedKey} {...restProps} itemRender={renderNodeWrapper} />; + return <MergedDivider key={mergedKey} {...restProps} />;
73-73: prefixCls 为空时会产生undefined-item-extra类名当未传入 prefixCls 时,
${prefixCls}-item-extra会变为undefined-item-extra。已在上条建议的重构中加了保护;如暂不调整结构,至少先加保护。- {(!!extra || extra === 0) && <span className={`${prefixCls}-item-extra`}>{extra}</span>} + {(!!extra || extra === 0) && prefixCls && ( + <span className={`${prefixCls}-item-extra`}>{extra}</span> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/utils/nodeUtil.tsx(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/utils/nodeUtil.tsx (1)
src/interface.ts (1)
ItemType(78-78)
🔇 Additional comments (2)
src/utils/nodeUtil.tsx (2)
42-43: 递归向子项传递 itemRender 👍对子树递归传递 itemRender 的修正到位,保证自定义渲染在子节点生效。
55-55: 递归传递 itemRender 到 SubMenu 子项 👍对子树的继续传递是正确的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/interface.ts (1)
148-151: 为info.item收窄为非空并收敛返回类型到 ReactNode与调用方约定保持一致:
info.item实际不会为null,返回类型使用React.ReactNode即可覆盖ReactElement。这有助于提示正确用法,减少不必要的联合类型。export type ItemRenderType = ( node: React.ReactElement | React.ReactElement<any, string | React.JSXElementConstructor<any>>[], - info: { item: ItemType; keys: string[] }, -) => React.ReactNode | React.ReactElement; + info: { item: NonNullable<ItemType>; keys: string[] }, +) => React.ReactNode;src/Menu.tsx (1)
254-263: 不在 parseItems 中传入 itemRender,避免测量树双渲染与副作用,赞测量树和可见树均不传
itemRender,把自定义渲染下沉到具体条目组件(Item/Group/Divider/SubMenu)层面,既避免了重复挂载与副作用问题,也保持了渲染路径的一致性。依赖数组移除itemRender也合理。src/MenuItem.tsx (1)
207-256: 严重:直接用itemRender替换根节点会破坏 ul>li 语义、rc-overflow 测量与可访问性;且meta.item构造不准确问题简述:
- DOM 语义与布局:当前将
renderNode = mergedItemRender(renderNode, ...),若用户返回<Link>{originNode}</Link>等外层包裹,会产生<ul><Link><li/></Link></ul>的非法结构,并破坏 rc-overflow 对直接子项的测量/收拢。- 可访问性/交互流失:根
<li>(由LegacyMenuItem/Overflow.Item提供的 role、tabIndex、onKeyDown/onClick 等)可能被替换导致键盘与选择逻辑失效。- 类型不符:
meta.item通过{ type: 'item', ...props } as ItemType伪造了type字段;按常规类型定义,MenuItem 并不携带type: 'item'。建议:
- 将自定义渲染“下沉”为 LI 内部内容:把
itemRender的输入改为“内部内容节点”(children + Icon),输出作为<LegacyMenuItem>的 children 注入,保留 LI/Overflow.Item 作为根,确保语义与测量不变。meta.item直接传递props as MenuItemType,不再构造type: 'item'。如需区分类型,可在 meta 增加自定义的kind: 'item'字段(不与类型系统的 discriminant 冲突)。按此思路可在本段直接改造如下:
- let renderNode: React.ReactElement = ( - <LegacyMenuItem - ref={legacyMenuItemRef} - elementRef={mergedEleRef} - role={role === null ? 'none' : role || 'menuitem'} - tabIndex={disabled ? null : -1} - data-menu-id={overflowDisabled && domDataId ? null : domDataId} - {...omit(restProps, ['extra'])} - {...activeProps} - {...optionRoleProps} - component="li" - aria-disabled={disabled} - style={{ - ...directionStyle, - ...style, - }} - className={classNames( - itemCls, - { - [`${itemCls}-active`]: active, - [`${itemCls}-selected`]: selected, - [`${itemCls}-disabled`]: mergedDisabled, - }, - className, - )} - onClick={onInternalClick} - onKeyDown={onInternalKeyDown} - onFocus={onInternalFocus} - > - {children} - <Icon - props={{ - ...props, - isSelected: selected, - }} - icon={mergedItemIcon} - /> - </LegacyMenuItem> - ); - - if (typeof mergedItemRender === 'function') { - renderNode = mergedItemRender(renderNode, { - item: { - type: 'item', - ...props, - } as ItemType, - keys: connectedKeys, - }) as React.ReactElement; - } + // 1) 先构造“内部内容”的原始节点 + const originInnerNode: React.ReactNode = ( + <> + {children} + <Icon + props={{ + ...props, + isSelected: selected, + }} + icon={mergedItemIcon} + /> + </> + ); + + // 2) 允许 itemRender 定制“内部内容”,而非替换根节点 + const contentNode: React.ReactNode = + typeof mergedItemRender === 'function' + ? mergedItemRender(originInnerNode, { + item: props as MenuItemType, + keys: connectedKeys, + }) + : originInnerNode; + + // 3) 保持 <li>/Overflow.Item 为根,确保 ul>li 语义、测量与可达性不变 + let renderNode: React.ReactElement = ( + <LegacyMenuItem + ref={legacyMenuItemRef} + elementRef={mergedEleRef} + role={role === null ? 'none' : role || 'menuitem'} + tabIndex={disabled ? null : -1} + data-menu-id={overflowDisabled && domDataId ? null : domDataId} + {...omit(restProps, ['extra'])} + {...activeProps} + {...optionRoleProps} + component="li" + aria-disabled={disabled} + style={{ + ...directionStyle, + ...style, + }} + className={classNames( + itemCls, + { + [`${itemCls}-active`]: active, + [`${itemCls}-selected`]: selected, + [`${itemCls}-disabled`]: mergedDisabled, + }, + className, + )} + onClick={onInternalClick} + onKeyDown={onInternalKeyDown} + onFocus={onInternalFocus} + > + {contentNode} + </LegacyMenuItem> + );补充验证建议:
- 用例覆盖:
- 断言 DOM 结构满足
ul > li,无ul > *:not(li)。- 包裹
<Link>/<Upload>后,回车键仍触发onItemClick,鼠标点击整行(将 Link 样式设为display:block)仍工作。- 水平/垂直溢出场景中 rc-overflow 的测量、折叠行为不受影响。
🧹 Nitpick comments (7)
src/context/MenuContext.tsx (1)
57-58: 为 itemRender 增补契约说明,防止误用破坏可访问性与结构建议在上下文属性处补充 JSDoc,明确约束与语义,降低误用风险(如返回非
<li>根导致<ul>/<li>结构失效,或移除可聚焦元素影响键盘导航)。可在当前位置追加注释:
- itemRender?: ItemRenderType; + /** + * 自定义渲染单个条目或其子列表。 + * 注意: + * - 请保留 <li> 的语义结构与可访问性(role、tabIndex、键盘交互等)。 + * - Divider / Item / SubMenu 等应返回以 <li> 为根的节点;Group 的入参为“子列表数组”。 + * - 尽量避免带副作用的渲染(测量树不会调用 itemRender)。 + */ + itemRender?: ItemRenderType;src/interface.ts (1)
12-13: 在公共类型处补充 JSDoc,统一语义且提示使用约束作为公开 API 的一部分,建议在
ItemSharedProps.itemRender上补充契约说明,和上下文中的注释保持一致,提醒用户保留<li>语义,避免破坏可访问性。interface ItemSharedProps { ref?: React.Ref<HTMLLIElement | null>; style?: React.CSSProperties; className?: string; - itemRender?: ItemRenderType; + /** + * 自定义渲染单个条目或其子列表。 + * - Item/Divider/SubMenu:应返回以 <li> 为根的节点; + * - Group:第一个参数为组内子项的数组; + * - 请保留可访问性与交互语义(role、键盘操作等)。 + */ + itemRender?: ItemRenderType; }src/Menu.tsx (1)
161-164: 公开itemRenderAPI 与 Context 透传对齐,但建议补充使用约束注释Props 与 Context 一致传递
itemRender,设计清晰。建议在 Props 处补充与 interface/Context 同步的 JSDoc,强调保持<li>语义,避免用户返回非<li>根节点:popupRender?: PopupRender; - itemRender?: ItemRenderType; + /** + * 自定义渲染 originNode 与条目信息。 + * - Divider / Item / SubMenu:请返回以 <li> 为根的节点; + * - Group:第一个参数为子项数组; + * - 建议避免副作用渲染(测量树不会调用该函数)。 + */ + itemRender?: ItemRenderType;Also applies to: 664-665
src/MenuItemGroup.tsx (2)
55-59: Group 级别的 itemRender 用法与其他项存在语义差异,建议在文档与类型注释中明确这里将第一个参数作为“子项列表(ReactElement[])”传入,而 Item/Divider 等传入的是整个
<li>节点。虽然ItemRenderType通过联合类型技术上兼容,但对用户心智不统一。建议:
- 在接口与文档明确:Group 的
itemRender第一个参数是“子项数组”,用于对子项进行变换;- 或引入更明确的命名(如
childrenRender/itemsRender)以区分用途(如不考虑短期 API 变更,可先通过 JSDoc 强化说明)。
65-76: 为 info.item 透出完整分组元数据的做法合理,但可避免不必要的类型断言
as ItemType的断言可通过将字面量完善为MenuItemGroupType来规避,从而减少断言:示例(仅表达意图):
- 使用明确的局部对象
const meta: MenuItemGroupType = { type: 'group', ...omit(props, ['itemRender']) }- 再传入
meta而非断言为ItemType非阻断,按需优化。
src/MenuItem.tsx (2)
15-15: 类型导入 OK,但后续不建议强行构造ItemType后文你通过人为添加
type: 'item'来断言为ItemType(见本评审中对 247–256 行的评论)。按 rc-menu/antd 既有类型约定,MenuItem 项通常没有type: 'item'的区分字段。建议将 meta.item 明确为MenuItemType,不再伪造type字段。若采纳该调整,本行可去掉对ItemType的导入。
118-119: 建议合并策略支持“先 context 后 props”的函数组合,而不是二选一当前
propItemRender || contextItemRender会忽略其一。更通用的做法是将二者串联:context 先包一层,props 再二次包裹,便于应用级别与组件级别同时生效。可在原地替换为下面的实现:
- const mergedItemRender = propItemRender || contextItemRender; + const mergedItemRender = React.useMemo(() => { + if (propItemRender && contextItemRender) { + return (node: React.ReactNode, meta: { item: MenuItemType | null; keys: React.Key[] }) => + propItemRender(contextItemRender(node, meta), meta); + } + return propItemRender || contextItemRender; + }, [propItemRender, contextItemRender]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
docs/examples/items.tsx(1 hunks)src/Divider.tsx(1 hunks)src/Menu.tsx(5 hunks)src/MenuItem.tsx(5 hunks)src/MenuItemGroup.tsx(2 hunks)src/SubMenu/index.tsx(3 hunks)src/context/MenuContext.tsx(2 hunks)src/interface.ts(2 hunks)src/utils/nodeUtil.tsx(1 hunks)tests/MenuItem.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/utils/nodeUtil.tsx
- docs/examples/items.tsx
- src/SubMenu/index.tsx
- tests/MenuItem.spec.tsx
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/context/MenuContext.tsx (1)
src/interface.ts (1)
ItemRenderType(148-151)
src/MenuItemGroup.tsx (4)
src/context/PathContext.tsx (2)
useFullPath(22-29)useMeasure(15-17)src/utils/commonUtil.ts (1)
parseChildren(4-30)src/context/MenuContext.tsx (1)
MenuContext(69-69)src/interface.ts (1)
ItemType(81-81)
src/Divider.tsx (3)
src/interface.ts (1)
MenuDividerType(77-79)src/context/MenuContext.tsx (1)
MenuContext(69-69)src/context/PathContext.tsx (2)
useMeasure(15-17)useFullPath(22-29)
src/MenuItem.tsx (2)
src/context/MenuContext.tsx (1)
MenuContext(69-69)src/interface.ts (1)
ItemType(81-81)
src/Menu.tsx (1)
src/interface.ts (1)
ItemRenderType(148-151)
🔇 Additional comments (7)
src/context/MenuContext.tsx (1)
12-13: 类型引入合理,和接口对齐将
ItemRenderType作为类型导入,避免运行时依赖,符合最佳实践。src/Menu.tsx (1)
31-32: 类型导入与拆分引入方式合理
- 将
ItemRenderType作为类型导入,避免运行时负担。- 将
SubMenu的类型与默认导入拆分,利于 tree-shaking 与类型检查。Also applies to: 34-35
src/Divider.tsx (1)
6-6: 引入 useFullPath 获取 keyPath 的做法正确
keys能用于自定义渲染的上下文信息(如埋点、定位),与整体方案一致。src/MenuItemGroup.tsx (1)
6-6: 类型导入补充 ItemType,便于为 info.item 正确定型为
info.item提供显式类型有助于避免any逃逸。src/MenuItem.tsx (3)
92-93: props 侧itemRender命名为propItemRender👍从 props 解构并重命名,便于与 context 中的同名项合并使用,方向正确。
115-116: 上下文注入itemRender👍从 MenuContext 读取
itemRender以支持全局/父级定制,设计符合预期。
118-119: 确认 itemRender 二参签名一致已验证:
src/interface.ts中ItemRenderType定义为(node, info)两参签名src/context/MenuContext.tsx中itemRender?: ItemRenderType- 所有调用点均传入两个参数:
src/SubMenu/index.tsx:itemRender(childList, { item, keys })src/MenuItem.tsx:mergedItemRender(renderNode, { item, keys })无隐式 any 或类型断言导致的签名漂移问题。
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
src/Divider.tsx
Outdated
|
|
||
| if (typeof mergedItemRender === 'function') { | ||
| return mergedItemRender(renderNode, { | ||
| item: { type: 'divider', ...props }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不能用 props,这个和 Item 的数据不等价。应该直接拿 item 的数据传入。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
其他几个也看看类似的问题~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好了
6a22b41 to
364df90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/nodeUtil.tsx (2)
9-13: 缺少 itemRender 参数导致嵌套菜单项无法应用自定义渲染当前
convertItemsToNodes函数签名中未包含itemRender参数,导致在递归转换子节点时(第 34、42 行)无法向下传递自定义渲染函数。这会使得嵌套在 Group 或 SubMenu 中的菜单项失去itemRender功能。建议修改函数签名并在递归调用时传递 itemRender:
function convertItemsToNodes( list: ItemType[], components: Required<Components>, prefixCls?: string, + itemRender?: ItemRenderType, ) {然后在递归调用处传递:
<MergedMenuItemGroup key={mergedKey} eventOpt={opt} {...restProps} title={label}> - {convertItemsToNodes(children, components, prefixCls)} + {convertItemsToNodes(children, components, prefixCls, itemRender)} </MergedMenuItemGroup><MergedSubMenu key={mergedKey} eventOpt={opt} {...restProps} title={label}> - {convertItemsToNodes(children, components, prefixCls)} + {convertItemsToNodes(children, components, prefixCls, itemRender)} </MergedSubMenu>
82-84: 确保 itemRender 在 items 数组模式中支持并正确传递验证确认存在实际问题:
itemRender属性存在于 Menu 组件,但未通过parseItems传递到convertItemsToNodes,导致使用items数组时无法应用自定义渲染。需要的修复:
- parseItems 函数签名(src/utils/nodeUtil.tsx:65):添加
itemRender参数- convertItemsToNodes 调用(src/utils/nodeUtil.tsx:83):传递
itemRender参数- convertItemsToNodes 函数签名(src/utils/nodeUtil.tsx:9):添加
itemRender参数并在渲染逻辑中使用- Menu.tsx useMemo 依赖数组(src/Menu.tsx:262):添加
itemRender到依赖列表缺少这些修复会导致使用
items属性的菜单无法支持itemRender自定义。
🧹 Nitpick comments (5)
src/interface.ts (2)
9-9: 建议为 itemRender 补充 JSDoc 说明可访问性要求当前
itemRender缺少文档说明,建议添加 JSDoc 注释,明确以下契约:
- 自定义渲染时必须保留原始节点的可访问性与菜单交互语义(如保留可聚焦元素、键盘交互、ARIA role 等)
- 建议至少包含
originNode,避免破坏键盘导航与选择逻辑示例:
/** * 自定义菜单项渲染函数。 * @param originNode - 原始渲染节点 * @param info - 菜单项元数据,包含 item 和 keys * @returns 自定义渲染结果(建议保留 originNode 以维持可访问性) * * 注意:必须保留菜单项的可聚焦元素、键盘交互和 ARIA 属性,避免破坏导航功能。 */ itemRender?: ItemRenderType;
146-149: 建议将 info.item 类型收窄为 NonNullable当前
info.item的类型是ItemType(可为 null),但在实际调用itemRender时始终传入有效的条目对象。建议收窄类型以提升类型安全性并避免使用者误以为需要处理 null 的情况。应用此修改:
export type ItemRenderType = ( node: React.ReactElement | React.ReactElement<any, string | React.JSXElementConstructor<any>>[], - info: { item: ItemType; keys: string[] }, + info: { item: NonNullable<ItemType>; keys: string[] }, ) => React.ReactNode | React.ReactElement;src/utils/nodeUtil.tsx (1)
55-55: 建议为 className 添加 prefixCls 保护当前在构造
className时未检查prefixCls是否存在,可能导致出现undefined-item-extra的类名。应用此修改避免潜在问题:
-{(!!extra || extra === 0) && <span className={`${prefixCls}-item-extra`}>{extra}</span>} +{(!!extra || extra === 0) && prefixCls && <span className={`${prefixCls}-item-extra`}>{extra}</span>}src/SubMenu/index.tsx (1)
410-419: itemRender 语义不一致:用于列表转换而非单项转换当前
SubMenu将itemRender用于转换整个childList(子项数组),而从ItemRenderType的签名来看,它应该是对单个菜单项节点的转换。这种语义不一致可能导致以下问题:
- 语义混淆:使用者可能期望
itemRender是逐项应用,而非对整个列表应用- 类型安全:第 415 行将
eventOpt展开到 item 对象中,但eventOpt可能为 null,且其属性可能与type: 'submenu'冲突- 结构风险:允许替换整个子列表可能破坏 SubMenu 的 DOM 结构和可访问性
建议考虑以下方案之一:
方案 A(推荐):将此能力重命名为
childrenRender或popupRender,与单项级别的itemRender区分开:- const childListNode = - typeof itemRender === 'function' - ? itemRender(childList, { + const childListNode = + typeof childrenRender === 'function' + ? childrenRender(childList, { item: { type: 'submenu', ...eventOpt, } as ItemType, keys: connectedKeyPath, }) : childList;方案 B:保持
itemRender名称,但在文档中明确说明 SubMenu 的itemRender是对子列表的转换,而非单项转换。同时建议添加
eventOpt的 null 检查:item: { type: 'submenu', - ...eventOpt, + ...(eventOpt || {}), } as ItemType,src/MenuItemGroup.tsx (1)
69-77: itemRender 语义不一致且存在类型安全问题与
SubMenu类似,MenuItemGroup也将itemRender用于转换整个childList而非单个菜单项,这与ItemRenderType的语义定义不一致。同时存在以下问题:
- 类型安全:第 73 行直接展开
eventOpt,但它可能为 null 或 undefined,可能导致运行时错误- 语义混淆:使用者期望的是逐项应用
itemRender,而非对整个组进行转换建议修改:
- 添加 null 检查:
item: { type: 'group', - ...eventOpt, + ...(eventOpt || {}), },
重命名以明确语义(可选):将 Group 级别的
itemRender重命名为groupRender或childrenRender,与单项级别的itemRender区分。统一语义:如果保持
itemRender命名,建议在文档中明确说明 Group/SubMenu 的itemRender是对子列表的转换,避免与 MenuItem 的单项转换混淆。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/examples/items.tsx(2 hunks)src/Divider.tsx(1 hunks)src/MenuItem.tsx(5 hunks)src/MenuItemGroup.tsx(1 hunks)src/SubMenu/index.tsx(3 hunks)src/interface.ts(2 hunks)src/utils/nodeUtil.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Divider.tsx
- docs/examples/items.tsx
- src/MenuItem.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-23T06:01:34.975Z
Learnt from: codewizardTM
Repo: react-component/menu PR: 818
File: src/Menu.tsx:415-431
Timestamp: 2025-10-23T06:01:34.975Z
Learning: In src/Menu.tsx focus logic, defaultFocusKey should only use keys from focusableElements/key2element, not from childList.find(), because childList items might not be rendered in DOM (e.g., in overflow menu) which would cause focus to fail.
Applied to files:
src/utils/nodeUtil.tsxsrc/interface.tssrc/MenuItemGroup.tsxsrc/SubMenu/index.tsx
📚 Learning: 2025-10-23T07:19:07.493Z
Learnt from: codewizardTM
Repo: react-component/menu PR: 818
File: src/Menu.tsx:415-431
Timestamp: 2025-10-23T07:19:07.493Z
Learning: In src/Menu.tsx focus logic, when validating candidate focus keys (selected/active/default), must check against focusableElements (not just key2element) to exclude disabled items. Use focusableKeys = new Set(focusableElements.map(el => element2key.get(el))) and validate with focusableKeys.has(k), otherwise disabled selected items will cause focus to fail without fallback.
Applied to files:
src/utils/nodeUtil.tsx
🧬 Code graph analysis (2)
src/MenuItemGroup.tsx (3)
src/context/PathContext.tsx (2)
useFullPath(22-29)useMeasure(15-17)src/utils/commonUtil.ts (1)
parseChildren(4-30)src/context/MenuContext.tsx (1)
MenuContext(69-69)
src/SubMenu/index.tsx (1)
src/interface.ts (1)
ItemType(79-79)
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log
Summary by CodeRabbit