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

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()
+    })
+  })
+})

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

@@ -1,201 +0,0 @@
-import React, { useEffect } from 'react'
-import { createEditor, Text, Transforms } from 'slate'
-import { create, act, ReactTestRenderer } from 'react-test-renderer'
-import { Slate, withReact, Editable } from '../src'
-
-const createNodeMock = () => ({
-  ownerDocument: global.document,
-  getRootNode: () => global.document,
-})
-
-class MockResizeObserver {
-  observe() {}
-
-  unobserve() {}
-
-  disconnect() {}
-}
-
-describe('slate-react', () => {
-  window.ResizeObserver = MockResizeObserver as any
-
-  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()
-
-        let el: ReactTestRenderer
-
-        act(() => {
-          el = create(
-            <Slate
-              editor={editor}
-              initialValue={initialValue}
-              onChange={() => {}}
-            >
-              <Editable
-                renderElement={({ children }) => {
-                  useEffect(() => mounts(), [])
-
-                  return children
-                }}
-              />
-            </Slate>,
-            { createNodeMock }
-          )
-        })
-
-        // 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()
-
-        let el: ReactTestRenderer
-
-        act(() => {
-          el = create(
-            <Slate
-              editor={editor}
-              initialValue={initialValue}
-              onChange={() => {}}
-            >
-              <Editable
-                renderElement={({ children }) => {
-                  useEffect(() => mounts(), [])
-
-                  return children
-                }}
-              />
-            </Slate>,
-            { createNodeMock }
-          )
-        })
-
-        // 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(() => {
-      create(
-        <Slate
-          editor={editor}
-          initialValue={initialValue}
-          onChange={onChange}
-          onValueChange={onValueChange}
-          onSelectionChange={onSelectionChange}
-        >
-          <Editable />
-        </Slate>,
-        { createNodeMock }
-      )
-    })
-
-    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(() => {
-      create(
-        <Slate
-          editor={editor}
-          initialValue={initialValue}
-          onChange={onChange}
-          onValueChange={onValueChange}
-          onSelectionChange={onSelectionChange}
-        >
-          <Editable />
-        </Slate>,
-        { createNodeMock }
-      )
-    })
-
-    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(() => {
-      create(
-        <Slate
-          editor={editor}
-          initialValue={initialValue}
-          onChange={onChange}
-          onValueChange={onValueChange}
-          onSelectionChange={onSelectionChange}
-        >
-          <Editable />
-        </Slate>,
-        { createNodeMock }
-      )
-    })
-
-    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()
-  })
-})

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

@@ -0,0 +1,90 @@
+import React, { useEffect } from 'react'
+import { createEditor, Text, Transforms } from 'slate'
+import { act, render } from '@testing-library/react'
+import { Slate, withReact, Editable, ReactEditor } from '../src'
+
+describe('slate-react', () => {
+  describe('ReactEditor', () => {
+    describe('.focus', () => {
+      test('should set focus in top of document with no editor selection', async () => {
+        const editor = withReact(createEditor())
+        const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+        const testSelection = {
+          anchor: { path: [0, 0], offset: 0 },
+          focus: { path: [0, 0], offset: 0 },
+        }
+
+        act(() => {
+          render(
+            <Slate editor={editor} initialValue={initialValue}>
+              <Editable />
+            </Slate>
+          )
+        })
+
+        expect(editor.selection).toBe(null)
+
+        await act(async () => {
+          ReactEditor.focus(editor)
+        })
+
+        expect(editor.selection).toEqual(testSelection)
+
+        await act(async () => {
+          const windowSelection = ReactEditor.getWindow(editor).getSelection()
+          expect(windowSelection?.focusNode?.textContent).toBe('test')
+          expect(windowSelection?.anchorNode?.textContent).toBe('test')
+          expect(windowSelection?.anchorOffset).toBe(
+            testSelection.anchor.offset
+          )
+          expect(windowSelection?.focusOffset).toBe(testSelection.focus.offset)
+        })
+      })
+
+      test('should be able to call .focus without getting toDOMNode errors', async () => {
+        const editor = withReact(createEditor())
+        const initialValue = [{ type: 'block', children: [{ text: 'test' }] }]
+        const propagatedValue = [
+          { type: 'block', children: [{ text: 'foo' }] },
+          { type: 'block', children: [{ text: 'bar' }] },
+        ]
+
+        const testSelection = {
+          anchor: { path: [1, 0], offset: 0 },
+          focus: { path: [1, 0], offset: 3 },
+        }
+
+        act(() => {
+          render(
+            <Slate editor={editor} initialValue={initialValue}>
+              <Editable />
+            </Slate>
+          )
+        })
+
+        await act(async () => {
+          Transforms.removeNodes(editor, { at: [0] })
+          Transforms.insertNodes(editor, propagatedValue)
+          ReactEditor.focus(editor) // Note: calling focus in the middle of these transformations.
+          Transforms.select(editor, testSelection)
+        })
+
+        expect(editor.selection).toEqual(testSelection)
+
+        await act(async () => {
+          ReactEditor.focus(editor)
+        })
+
+        await act(async () => {
+          const windowSelection = ReactEditor.getWindow(editor).getSelection()
+          expect(windowSelection?.focusNode?.textContent).toBe('bar')
+          expect(windowSelection?.anchorNode?.textContent).toBe('bar')
+          expect(windowSelection?.anchorOffset).toBe(
+            testSelection.anchor.offset
+          )
+          expect(windowSelection?.focusOffset).toBe(testSelection.focus.offset)
+        })
+      })
+    })
+  })
+})