Skip to content

Commit 29f83df

Browse files
committed
[DevTools] Fix null ref crash in ContextMenu when items list is empty
1 parent e49335e commit 29f83df

2 files changed

Lines changed: 128 additions & 1 deletion

File tree

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow
8+
*/
9+
10+
describe('ContextMenu', () => {
11+
let React;
12+
let ReactDOMClient;
13+
let act;
14+
let ContextMenu;
15+
let container;
16+
let portalRoot;
17+
18+
beforeEach(() => {
19+
React = require('react');
20+
ReactDOMClient = require('react-dom/client');
21+
act = React.act;
22+
23+
ContextMenu =
24+
require('react-devtools-shared/src/devtools/ContextMenu/ContextMenu').default;
25+
26+
container = document.createElement('div');
27+
document.body.appendChild(container);
28+
29+
portalRoot = document.createElement('div');
30+
portalRoot.setAttribute('data-react-devtools-portal-root', '');
31+
document.body.appendChild(portalRoot);
32+
});
33+
34+
afterEach(() => {
35+
document.body.removeChild(container);
36+
document.body.removeChild(portalRoot);
37+
});
38+
39+
// @reactVersion >= 18
40+
it('should not crash when items list is empty', () => {
41+
// Simulates the real usage: ContextMenuContainer only renders ContextMenu
42+
// after the anchor is already mounted (shouldShow toggled via right-click).
43+
const anchorRef = React.createRef();
44+
let showMenu;
45+
46+
function Wrapper() {
47+
const [show, setShow] = React.useState(false);
48+
showMenu = setShow;
49+
50+
return (
51+
<div ref={anchorRef}>
52+
{show && (
53+
<ContextMenu
54+
anchorElementRef={anchorRef}
55+
items={[]}
56+
position={{x: 0, y: 0}}
57+
hide={() => setShow(false)}
58+
/>
59+
)}
60+
</div>
61+
);
62+
}
63+
64+
const root = ReactDOMClient.createRoot(container);
65+
act(() => {
66+
root.render(<Wrapper />);
67+
});
68+
69+
// Now show the context menu with empty items.
70+
// Should not throw "Cannot read properties of null (reading 'ownerDocument')".
71+
expect(() => {
72+
act(() => {
73+
showMenu(true);
74+
});
75+
}).not.toThrow();
76+
77+
act(() => {
78+
root.unmount();
79+
});
80+
});
81+
82+
// @reactVersion >= 18
83+
it('should render context menu when items are provided', () => {
84+
const anchorRef = React.createRef();
85+
let showMenu;
86+
87+
function Wrapper() {
88+
const [show, setShow] = React.useState(false);
89+
showMenu = setShow;
90+
91+
return (
92+
<div ref={anchorRef}>
93+
{show && (
94+
<ContextMenu
95+
anchorElementRef={anchorRef}
96+
items={[{onClick: () => {}, content: 'Test Item'}]}
97+
position={{x: 10, y: 10}}
98+
hide={() => setShow(false)}
99+
/>
100+
)}
101+
</div>
102+
);
103+
}
104+
105+
const root = ReactDOMClient.createRoot(container);
106+
act(() => {
107+
root.render(<Wrapper />);
108+
});
109+
110+
act(() => {
111+
showMenu(true);
112+
});
113+
114+
// The context menu should be rendered inside the portal root.
115+
expect(portalRoot.textContent).toContain('Test Item');
116+
117+
act(() => {
118+
root.unmount();
119+
});
120+
});
121+
});

packages/react-devtools-shared/src/devtools/ContextMenu/ContextMenu.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,13 @@ export default function ContextMenu({
7474
);
7575

7676
useLayoutEffect(() => {
77-
const menu = ((ref.current: any): HTMLElement);
77+
const menu = ref.current;
78+
79+
// The portal may not be rendered if items is empty or the portal
80+
// container is missing, in which case the ref will be null.
81+
if (menu == null) {
82+
return;
83+
}
7884

7985
function hideUnlessContains(event: Event) {
8086
if (!menu.contains(((event.target: any): Node))) {

0 commit comments

Comments
 (0)