diff --git a/.changeset/validate-selection-defer.md b/.changeset/validate-selection-defer.md new file mode 100644 index 0000000000..811147372b --- /dev/null +++ b/.changeset/validate-selection-defer.md @@ -0,0 +1,11 @@ +--- +'@portabletext/editor': patch +--- + +fix: defer selection validation when the DOM hasn't caught up to the model + +When a behavior runs an action set that synchronously mutates structure (insert + delete a sibling, replace a block), the editor's `MutationObserver` fires while React is still mid-commit. The selection validation machine then runs against a model that's ahead of the DOM, `toDOMRange` throws, and the catch path used to deselect and re-select the top of the document, clobbering any selection the action set placed. + +The validation machine now treats the throw as "DOM hasn't caught up yet" rather than "selection is invalid." It re-fires itself in a microtask, giving React time to commit. After three failed retries it stops, leaving the selection alone instead of forcing it to the top of the document. + +Consumer behaviors that previously had to pre-emptively `select(null)` and `queueMicrotask(re-select)` to work around this can drop the workaround. diff --git a/packages/editor/src/editor/validate-selection-machine.ts b/packages/editor/src/editor/validate-selection-machine.ts index d5d51e9efd..b9e39a452a 100644 --- a/packages/editor/src/editor/validate-selection-machine.ts +++ b/packages/editor/src/editor/validate-selection-machine.ts @@ -1,14 +1,15 @@ import {setup} from 'xstate' -import {applyDeselect, applySelect} from '../internal-utils/apply-selection' import {debug} from '../internal-utils/debug' import {DOMEditor} from '../slate/dom/plugin/dom-editor' -import {start} from '../slate/editor/start' import type {PortableTextSlateEditor} from '../types/slate-editor' +const MAX_RETRIES = 3 + const validateSelectionSetup = setup({ types: { context: {} as { slateEditor: PortableTextSlateEditor + retryCount: number }, input: {} as { slateEditor: PortableTextSlateEditor @@ -25,8 +26,22 @@ const validateSelectionSetup = setup({ }) const validateSelectionAction = validateSelectionSetup.createAction( - ({context, event}) => { - validateSelection(context.slateEditor, event.editorElement) + ({context, event, self}) => { + const result = validateSelection(context.slateEditor, event.editorElement) + + if (result === 'retry' && context.retryCount < MAX_RETRIES) { + context.retryCount++ + // The DOM hasn't caught up to the model yet. Defer the next attempt + // to a microtask so React has a chance to commit the pending render. + queueMicrotask(() => { + self.send({ + type: 'validate selection', + editorElement: event.editorElement, + }) + }) + } else { + context.retryCount = 0 + } }, ) @@ -34,6 +49,7 @@ export const validateSelectionMachine = validateSelectionSetup.createMachine({ id: 'validate selection', context: ({input}) => ({ slateEditor: input.slateEditor, + retryCount: 0, }), initial: 'idle', states: { @@ -89,14 +105,19 @@ export const validateSelectionMachine = validateSelectionSetup.createMachine({ // Also the other way around, when the DOMEditor will try to create a DOM Range // from the current slateEditor.selection, it may throw unrecoverable errors // if the current editor.selection is invalid according to the DOM. -// If this is the case, default to selecting the top of the document, if the -// user already had a selection. +// +// When `toDOMRange` throws, it usually means the DOM hasn't yet rendered the +// latest model state — for example mid-action-set, between operation commit +// and React render. Returning 'retry' lets the machine defer the next attempt +// to a microtask. After MAX_RETRIES, fall back to deselecting (without +// selecting the top of the document, which would silently override the +// caller's intended selection). function validateSelection( slateEditor: PortableTextSlateEditor, editorElement: HTMLDivElement, -) { +): 'retry' | undefined { if (!slateEditor.selection) { - return + return undefined } let root: Document | ShadowRoot | undefined @@ -107,17 +128,17 @@ function validateSelection( if (!root) { // The editor has most likely been unmounted - return + return undefined } // Return if the editor isn't the active element if (editorElement !== root.activeElement) { - return + return undefined } const window = DOMEditor.getWindow(slateEditor) const domSelection = window.getSelection() if (!domSelection || domSelection.rangeCount === 0) { - return + return undefined } const existingDOMRange = domSelection.getRangeAt(0) try { @@ -132,14 +153,11 @@ function validateSelection( // Set the correct range domSelection.addRange(newDOMRange) } + return undefined } catch { - debug.selection(`Could not resolve selection, selecting top document`) - // Deselect the editor - applyDeselect(slateEditor) - // Select top document if there is a top block to select - if (slateEditor.children.length > 0) { - applySelect(slateEditor, start(slateEditor, [])) - } - slateEditor.onChange() + debug.selection( + 'Could not resolve selection, deferring validation to next tick', + ) + return 'retry' } } diff --git a/packages/editor/tests/validate-selection-action-set.test.tsx b/packages/editor/tests/validate-selection-action-set.test.tsx new file mode 100644 index 0000000000..55db2fd89b --- /dev/null +++ b/packages/editor/tests/validate-selection-action-set.test.tsx @@ -0,0 +1,120 @@ +import {createTestKeyGenerator} from '@portabletext/test' +import {describe, expect, test, vi} from 'vitest' +import {userEvent} from 'vitest/browser' +import {defineSchema} from '../src' +import {execute} from '../src/behaviors/behavior.types.action' +import {defineBehavior} from '../src/behaviors/behavior.types.behavior' +import {BehaviorPlugin} from '../src/plugins/plugin.behavior' +import {createTestEditor} from '../src/test/vitest' + +describe('validate-selection-machine: action set across blocks', () => { + test('Scenario: An action set that swaps a block keeps selection on the new block', async () => { + const keyGenerator = createTestKeyGenerator() + const blockKey = keyGenerator() + const spanKey = keyGenerator() + const imageKey = keyGenerator() + const newBlockKey = 'newBlock' + const newSpanKey = 'newSpan' + + const {editor, locator} = await createTestEditor({ + keyGenerator, + schemaDefinition: defineSchema({ + blockObjects: [ + {name: 'image', fields: [{name: 'src', type: 'string'}]}, + ], + }), + initialValue: [ + { + _key: blockKey, + _type: 'block', + children: [{_key: spanKey, _type: 'span', text: 'foo'}], + }, + { + _key: imageKey, + _type: 'image', + src: 'https://example.com/image.jpg', + }, + ], + children: ( + [ + execute({ + type: 'insert.block', + block: { + _key: newBlockKey, + _type: 'block', + children: [ + {_key: newSpanKey, _type: 'span', text: '', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + placement: 'after', + select: 'start', + }), + execute({ + type: 'delete.block', + at: [{_key: imageKey}], + }), + ], + ], + }), + ]} + /> + ), + }) + + await userEvent.click(locator) + + editor.send({ + type: 'select', + at: { + anchor: {path: [{_key: imageKey}], offset: 0}, + focus: {path: [{_key: imageKey}], offset: 0}, + }, + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.selection?.anchor.path).toEqual([ + {_key: imageKey}, + ]) + }) + + editor.send({type: 'custom.swap-image'} as never) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: blockKey, + _type: 'block', + children: [{_key: spanKey, _type: 'span', text: 'foo', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: newBlockKey, + _type: 'block', + children: [{_key: newSpanKey, _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + expect(editor.getSnapshot().context.selection).toEqual({ + anchor: { + path: [{_key: newBlockKey}, 'children', {_key: newSpanKey}], + offset: 0, + }, + focus: { + path: [{_key: newBlockKey}, 'children', {_key: newSpanKey}], + offset: 0, + }, + backward: false, + }) + }) +})