fix: ensure distinct popup ids across menu instances - #888
Conversation
|
@atharv-sys32 is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
Walkthrough菜单在未提供 Changes菜单弹层标识
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
![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
🤖 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/Menu.tsx`:
- Around line 262-263: Update the UUID generation in Menu so id-less instances
receive distinct IDs that remain identical between server rendering and initial
client hydration, including on React versions below 18 where the compatibility
useId returns ssr-id. Preserve the caller-provided id behavior, or alternatively
raise the React peer requirement to 18+ and add regression coverage for legacy
React SSR hydration.
🪄 Autofix
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 Plus
Run ID: 54307688-14e2-4038-b984-4e762b83b208
⛔ Files ignored due to path filters (6)
tests/__snapshots__/Keyboard.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Menu.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/MenuItem.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Options.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Responsive.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/SubMenu.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/Menu.tsxtests/Menu.spec.tsx
| const mergedUuid = useId(); | ||
| const uuid = useId(id ? `rc-menu-uuid-${id}` : `rc-menu-uuid-${mergedUuid}`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 检查项目声明的 React peer range。
fd -a -t f -E node_modules -E dist -E build \
'package.json|package-lock.json|yarn.lock|pnpm-lock.yaml' . \
| xargs -r rg -n -C 5 '"peerDependencies"|"react"\s*:'
# 检查仓库中是否包含 useId 兼容实现。
fd -a -t f -E node_modules \
'useId\.(js|ts|tsx)$' . \
| xargs -r rg -n -C 8 'ssr-id|useCompatId|useOriginId|NODE_ENV.*test'
# 检查现有 SSR 或 hydration 测试。
rg -n -C 3 'renderToString|renderToPipeableStream|hydrateRoot' \
--glob '*.{js,jsx,ts,tsx}' . || trueRepository: react-component/menu
Length of output: 606
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- package.json peer deps ---\n'
sed -n '80,95p' package.json || true
printf '\n--- locate Menu.tsx and relevant rc util/useId references ---\n'
fd -a 'Menu\.tsx$' .
printf '\n--- inspect Menu.tsx around lines 240-275 ---\n'
sed -n '240,275p' src/Menu.tsx || true
printf '\n--- search useId/useCompatId/ssr-id in tracked files ---\n'
git ls-files \
| grep -E '(^|/)(package.json|node_modules/.*useId\.(js|ts|tsx)|node_modules/@rc-component/util/.*)$' \
|| true
rg -n -C 5 'useId|ssr-id|useCompatId|useOriginId' package.json src || trueRepository: react-component/menu
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- package.json peer deps ---\n'
sed -n '80,95p' package.json || true
printf '\n--- locate Menu.tsx and relevant rc util/useId references ---\n'
fd -a 'Menu\.tsx$' .
printf '\n--- inspect Menu.tsx around lines 240-275 ---\n'
sed -n '240,275p' src/Menu.tsx || true
printf '\n--- search useId/useCompatId/ssr-id in tracked files ---\n'
git ls-files \
| grep -E 'useId\.(js|ts|tsx)$|`@rc-component/util`' \
|| true
rg -n -C 5 'useId|ssr-id|useCompatId|useOriginId' package.json src || trueRepository: react-component/menu
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package.json peer deps ---'
sed -n '80,95p' package.json || true
printf '%s\n' ''
printf '%s\n' '--- locate Menu.tsx and relevant rc util/useId references ---'
fd -a 'Menu\.tsx$' .
printf '%s\n' ''
printf '%s\n' '--- inspect Menu.tsx around lines 240-275 ---'
sed -n '240,275p' src/Menu.tsx || true
printf '%s\n' ''
printf '%s\n' '--- search useId/useCompatId/ssr-id in tracked files ---'
git ls-files \
| grep -E 'useId\.(js|ts|tsx)$|`@rc-component/util`' \
|| true
rg -n -C 5 'useId|ssr-id|useCompatId|useOriginId' package.json src || trueRepository: react-component/menu
Length of output: 2993
🌐 Web query:
@rc-component/util useId ssr-id useCompatId useOriginId
💡 Result:
In the @rc-component/util library, the useId hook is a compatibility wrapper designed to manage ID generation consistently across environments, especially when React's native useId hook might not be available (e.g., in older React versions) or to handle specific synchronization requirements [1]. The implementation logic behind these functions is as follows: 1. useId (The Primary Hook): This is the main exported hook [1]. It acts as a gateway that checks if a native useOriginId is available [1]. If it is, it uses React's native useId [1]. If not, it falls back to useCompatId to provide a compatible ID generation mechanism [1]. It also prioritizes an externally provided id if passed by the developer, treating it as the "single source of truth," and returns a mock ID ('test-id') when running in test environments [1]. 2. useOriginId: This is an internal helper that resolves to React's native useId hook if it exists in the current environment [1]. It is used by useId to detect if the native capability is available [1]. 3. useCompatId: This is the fallback mechanism used when native useId is unavailable [1]. It generates IDs using a simple internal counter (starting with 'ssr-id' and incrementing via rc_unique_{number}) [1]. Because it relies on React.useEffect to initialize, this ID is only generated on the client side, meaning it may not match across server-side and client-side rendering [1]. 4. ssr-id: This is not a function, but a default value ('ssr-id') used by useCompatId during the initial render phase before the effect runs [1]. In practice, when using components from the Ant Design ecosystem (such as Tooltip, Select, or Menu), you import useId directly from @rc-component/util [2][3][4]. The hook handles the underlying logic of whether to use native features or the compatibility fallback automatically [1].
Citations:
- 1: https://github.com/react-component/util/blob/master/src/hooks/useId.ts
- 2: https://github.com/react-component/tooltip/blob/master/src/Tooltip.tsx
- 3: https://github.com/react-component/select/blob/master/src/Select.tsx
- 4: https://github.com/react-component/menu/blob/master/src/Menu.tsx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- package.json full ---'
sed -n '1,140p' package.json || trueRepository: react-component/menu
Length of output: 2872
补齐旧版 React 的 SSR 唯一性。
当前 package.json 仍声明 react/react-dom >=16.9.0，但新增的 mergedUuid 在旧版 React 中会落入 @rc-component/util 的兼容 useId。该兼容路径 SSR 首屏返回 ssr-id，到客户端 useEffect 后才生成 rc_unique_*。两个无 id 的 Menu 因此会在 SSR 和 hydration 初始输出中得到相同的 rc-menu-uuid-ssr-id，重复的弹层 ID 会影响 data-menu-id 和 aria-controls 关系。如果这个 peer range 仍有效，请使用 SSR 可水合的实例唯一 ID；否则提升到 React 18+，并增加旧版 React/SSR 回归测试。
🤖 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/Menu.tsx` around lines 262 - 263, Update the UUID generation in Menu so
id-less instances receive distinct IDs that remain identical between server
rendering and initial client hydration, including on React versions below 18
where the compatibility useId returns ssr-id. Preserve the caller-provided id
behavior, or alternatively raise the React peer requirement to 18+ and add
regression coverage for legacy React SSR hydration.
Source: MCP tools
1 participant
Problem
When two
Menuinstances render submenus, both popups get the same ARIAid. This breaks accessibility (duplicate IDs violate WCAG; screen readers andaria-controls/aria-labelledbyreferences become ambiguous).This is a follow-up on ant-design/ant-design#53707.
Root cause
In
src/Menu.tsx, the menu uuid is:rc-util'suseIdreturns the passedidunchanged when it is a stable string (itsuseSSRContextlogic doesif (id) return id). So for every menu without an explicitid,uuidcollapses to the constant stringrc-menu-uuidand the generated popup ids collide:Solution
Derive the fallback uuid from a fresh
useId()so every menu instance is unique:After the fix the popup ids are distinct per instance (
rc-menu-uuid-:r0:-admin-popupvsrc-menu-uuid-:r3:-admin-popup).Test
Added
gives distinct popup ids to separate menu instancesintests/Menu.spec.tsx: renders two vertical menus with the same submenu keys, then asserts the two popupids differ. It fails on the previous code and passes with the fix.Snapshots were regenerated (
-u) becausedata-menu-id/aria-controlsembed the uuid, which now carries thetest-idsuffix underNODE_ENV=test.Summary by CodeRabbit
Bug 修复
id可能重复的问题。测试