feat: pass full opt object to itemData in convertItemsToNodes#867
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough为菜单项事件回调数据新增并透传 title 字段：扩展 ItemData 类型、在 convertItemsToNodes 中保留原始 item 数据作为 itemData，并在 MenuItem.getEventInfo 中将 props.title 填入事件信息；测试更新验证回调接收完整 itemData。 改动事件数据 title 字段支持
评审工作量估算🎯 3 (Moderate) | ⏱️ ~20 分钟 可能相关的 PR
建议评审人
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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/utils/nodeUtil.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration 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 adds a title property to MenuItem and updates the ItemData interface accordingly. It also refactors nodeUtil.tsx to pass the opt object directly as itemData. A review comment correctly identifies that passing opt directly could result in an undefined key property, which violates the ItemData type requirements, and suggests merging mergedKey into itemData to ensure type safety.
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/interface.ts (1)
67-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick win类型定义与实际数据契约不一致。
ItemData类型明确列出了允许的字段（label、itemIcon、extra、key、title），但在nodeUtil.tsx中的变更将整个opt对象作为itemData传递，而opt可能包含任意自定义属性。这会导致：
- 类型不安全：
opt被类型标注为any（nodeUtil.tsx:24），可以包含ItemData中未定义的字段（如type、children等内部字段以及用户自定义字段）。- 契约不明确：当前类型定义暗示只有列出的字段可用，但实际运行时会包含更多属性。
如果目标是支持所有用户自定义属性，建议使用索引签名扩展类型定义：
建议的类型修复
export type ItemData = { label?: React.ReactNode; itemIcon?: RenderIconType; extra?: React.ReactNode; key: React.Key; title?: string; + [key: string]: any; };或者使用更精确的类型定义，明确区分已知字段和自定义字段。
🤖 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/interface.ts` around lines 67 - 73, ItemData's current shape is too narrow compared to how nodeUtil.tsx passes the entire opt object as itemData (opt is typed any and may include fields like type/children or custom props); update the ItemData type to allow additional arbitrary properties by adding an index signature (e.g., [key: string]: any) or otherwise extend it to include unknown keys, and then tighten/remove the any on opt in nodeUtil.tsx (referencing ItemData and the opt variable / itemData usage) so the compiler knows extra user/custom fields are allowed while preserving the known fields (label, itemIcon, extra, key, title).src/MenuItem.tsx (1)
138-153: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win两种渲染模式的
itemData构造不一致。当前实现在两种模式下对
itemData的处理方式不同：
- Children 模式（
propsItemData为空时，line 138-144）：手动构造itemData，仅包含key、label、itemIcon、extra、title这些显式字段。- Items 模式（
propsItemData由nodeUtil.tsx传入）：现在传递完整的opt对象，包含所有自定义属性。这导致：
- 使用
<MenuItem>子组件写法时，自定义属性不会出现在回调的itemData中- 使用
itemsprop 写法时，所有自定义属性都会出现在itemData中建议统一两种模式的行为，或在文档中明确说明这种差异。
🤖 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/MenuItem.tsx` around lines 138 - 153, The constructed itemData differs between children mode and items mode; update the children-mode branch that builds itemData (the block creating itemData when propsItemData is falsy using eventKey, children, itemIcon, props.extra, props.title) to merge all custom props into the returned itemData (e.g., spread props or relevant fields) so that the shape matches the full opt passed from nodeUtil.tsx and custom attributes are present in the callback itemData (still return propsItemData || itemData for the itemData field and keep other returned fields like key, keyPath, item: legacyMenuItemRef.current, domEvent: e unchanged).
🤖 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.
Inline comments:
In `@src/utils/nodeUtil.tsx`:
- Line 54: convertItemsToNodes currently passes the entire opt into
MergedMenuItem as itemData (itemData={opt}), which leaks internal/custom props
(e.g., foo, type, children) and leaves itemData.key undefined when key was
missing; change it to build a sanitized itemData object instead of passing opt:
include key: mergedKey, label, and only the expected props (e.g., extra and any
intended itemIcon/item-specific fields), and pass restProps only to the
component props (keep {...restProps} as before) while supplying
itemData={itemData} to MergedMenuItem so callbacks receive the constrained
shape.
---
Outside diff comments:
In `@src/interface.ts`:
- Around line 67-73: ItemData's current shape is too narrow compared to how
nodeUtil.tsx passes the entire opt object as itemData (opt is typed any and may
include fields like type/children or custom props); update the ItemData type to
allow additional arbitrary properties by adding an index signature (e.g., [key:
string]: any) or otherwise extend it to include unknown keys, and then
tighten/remove the any on opt in nodeUtil.tsx (referencing ItemData and the opt
variable / itemData usage) so the compiler knows extra user/custom fields are
allowed while preserving the known fields (label, itemIcon, extra, key, title).
In `@src/MenuItem.tsx`:
- Around line 138-153: The constructed itemData differs between children mode
and items mode; update the children-mode branch that builds itemData (the block
creating itemData when propsItemData is falsy using eventKey, children,
itemIcon, props.extra, props.title) to merge all custom props into the returned
itemData (e.g., spread props or relevant fields) so that the shape matches the
full opt passed from nodeUtil.tsx and custom attributes are present in the
callback itemData (still return propsItemData || itemData for the itemData field
and keep other returned fields like key, keyPath, item:
legacyMenuItemRef.current, domEvent: e unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 052cb7ba-95d3-48f7-aa50-16853cd31a79
📒 Files selected for processing (3)
src/MenuItem.tsxsrc/interface.tssrc/utils/nodeUtil.tsx
…lick when using items prop
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #867 +/- ##
=======================================
Coverage 99.72% 99.72%
=======================================
Files 26 26
Lines 732 732
Branches 204 204
=======================================
Hits 730 730
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/MenuItem.spec.tsx (1)
184-203: ⚡ Quick win建议补一条 children 模式下
title透传断言。当前用例只覆盖了
items模式；本次改动还涉及MenuItem在 children 模式下把props.title注入itemData。建议在同组里补一个<MenuItem title="...">点击后info.itemData.title的断言，避免该路径后续回归。可参考补充用例
+ it('should pass title in itemData when using children mode', () => { + const onClick = jest.fn(); + const { container } = render( + <Menu onClick={onClick}> + <MenuItem key="1" title="child-title"> + Menu Item + </MenuItem> + </Menu>, + ); + + fireEvent.click(container.querySelector('.rc-menu-item')!); + expect(onClick).toHaveBeenCalledWith( + expect.objectContaining({ + key: '1', + itemData: expect.objectContaining({ + key: '1', + title: 'child-title', + }), + }), + ); + });🤖 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/MenuItem.spec.tsx` around lines 184 - 203, Add a sibling test in MenuItem.spec.tsx that exercises the children mode: render <Menu selectable><MenuItem title="test title">Menu Item</MenuItem></Menu> with jest mocks for onSelect and onClick (same as the existing items-mode test), fireEvent.click the rendered .rc-menu-item, and assert that both onSelect and onClick were called with an info object whose itemData includes the title property (expect.objectContaining({ title: 'test title' })). This ensures MenuItem in children mode correctly injects props.title into itemData.
🤖 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 `@tests/MenuItem.spec.tsx`:
- Around line 184-203: Add a sibling test in MenuItem.spec.tsx that exercises
the children mode: render <Menu selectable><MenuItem title="test title">Menu
Item</MenuItem></Menu> with jest mocks for onSelect and onClick (same as the
existing items-mode test), fireEvent.click the rendered .rc-menu-item, and
assert that both onSelect and onClick were called with an info object whose
itemData includes the title property (expect.objectContaining({ title: 'test
title' })). This ensures MenuItem in children mode correctly injects props.title
into itemData.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15de9fe6-ab71-4c5c-be3b-2bbbfa882c38
📒 Files selected for processing (1)
tests/MenuItem.spec.tsx
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
2 participants
描述：
改动内容
在
src/utils/nodeUtil.tsx中，将itemData从手动构造的部分对象改为直接使用完整的opt对象。这样用户传入的所有属性（如
title、fildes、test等）都能在回调的itemData中获取到。具体修改
src/utils/nodeUtil.tsx: 将itemData={{ label, key: mergedKey, itemIcon: restProps?.itemIcon, extra }}改为itemData={opt}src/interface.ts: 在ItemData接口中添加title?: string（TypeScript 支持）src/MenuItem.tsx: 将titleprop 传递给itemData动机
当使用
items属性生成菜单项时，用户希望在onClick或onSelect回调中通过info.itemData获取自定义属性。之前itemData只包含label、key、itemIcon和extra，现在所有传入的属性都会被透传。Summary by CodeRabbit
新特性
行为调整
测试