fix(editor): propagate normalization of remote changes

BREAKING CHANGE: removed isLocal, asLocal, applySlateOperations, applyYjsEvents from YjsEditor. asRemote applies to the callback function only, as opposed to spanning to the next tick as well, which prevents it from inadvertently marking operations such as normalizations as remote operations.

src/applyToSlate/index.ts

@@ -30,8 +30,6 @@ export function translateYjsEvent(
 
 /**
  * Applies multiple yjs events to a slate editor.
- *
- * @param event
  */
 export function applyYjsEvents(editor: Editor, events: Y.YEvent[]): void {
   Editor.withoutNormalizing(editor, () => {

src/applyToYjs/index.ts

@@ -35,20 +35,18 @@ export function applySlateOp(
 
 /**
  * Applies slate operations to a SharedType
- *
- * @param sharedType
- * @param op
  */
 export default function applySlateOps(
   sharedType: SharedType,
-  ops: Operation[]
+  ops: Operation[],
+  origin: unknown
 ): SharedType {
   invariant(sharedType.doc, 'Shared type without attached document');
 
   if (ops.length > 0) {
     sharedType.doc.transact(() => {
       ops.forEach((op) => applySlateOp(sharedType, op));
-    });
+    }, origin);
   }
 
   return sharedType;

src/model/index.ts

@@ -6,6 +6,8 @@ export type SyncElement = Y.Map<any>;
 export type SharedType = Y.Array<SyncElement>;
 export type SyncNode = SharedType | SyncElement;
 
+export const slateYjsSymbol = Symbol('slate-yjs');
+
 export interface Cursor extends Range {
   data: {
     [key: string]: unknown;

src/plugin/useCursors.ts

@@ -38,7 +38,7 @@ export const useCursors = (
         })
         .filter((cursor) => cursor.anchor && cursor.focus);
 
-      setCursorData((newCursorData as unknown) as Cursor[]);
+      setCursorData(newCursorData as unknown as Cursor[]);
     });
   }, [editor]);
 

src/plugin/yjsEditor.ts

@@ -3,11 +3,11 @@ import invariant from 'tiny-invariant';
 import * as Y from 'yjs';
 import { applyYjsEvents } from '../applyToSlate';
 import applySlateOps from '../applyToYjs';
-import { SharedType } from '../model';
-import { toSlateDoc } from '../utils/convert';
+import { SharedType, slateYjsSymbol } from '../model';
+import { toSlateDoc } from '../utils';
 
 const IS_REMOTE: WeakSet<Editor> = new WeakSet();
-const IS_LOCAL: WeakSet<Editor> = new WeakSet();
+const LOCAL_OPERATIONS: WeakMap<Editor, Set<Operation>> = new WeakMap();
 const SHARED_TYPES: WeakMap<Editor, SharedType> = new WeakMap();
 
 export interface YjsEditor extends Editor {
@@ -34,15 +34,6 @@ export const YjsEditor = {
     return sharedType;
   },
 
-  /**
-   * Applies a slate operations to the bound shared type.
-   */
-  applySlateOperations: (editor: YjsEditor, operations: Operation[]): void => {
-    YjsEditor.asLocal(editor, () => {
-      applySlateOps(YjsEditor.sharedType(editor), operations);
-    });
-  },
-
   /**
    * Returns whether the editor currently is applying remote changes.
    */
@@ -60,40 +51,51 @@ export const YjsEditor = {
     fn();
 
     if (!wasRemote) {
-      Promise.resolve().then(() => IS_REMOTE.delete(editor));
+      IS_REMOTE.delete(editor);
     }
   },
+};
 
-  /**
-   * Apply Yjs events to slate
-   */
-  applyYjsEvents: (editor: YjsEditor, events: Y.YEvent[]): void => {
-    YjsEditor.asRemote(editor, () => {
-      applyYjsEvents(editor, events);
-    });
-  },
+function localOperations(editor: YjsEditor): Set<Operation> {
+  const operations = LOCAL_OPERATIONS.get(editor);
+  invariant(operations, 'YjsEditor without attached local operations');
+  return operations;
+}
 
-  /**
-   * Performs an action as a local operation.
-   */
-  asLocal: (editor: YjsEditor, fn: () => void): void => {
-    const wasLocal = YjsEditor.isLocal(editor);
-    IS_LOCAL.add(editor);
+function trackLocalOperations(editor: YjsEditor, operation: Operation): void {
+  if (!YjsEditor.isRemote(editor)) {
+    localOperations(editor).add(operation);
+  }
+}
 
-    fn();
+/**
+ * Applies a slate operations to the bound shared type.
+ */
+function applyLocalOperations(editor: YjsEditor): void {
+  const editorLocalOperations = localOperations(editor);
 
-    if (!wasLocal) {
-      IS_LOCAL.delete(editor);
-    }
-  },
+  applySlateOps(
+    YjsEditor.sharedType(editor),
+    Array.from(editorLocalOperations),
+    slateYjsSymbol
+  );
 
-  /**
-   * Returns whether the editor currently is applying a remote change to the yjs doc.
-   */
-  isLocal: (editor: YjsEditor): boolean => {
-    return IS_LOCAL.has(editor);
-  },
-};
+  editorLocalOperations.clear();
+}
+
+/**
+ * Apply Yjs events to slate
+ */
+function applyRemoteYjsEvents(editor: YjsEditor, events: Y.YEvent[]): void {
+  Editor.withoutNormalizing(editor, () =>
+    YjsEditor.asRemote(editor, () =>
+      applyYjsEvents(
+        editor,
+        events.filter((event) => event.transaction.origin !== slateYjsSymbol)
+      )
+    )
+  );
+}
 
 export function withYjs<T extends Editor>(
   editor: T,
@@ -103,23 +105,22 @@ export function withYjs<T extends Editor>(
 
   e.sharedType = sharedType;
   SHARED_TYPES.set(editor, sharedType);
+  LOCAL_OPERATIONS.set(editor, new Set());
 
-  setTimeout(() => {
-    YjsEditor.synchronizeValue(e);
-  });
+  setTimeout(() => YjsEditor.synchronizeValue(e), 0);
 
-  sharedType.observeDeep((events) => {
-    if (!YjsEditor.isLocal(e)) {
-      YjsEditor.applyYjsEvents(e, events);
-    }
-  });
+  sharedType.observeDeep((events) => applyRemoteYjsEvents(e, events));
+
+  const { apply, onChange } = e;
 
-  const { onChange } = editor;
+  e.apply = (op: Operation) => {
+    trackLocalOperations(e, op);
+
+    apply(op);
+  };
 
   e.onChange = () => {
-    if (!YjsEditor.isRemote(e)) {
-      YjsEditor.applySlateOperations(e, e.operations);
-    }
+    applyLocalOperations(e);
 
     onChange();
   };

test/collaboration.test.ts

@@ -345,8 +345,8 @@ describe('slate operations propagate between editors', () => {
   tests.forEach(([testName, input, ...cases]) => {
     it(`${testName}`, async () => {
       // Create two editors.
-      const src = createTestEditor();
-      const dst = createTestEditor();
+      const src = await createTestEditor();
+      const dst = await createTestEditor();
 
       // Set initial state for src editor, propagate changes to dst editor.
       TestEditor.applyTransform(

test/concurrent.test.ts

@@ -173,8 +173,8 @@ const tests: Test[] = [
 
 const runOneTest = async (ti: Test, tj: Test) => {
   // Create two editors.
-  const ei = createTestEditor();
-  const ej = createTestEditor();
+  const ei = await createTestEditor();
+  const ej = await createTestEditor();
 
   // Set initial state for 1st editor, propagate changes to 2nd.
   TestEditor.applyTransform(

test/plugin/yjsEditor.test.ts

@@ -0,0 +1,76 @@
+import { Node, Element } from 'slate';
+import { toSlateDoc, toSyncElement, YjsEditor } from '../../src';
+import { createTestEditor } from '../utils';
+
+// onChange fires in the next tick, see slate createEditor
+async function waitForSlateOnChangeAround(callback: () => void) {
+  await callback();
+  await Promise.resolve();
+}
+
+describe('YjsEditor', () => {
+  let yjsEditor: YjsEditor;
+
+  const initialValue: Node[] = [
+    { type: 'paragraph', children: [{ text: '' }] },
+  ];
+
+  const newElement: Element = {
+    type: 'paragraph',
+    children: [{ text: 'new' }],
+  };
+
+  beforeEach(async () => {
+    yjsEditor = await createTestEditor(initialValue);
+  });
+
+  it('should sync changes to the shared type with slate', async () => {
+    await waitForSlateOnChangeAround(() =>
+      yjsEditor.sharedType.push([toSyncElement(newElement)])
+    );
+
+    expect(yjsEditor.children).toEqual<Node[]>([...initialValue, newElement]);
+    // make sure we didn't erroneously add elements to the shared type - can happen
+    // if the observer we have for it applies "local" events
+    expect(toSlateDoc(yjsEditor.sharedType)).toEqual<Node[]>(
+      yjsEditor.children
+    );
+  });
+
+  it('should apply slate operations to yjs', async () => {
+    await waitForSlateOnChangeAround(() => yjsEditor.insertNode(newElement));
+
+    expect(toSlateDoc(yjsEditor.sharedType)).toEqual<Node[]>([
+      ...initialValue,
+      newElement,
+    ]);
+  });
+
+  it('should apply slate normalizations to yjs', async () => {
+    const normalization: Element = {
+      type: 'paragraph',
+      children: [{ text: 'some normalization' }],
+    };
+
+    const { normalizeNode } = yjsEditor;
+
+    yjsEditor.normalizeNode = (entry) => {
+      const isNewElement = entry[0].text === newElement.children[0].text;
+      if (isNewElement) {
+        yjsEditor.insertNode(normalization);
+      }
+
+      normalizeNode(entry);
+    };
+
+    await waitForSlateOnChangeAround(() =>
+      yjsEditor.sharedType.push([toSyncElement(newElement)])
+    );
+
+    expect(toSlateDoc(yjsEditor.sharedType)).toEqual<Node[]>([
+      ...initialValue,
+      newElement,
+      normalization,
+    ]);
+  });
+});

test/utils.ts

@@ -49,13 +49,18 @@ export function wait(ms = 0): Promise<void> {
   return new Promise<void>((resolve) => setTimeout(resolve, ms));
 }
 
-export function createTestEditor(value?: Node[]): TestEditor {
+export async function createTestEditor(value?: Node[]): Promise<TestEditor> {
   const doc = new Y.Doc();
   const syncType = doc.getArray<SyncElement>('content');
 
   if (value) {
     toSharedType(syncType, value);
   }
 
-  return withTest(withYjs(createEditor(), syncType));
+  const editor = withTest(withYjs(createEditor(), syncType));
+
+  // wait for value sync
+  await wait();
+
+  return editor;
 }