From 561941d5e0d8c0bed852156e06820780e65f0656 Mon Sep 17 00:00:00 2001 From: Tomas Valenta Date: Wed, 22 Dec 2021 12:13:36 +0100 Subject: [PATCH] fix: Remove `element.current` from `useEffect` in `BubbleMenu` and `FloatingMenu` (#2297) * Remove `element.current` from `useEffect` dependencies Changes to the `element.current` don't trigger `useEffect` rerender and shouldn't be used in the dependency array. One discussion about is this is for example here: https://stackoverflow.com/questions/60476155/is-it-safe-to-use-ref-current-as-useeffects-dependency-when-ref-points-to-a-dom It's also causing some subtle bugs when mounting and unmounting editors. * Fix `FloatingMenu` and `BubbleMenu` element references * Fix linting errors * Don't register plugin when the editor is already destroyed; Simplify `HTMLElement` reference handling * Fix lint error --- packages/react/src/BubbleMenu.tsx | 27 ++++++++++++++++----------- packages/react/src/FloatingMenu.tsx | 27 ++++++++++++++++----------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/packages/react/src/BubbleMenu.tsx b/packages/react/src/BubbleMenu.tsx index 35631034..2d9481ed 100644 --- a/packages/react/src/BubbleMenu.tsx +++ b/packages/react/src/BubbleMenu.tsx @@ -1,4 +1,6 @@ -import React, { useEffect, useRef } from 'react' +import React, { + useEffect, useState, +} from 'react' import { BubbleMenuPlugin, BubbleMenuPluginProps } from '@tiptap/extension-bubble-menu' type Optional = Pick, K> & Omit @@ -8,10 +10,14 @@ export type BubbleMenuProps = Omit, } export const BubbleMenu: React.FC = props => { - const element = useRef(null) + const [element, setElement] = useState(null) useEffect(() => { - if (!element.current) { + if (!element) { + return + } + + if (props.editor.isDestroyed) { return } @@ -22,24 +28,23 @@ export const BubbleMenu: React.FC = props => { shouldShow = null, } = props - editor.registerPlugin(BubbleMenuPlugin({ + const plugin = BubbleMenuPlugin({ pluginKey, editor, - element: element.current as HTMLElement, + element, tippyOptions, shouldShow, - })) + }) - return () => { - editor.unregisterPlugin(pluginKey) - } + editor.registerPlugin(plugin) + return () => editor.unregisterPlugin(pluginKey) }, [ props.editor, - element.current, + element, ]) return ( -
+
{props.children}
) diff --git a/packages/react/src/FloatingMenu.tsx b/packages/react/src/FloatingMenu.tsx index fb20f851..6420d8dc 100644 --- a/packages/react/src/FloatingMenu.tsx +++ b/packages/react/src/FloatingMenu.tsx @@ -1,4 +1,6 @@ -import React, { useEffect, useRef } from 'react' +import React, { + useEffect, useState, +} from 'react' import { FloatingMenuPlugin, FloatingMenuPluginProps } from '@tiptap/extension-floating-menu' type Optional = Pick, K> & Omit @@ -8,10 +10,14 @@ export type FloatingMenuProps = Omit = props => { - const element = useRef(null) + const [element, setElement] = useState(null) useEffect(() => { - if (!element.current) { + if (!element) { + return + } + + if (props.editor.isDestroyed) { return } @@ -22,24 +28,23 @@ export const FloatingMenu: React.FC = props => { shouldShow = null, } = props - editor.registerPlugin(FloatingMenuPlugin({ + const plugin = FloatingMenuPlugin({ pluginKey, editor, - element: element.current as HTMLElement, + element, tippyOptions, shouldShow, - })) + }) - return () => { - editor.unregisterPlugin(pluginKey) - } + editor.registerPlugin(plugin) + return () => editor.unregisterPlugin(pluginKey) }, [ props.editor, - element.current, + element, ]) return ( -
+
{props.children}
)