slate-yjs icon indicating copy to clipboard operation
slate-yjs copied to clipboard

「Bug」Absolute position doesn't match slateRoot, cannot descent into text

Open niuyueyang2022 opened this issue 2 years ago • 6 comments

image result

niuyueyang2022 avatar May 10 '22 03:05 niuyueyang2022

I am seeing this very occasionally also. I think it has something to do with react-editor and the underlying Y.Doc getting out of sync.

rcbevans avatar Nov 22 '22 02:11 rcbevans

I'm still seeing this an have a solid repro.

Repro:

  • Have two editors side by side connected to the same doc. The editor uses useDecorateRemoteCursors based on the example to render remote cursor information.
  • Type something in the editor and select it.
  • In the second editor, select the same text and apply list (numbered or bulleted) formatting, then immediately remove it again.
  • Editor crashes with the following error:
Uncaught Error: Absolute position doesn't match slateRoot, cannot descent into text
    absolutePositionToSlatePoint position.ts:48
    relativePositionToSlatePoint position.ts:79
    relativeRangeToSlateRange position.ts:274
    getCursorRange getCursorRange.ts:31
    useDecorateRemoteCursors useDecorateRemoteCursors.ts:104
    useDecorateRemoteCursors useDecorateRemoteCursors.ts:103
    Editable editable.tsx:782
    React 14
    renderWithHooks
    updateFunctionComponent
    beginWork
    callCallback2
    invokeGuardedCallbackDev
    invokeGuardedCallback
    beginWork$1
    performUnitOfWork
    workLoopSync
    renderRootSync
    recoverFromConcurrentError
    performSyncWorkOnRoot
    flushSyncCallbacks
    ensureRootIsScheduled
 position.ts:48:10
    absolutePositionToSlatePoint position.ts:48
    relativePositionToSlatePoint position.ts:79
    relativeRangeToSlateRange position.ts:274
    getCursorRange getCursorRange.ts:31
    useDecorateRemoteCursors useDecorateRemoteCursors.ts:104
    FlattenIntoArray self-hosted:656
    flatMap self-hosted:634
    useDecorateRemoteCursors useDecorateRemoteCursors.ts:103
    Editable editable.tsx:782
    React 14

In my schema, the initial text is a paragraph

{ type: 'paragraph', children: [{text: 'something}]}

The code to toggle the list formatting is

export const toggleBlock = (
  editor: Editor,
  format: Element['type'],
  additionalProps?: Partial<Element>,
): void => {
  const isActive = isBlockActive(editor, format);
  const isList = LIST_TYPES.includes(format);

  Editor.withoutNormalizing(editor, () => {
    Transforms.unwrapNodes(editor, {
      mode: 'all',
      match: (n) => Element.isElement(n) && LIST_TYPES.includes(n.type),
      split: true,
    });

    Transforms.setNodes(editor, {
      type: isActive ? 'paragraph' : isList ? 'list-item' : format,
      ...additionalProps,
    });

    if (!isActive && isList) {
      const block = { type: format, children: [] } as CustomElement;
      Transforms.wrapNodes(editor, block);
    }
  });
};

Which converts the paragraph to

{ type: 'numbered-list', children: [{ type: 'list-item', children: [{text: 'something}]}]}

The initial toggle to apply the formatting works fine, and the selection status is preserved in both editors, but on removing the formatting again, the editor crashes, leading me to think the issue relates to Transforms.unwrapNodes.

rcbevans avatar Jun 07 '23 20:06 rcbevans

It seems that the initial remote cursor selection is something like [0,0], [0, -1], then after making the content a list it becomes [0, 0, 0], [0, 0, -1], but after unwrapping, the selection path is now invalid, and should instead be [0, 0], [0, -1] again, but instead, useDecorateRemoteCursors throws because it can't index down to [0, 0, 0], [0, 0, -1] and the remote hasn't yet updated its cursor information.

rcbevans avatar Jun 07 '23 20:06 rcbevans

@niuyueyang2022 did you ever figure out a workaround for this?

rcbevans avatar Jun 07 '23 20:06 rcbevans

The problematic code is

export function absolutePositionToSlatePoint(
  sharedRoot: Y.XmlText,
  slateRoot: Node,
  { type, index, assoc }: Y.AbsolutePosition
): BasePoint | null {
  if (!(type instanceof Y.XmlText)) {
    throw new Error('Absolute position points to a non-XMLText');
  }

  const parentPath = getSlatePath(sharedRoot, slateRoot, type);
  const parent = Node.get(slateRoot, parentPath);

  if (Text.isText(parent)) {
    throw new Error(
      "Absolute position doesn't match slateRoot, cannot descent into text"
    );
  }

  const [pathOffset, textOffset] = yOffsetToSlateOffsets(parent, index, {
    assoc,
  });

  const target = parent.children[pathOffset];
  if (!Text.isText(target)) {
    return null;
  }

  return { path: [...parentPath, pathOffset], offset: textOffset };
}

When the nodes are being unwrapped, the deepest text node is becoming it's parent, and so

  if (Text.isText(parent)) {
    throw new Error(
      "Absolute position doesn't match slateRoot, cannot descent into text"
    );
  }

is throwing.

Two questions:

  1. Why is this error being thrown here? What are the implications of returning null instead (and maybe console.erroring) like is done later
  const target = parent.children[pathOffset];
  if (!Text.isText(target)) {
    return null;
  }
  1. Something like remote cursors failing to render is not an exceptional failure and doesn't really have any justification to crash the whole editor (or site, if lacking error boundaries). Since this error is being thrown in a hook, it's erroring the whole component stack up the nearest error boundary for something the user doesn't necessarily care about and will likely resolve as soon as the remove cursor moves within the updated content.

I'm happy to fix this if @BitPhinix can provide some insight on how best to go about it, either return null from absolutePositionToSlatePoint, or catch the error within useDecorateRemoteCursors/getCursorRange and return null if absolutePositionToSlatePoint throws.

rcbevans avatar Jun 07 '23 21:06 rcbevans

I'm getting this in undo scenarios (using withYHistory). Specifically, it is trigged for single void elements. void inline elements work fine. If I cut more than the void element, undo works fine. So it's trigger just for the case where a single void element (having a single {'text': ''} child is the subject of an undo/redo.

thovden avatar Jan 14 '24 12:01 thovden