fix: LinkToolbar Event Listener leak (#2335)

Author: YousefED

packages/core/src/extensions/LinkToolbar/LinkToolbar.ts

@@ -1,6 +1,6 @@
 import { getMarkRange, posToDOMRect } from "@tiptap/core";
-import { createExtension } from "../../editor/BlockNoteExtension.js";
 import { getPmSchema } from "../../api/pmUtil.js";
+import { createExtension } from "../../editor/BlockNoteExtension.js";
 
 export const LinkToolbarExtension = createExtension(({ editor }) => {
   function getLinkElementAtPos(pos: number) {

packages/react/src/components/FormattingToolbar/DefaultButtons/CreateLinkButton.tsx

@@ -94,10 +94,11 @@ export const CreateLinkButton = () => {
       }
     };
 
-    editor.domElement?.addEventListener("keydown", callback);
+    const domElement = editor.domElement;
+    domElement?.addEventListener("keydown", callback);
 
     return () => {
-      editor.domElement?.removeEventListener("keydown", callback);
+      domElement?.removeEventListener("keydown", callback);
     };
   }, [editor.domElement]);
 

packages/react/src/components/LinkToolbar/LinkToolbarController.tsx

@@ -4,14 +4,14 @@ import { Range } from "@tiptap/core";
 import { FC, useEffect, useMemo, useState } from "react";
 
 import { useBlockNoteEditor } from "../../hooks/useBlockNoteEditor.js";
-import { LinkToolbar } from "./LinkToolbar.js";
-import { LinkToolbarProps } from "./LinkToolbarProps.js";
 import { useExtension } from "../../hooks/useExtension.js";
 import { FloatingUIOptions } from "../Popovers/FloatingUIOptions.js";
 import {
   GenericPopover,
   GenericPopoverReference,
 } from "../Popovers/GenericPopover.js";
+import { LinkToolbar } from "./LinkToolbar.js";
+import { LinkToolbarProps } from "./LinkToolbarProps.js";
 
 export const LinkToolbarController = (props: {
   linkToolbar?: FC<LinkToolbarProps>;
@@ -98,15 +98,16 @@ export const LinkToolbarController = (props: {
     const destroyOnSelectionChangeHandler =
       editor.onSelectionChange(textCursorCallback);
 
-    editor.domElement?.addEventListener("mouseover", mouseCursorCallback);
+    const domElement = editor.domElement;
+
+    domElement?.addEventListener("mouseover", mouseCursorCallback);
 
     return () => {
       destroyOnChangeHandler();
       destroyOnSelectionChangeHandler();
-
-      editor.domElement?.removeEventListener("mouseover", mouseCursorCallback);
+      domElement?.removeEventListener("mouseover", mouseCursorCallback);
     };
-  }, [editor, linkToolbar, link, toolbarPositionFrozen]);
+  }, [editor, editor.domElement, linkToolbar, link, toolbarPositionFrozen]);
 
   const floatingUIOptions = useMemo<FloatingUIOptions>(
     () => ({
@@ -161,6 +162,7 @@ export const LinkToolbarController = (props: {
     [link?.element],
   );
 
+  // TODO: this should be a hook to be reactive
   if (!editor.isEditable) {
     return null;
   }

packages/react/src/components/SuggestionMenu/GridSuggestionMenu/hooks/useGridSuggestionMenuKeyboardNavigation.ts

@@ -66,14 +66,11 @@ export function useGridSuggestionMenuKeyboardNavigation<Item>(
       return false;
     };
 
-    editor.domElement?.addEventListener(
-      "keydown",
-      handleMenuNavigationKeys,
-      true,
-    );
+    const domElement = editor.domElement;
+    domElement?.addEventListener("keydown", handleMenuNavigationKeys, true);
 
     return () => {
-      editor.domElement?.removeEventListener(
+      domElement?.removeEventListener(
         "keydown",
         handleMenuNavigationKeys,
         true,

packages/react/src/components/SuggestionMenu/hooks/useSuggestionMenuKeyboardNavigation.ts

@@ -15,14 +15,11 @@ export function useSuggestionMenuKeyboardNavigation<Item>(
     useSuggestionMenuKeyboardHandler(items, onItemClick);
 
   useEffect(() => {
-    (element || editor.domElement)?.addEventListener("keydown", handler, true);
+    const el = element || editor.domElement;
+    el?.addEventListener("keydown", handler, true);
 
     return () => {
-      (element || editor.domElement)?.removeEventListener(
-        "keydown",
-        handler,
-        true,
-      );
+      el?.removeEventListener("keydown", handler, true);
     };
   }, [editor.domElement, items, selectedIndex, onItemClick, element, handler]);
 

tests/src/unit/react/BlockNoteViewRemountHover.test.tsx

@@ -0,0 +1,83 @@
+import { BlockNoteEditor } from "@blocknote/core";
+import { BlockNoteView } from "@blocknote/mantine";
+import "@blocknote/mantine/style.css";
+import React from "react";
+import { flushSync } from "react-dom";
+import { createRoot } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+
+// https://github.com/TypeCellOS/BlockNote/pull/2335
+describe("BlockNoteView new editor + hover", () => {
+  let div: HTMLDivElement;
+
+  beforeEach(() => {
+    div = document.createElement("div");
+    document.body.appendChild(div);
+  });
+
+  afterEach(() => {
+    document.body.removeChild(div);
+  });
+
+  it("should not throw error when replacing editor in same container and mouseovering", async () => {
+    // 1. Setup container
+    const editor1 = BlockNoteEditor.create();
+    const root = createRoot(div);
+
+    // 2. Render first editor twice
+    flushSync(() => {
+      root.render(
+        <React.StrictMode>
+          <BlockNoteView editor={editor1} />
+        </React.StrictMode>,
+      );
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    flushSync(() => {
+      root.render(
+        <React.StrictMode>
+          <BlockNoteView editor={editor1} />
+        </React.StrictMode>,
+      );
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    const editor1DomElement = editor1.domElement;
+    expect(editor1DomElement).toBeDefined();
+
+    // 3. Replace with new editor in same container
+    // This causes LinkToolbarController of editor1 to unmount and editor2 to mount
+    const editor2 = BlockNoteEditor.create();
+    flushSync(() => {
+      root.render(
+        <React.StrictMode>
+          <BlockNoteView editor={editor2} />
+        </React.StrictMode>,
+      );
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    const editor2DomElement = editor2.domElement;
+    expect(editor2DomElement).toBeDefined();
+
+    // 4. Simulate mouseover on the OLD element.
+    // If the listener was leaked (due to editor.domElement being null/changed at cleanup),
+    // this will fire the callback. callback uses editor1 which might be in bad state -> Crash.
+
+    expect(() => {
+      editor1DomElement!.dispatchEvent(
+        new MouseEvent("mouseover", { bubbles: true }),
+      );
+    }).not.toThrow();
+
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    // Cleanup
+    editor1._tiptapEditor.destroy();
+    editor2._tiptapEditor.destroy();
+    root.unmount();
+  });
+});