Fix issue with ReactEditor.focus + tests (#5527)

* Fix issue with slate-react static ReactEditor.focus method

This will make sure we don't try to focus the editor while it's in the midst of applying operations.
If this is the case, retry setting focus in the next tick.

* Replace react-test-renderer with @testing-library/react

We need to be able to test against window features, like the DOM selection.
@testing-library/react has a very similar API, but have also these features,
which react-test-renderer is missing.

* Rewrite tests for @testing-library/react

This will rewrite the existing tests for Editable and move them into a own file.

* Add tests for ReactEditor.focus

* Add changeset

.changeset/thick-fishes-dance.md

@@ -0,0 +1,7 @@
+---
+'slate-react': minor
+---
+
+Fixes a bug with `ReactEditor.focus` where it would throw an error if the editor was in the middle of applying pending operations.
+With this change, setting focus will be retried until the editor no longer has any pending operations.
+Calling `ReactEditor.focus` on a editor without a current selection, will now make a selection in the top of the document.

packages/slate-react/package.json

@@ -26,15 +26,14 @@
   },
   "devDependencies": {
     "@babel/runtime": "^7.23.2",
+    "@testing-library/react": "^14.0.0",
     "@types/jest": "29.5.6",
     "@types/jsdom": "^21.1.4",
     "@types/react": "^18.2.28",
     "@types/react-dom": "^18.2.13",
-    "@types/react-test-renderer": "^18.0.3",
     "@types/resize-observer-browser": "^0.1.8",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
-    "react-test-renderer": "^18.2.0",
     "slate": "^0.100.0",
     "slate-hyperscript": "^0.100.0",
     "source-map-loader": "^4.0.1"

packages/slate-react/src/plugin/react-editor.ts

@@ -117,7 +117,7 @@ export interface ReactEditorInterface {
   /**
    * Focus the editor.
    */
-  focus: (editor: ReactEditor) => void
+  focus: (editor: ReactEditor, options?: { retries: number }) => void
 
   /**
    * Return the host window of the current editor.
@@ -411,19 +411,44 @@ export const ReactEditor: ReactEditorInterface = {
     )
   },
 
-  focus: editor => {
+  focus: (editor, options = { retries: 5 }) => {
+    // Return if already focused
+    if (IS_FOCUSED.get(editor)) {
+      return
+    }
+
+    // Retry setting focus if the editor has pending operations.
+    // The DOM (selection) is unstable while changes are applied.
+    // Retry until retries are exhausted or editor is focused.
+    if (options.retries <= 0) {
+      throw new Error(
+        'Could not set focus, editor seems stuck with pending operations'
+      )
+    }
+    if (editor.operations.length > 0) {
+      setTimeout(() => {
+        ReactEditor.focus(editor, { retries: options.retries - 1 })
+      }, 10)
+      return
+    }
+
     const el = ReactEditor.toDOMNode(editor, editor)
     const root = ReactEditor.findDocumentOrShadowRoot(editor)
-    IS_FOCUSED.set(editor, true)
-
     if (root.activeElement !== el) {
+      // Ensure that the DOM selection state is set to the editor's selection
       if (editor.selection && root instanceof Document) {
         const domSelection = root.getSelection()
         const domRange = ReactEditor.toDOMRange(editor, editor.selection)
         domSelection?.removeAllRanges()
         domSelection?.addRange(domRange)
       }
+      // Create a new selection in the top of the document if missing
+      if (!editor.selection) {
+        Transforms.select(editor, Editor.start(editor, []))
+        editor.onChange()
+      }
       el.focus({ preventScroll: true })
+      IS_FOCUSED.set(editor, true)
     }
   },
 

packages/slate-react/test/editable.spec.tsx

@@ -0,0 +1,204 @@
+import React, { useEffect } from 'react'
+import { createEditor, Text, Transforms } from 'slate'
+import { act, render } from '@testing-library/react'
+import { Slate, withReact, Editable } from '../src'
+
+describe('slate-react', () => {
+  describe('Editable', () => {
+    describe('NODE_TO_KEY logic', () => {
+      test('should not unmount the node that gets split on a split_node operation', async () => {
+        const editor = withReact(createEditor())
+        const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+        const mounts = jest.fn()
+
+        act(() => {
+          render(
+            <Slate
+              editor={editor}
+              initialValue={initialValue}
+              onChange={() => {}}
+            >
+              <Editable
+                renderElement={({ children }) => {
+                  useEffect(() => mounts(), [])
+
+                  return children
+                }}
+              />
+            </Slate>
+          )
+        })
+
+        // slate updates at next tick, so we need this to be async
+        await act(async () =>
+          Transforms.splitNodes(editor, { at: { path: [0, 0], offset: 2 } })
+        )
+
+        // 2 renders, one for the main element and one for the split element
+        expect(mounts).toHaveBeenCalledTimes(2)
+      })
+
+      test('should not unmount the node that gets merged into on a merge_node operation', async () => {
+        const editor = withReact(createEditor())
+        const initialValue = [
+          { type: 'block', children: [{ text: 'te' }] },
+          { type: 'block', children: [{ text: 'st' }] },
+        ]
+        const mounts = jest.fn()
+
+        act(() => {
+          render(
+            <Slate
+              editor={editor}
+              initialValue={initialValue}
+              onChange={() => {}}
+            >
+              <Editable
+                renderElement={({ children }) => {
+                  useEffect(() => mounts(), [])
+
+                  return children
+                }}
+              />
+            </Slate>
+          )
+        })
+
+        // slate updates at next tick, so we need this to be async
+        await act(async () =>
+          Transforms.mergeNodes(editor, { at: { path: [0, 0], offset: 0 } })
+        )
+
+        // only 2 renders for the initial render
+        expect(mounts).toHaveBeenCalledTimes(2)
+      })
+    })
+    test('calls onSelectionChange when editor select change', async () => {
+      const editor = withReact(createEditor())
+      const initialValue = [
+        { type: 'block', children: [{ text: 'te' }] },
+        { type: 'block', children: [{ text: 'st' }] },
+      ]
+      const onChange = jest.fn()
+      const onValueChange = jest.fn()
+      const onSelectionChange = jest.fn()
+
+      act(() => {
+        render(
+          <Slate
+            editor={editor}
+            initialValue={initialValue}
+            onChange={onChange}
+            onValueChange={onValueChange}
+            onSelectionChange={onSelectionChange}
+          >
+            <Editable />
+          </Slate>
+        )
+      })
+
+      await act(async () =>
+        Transforms.select(editor, { path: [0, 0], offset: 2 })
+      )
+
+      expect(onSelectionChange).toHaveBeenCalled()
+      expect(onChange).toHaveBeenCalled()
+      expect(onValueChange).not.toHaveBeenCalled()
+    })
+
+    test('calls onValueChange when editor children change', async () => {
+      const editor = withReact(createEditor())
+      const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+      const onChange = jest.fn()
+      const onValueChange = jest.fn()
+      const onSelectionChange = jest.fn()
+
+      act(() => {
+        render(
+          <Slate
+            editor={editor}
+            initialValue={initialValue}
+            onChange={onChange}
+            onValueChange={onValueChange}
+            onSelectionChange={onSelectionChange}
+          >
+            <Editable />
+          </Slate>
+        )
+      })
+
+      await act(async () => Transforms.insertText(editor, 'Hello word!'))
+
+      expect(onValueChange).toHaveBeenCalled()
+      expect(onChange).toHaveBeenCalled()
+      expect(onSelectionChange).not.toHaveBeenCalled()
+    })
+
+    test('calls onValueChange when editor setNodes', async () => {
+      const editor = withReact(createEditor())
+      const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+      const onChange = jest.fn()
+      const onValueChange = jest.fn()
+      const onSelectionChange = jest.fn()
+
+      act(() => {
+        render(
+          <Slate
+            editor={editor}
+            initialValue={initialValue}
+            onChange={onChange}
+            onValueChange={onValueChange}
+            onSelectionChange={onSelectionChange}
+          >
+            <Editable />
+          </Slate>
+        )
+      })
+
+      await act(async () =>
+        Transforms.setNodes(
+          editor,
+          // @ts-ignore
+          { bold: true },
+          {
+            at: { path: [0, 0], offset: 2 },
+            match: Text.isText,
+            split: true,
+          }
+        )
+      )
+
+      expect(onChange).toHaveBeenCalled()
+      expect(onValueChange).toHaveBeenCalled()
+      expect(onSelectionChange).not.toHaveBeenCalled()
+    })
+
+    test('calls onValueChange when editor children change', async () => {
+      const editor = withReact(createEditor())
+      const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+      const onChange = jest.fn()
+      const onValueChange = jest.fn()
+      const onSelectionChange = jest.fn()
+
+      act(() => {
+        render(
+          <Slate
+            editor={editor}
+            initialValue={initialValue}
+            onChange={onChange}
+            onValueChange={onValueChange}
+            onSelectionChange={onSelectionChange}
+          >
+            <Editable />
+          </Slate>
+        )
+      })
+
+      await act(async () => Transforms.insertText(editor, 'Hello word!'))
+
+      expect(onValueChange).toHaveBeenCalled()
+      expect(onChange).toHaveBeenCalled()
+      expect(onSelectionChange).not.toHaveBeenCalled()
+    })
+  })
+})