Skip to content

fix: ensure distinct popup ids across menu instances #888

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.

Sign up for GitHub

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

Open
atharv-sys32 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: ensure distinct popup ids across menu instances #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ const Menu = React.forwardRef<MenuRef, MenuProps>((props, ref) => {

const containerRef = React.useRef<HTMLUListElement>(null);

const uuid = useId(id ? `rc-menu-uuid-${id}` : 'rc-menu-uuid');
const mergedUuid = useId();
const uuid = useId(id ? `rc-menu-uuid-${id}` : `rc-menu-uuid-${mergedUuid}`);
Comment on lines +262 to +263

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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}' . || true

Repository: 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 || true

Repository: 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 || true

Repository: 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 || true

Repository: 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:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- package.json full ---'
sed -n '1,140p' package.json || true

Repository: 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


const isRtl = direction === 'rtl';

Expand Down
35 changes: 35 additions & 0 deletions tests/Menu.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,41 @@ describe('Menu', () => {
unmount();
});

it('gives distinct popup ids to separate menu instances', () => {
const originEnv = process.env.NODE_ENV;
process.env.NODE_ENV = 'development';

const renderMenu = () => (
<Menu mode="vertical" triggerSubMenuAction="click" openKeys={['admin']}>
<SubMenu key="admin" title="Admin">
<MenuItem key="users">Users</MenuItem>
</SubMenu>
</Menu>
);

try {
const { container } = render(
<div>
{renderMenu()}
{renderMenu()}
</div>,
);

act(() => {
jest.runAllTimers();
});

const ids = Array.from(container.querySelectorAll('.rc-menu-submenu-popup [id]')).map(
el => (el as HTMLElement).id,
);

expect(ids).toHaveLength(2);
expect(new Set(ids).size).toBe(ids.length);
} finally {
process.env.NODE_ENV = originEnv;
}
});

(['vertical', 'horizontal', 'inline'] as MenuMode[]).forEach(mode => {
it(`${mode} menu correctly`, () => {
const { container } = render(createMenu({ mode }));
Expand Down
10 changes: 5 additions & 5 deletions tests/__snapshots__/Keyboard.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Menu.Keyboard no data-menu-id by init 1`] = `
HTMLCollection [
Expand All @@ -13,11 +13,11 @@ HTMLCollection [
role="none"
>
<div
aria-controls="rc-menu-uuid-light-popup"
aria-controls="rc-menu-uuid-test-id-light-popup"
aria-expanded="true"
aria-haspopup="true"
class="rc-menu-submenu-title"
data-menu-id="rc-menu-uuid-light"
data-menu-id="rc-menu-uuid-test-id-light"
role="menuitem"
style="padding-left: 24px;"
tabindex="-1"
Expand All @@ -31,12 +31,12 @@ HTMLCollection [
<ul
class="rc-menu rc-menu-sub rc-menu-inline"
data-menu-list="true"
id="rc-menu-uuid-light-popup"
id="rc-menu-uuid-test-id-light-popup"
role="menu"
>
<li
class="rc-menu-item"
data-menu-id="rc-menu-uuid-bamboo"
data-menu-id="rc-menu-uuid-test-id-bamboo"
role="menuitem"
style="padding-left: 48px;"
tabindex="-1"
Expand Down
Loading