Skip to content

Commit 4610359

Browse files
fresh3nougheps1lon
andauthored
[DevTools] Fix null ref crash in ContextMenu when items list is empty (facebook#35929)
Co-authored-by: Sebastian Sebbie Silbermann <sebastian.silbermann@vercel.com>
1 parent 93882bd commit 4610359

File tree

2 files changed

+18
-9
lines changed

2 files changed

+18
-9
lines changed

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@
88
*/
99

1010
import * as React from 'react';
11-
import {useLayoutEffect, createRef} from 'react';
11+
import {useLayoutEffect} from 'react';
1212
import {createPortal} from 'react-dom';
1313

1414
import ContextMenuItem from './ContextMenuItem';
1515

1616
import type {
1717
ContextMenuItem as ContextMenuItemType,
1818
ContextMenuPosition,
19-
ContextMenuRef,
2019
} from './types';
2120

2221
import styles from './ContextMenu.css';
@@ -49,15 +48,13 @@ type Props = {
4948
items: ContextMenuItemType[],
5049
position: ContextMenuPosition,
5150
hide: () => void,
52-
ref?: ContextMenuRef,
5351
};
5452

5553
export default function ContextMenu({
5654
anchorElementRef,
5755
position,
5856
items,
5957
hide,
60-
ref = createRef(),
6158
}: Props): React.Node {
6259
// This works on the assumption that ContextMenu component is only rendered when it should be shown
6360
const anchor = anchorElementRef.current;
@@ -73,8 +70,21 @@ export default function ContextMenu({
7370
'[data-react-devtools-portal-root]',
7471
);
7572

73+
const hideMenu = portalContainer == null || items.length === 0;
74+
const menuRef = React.useRef<HTMLDivElement | null>(null);
75+
7676
useLayoutEffect(() => {
77-
const menu = ((ref.current: any): HTMLElement);
77+
// Match the early-return condition below.
78+
if (hideMenu) {
79+
return;
80+
}
81+
const maybeMenu = menuRef.current;
82+
if (maybeMenu === null) {
83+
throw new Error(
84+
"Can't access context menu element. This is a bug in React DevTools.",
85+
);
86+
}
87+
const menu = (maybeMenu: HTMLDivElement);
7888

7989
function hideUnlessContains(event: Event) {
8090
if (!menu.contains(((event.target: any): Node))) {
@@ -98,14 +108,14 @@ export default function ContextMenu({
98108

99109
ownerWindow.removeEventListener('resize', hide);
100110
};
101-
}, []);
111+
}, [hideMenu]);
102112

103-
if (portalContainer == null || items.length === 0) {
113+
if (hideMenu) {
104114
return null;
105115
}
106116

107117
return createPortal(
108-
<div className={styles.ContextMenu} ref={ref}>
118+
<div className={styles.ContextMenu} ref={menuRef}>
109119
{items.map(({onClick, content}, index) => (
110120
<ContextMenuItem key={index} onClick={onClick} hide={hide}>
111121
{content}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ export default function ContextMenuContainer({
5353
position={position}
5454
hide={hide}
5555
items={items}
56-
ref={ref}
5756
/>
5857
);
5958
}

0 commit comments

Comments
 (0)