docs: add some notes and a few refactors (#7)

* docs: add some notes and a few refactors * Update src/sync-plugin.ts Co-authored-by: Thiago Bellini Ribeiro <[email protected]> * fix: based on suggestions from comments and add a few more notes --------- Co-authored-by: Thiago Bellini Ribeiro <[email protected]>
loro-dev · Apr 3, 2024 · 34a1c29 · 34a1c29
1 parent 06510f4
commit 34a1c29
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 36 deletions.
diff --git a/package.json b/package.json
@@ -12,6 +12,7 @@
   "author": "",
   "license": "ISC",
   "dependencies": {
+    "fast-deep-equal": "^3.1.3",
     "lib0": "^0.2.42",
     "loro-crdt": "^0.13.1"
   },

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/lib.ts b/src/lib.ts
@@ -1,4 +1,5 @@
 import { simpleDiff } from "lib0/diff";
+import deepEq from "fast-deep-equal";
 
 import {
   ContainerID,
@@ -21,13 +22,31 @@ export type LoroContainer =
   | LoroText
   | LoroTree;
 export type LoroType = LoroContainer | Value;
+
+// Mapping from a Loro Container ID to a ProseMirror non-text node 
+// or to the children of a ProseMirror text node.
+//
+// - For an non-text, it will be a LoroMap mapping to a Node
+// - For a text, it will be a LoroText mapping to Node. (PM stores 
+//   rich text as arrays of text nodes, each one with its marks, 
+//   and that's why we have some conversion utilities between both)
+//
+// So that ContainerID should always be of a LoroMap or a LoroText.
+// Anything else is considered an error.
+//
+// A PM non-text node, it has attributes and children, which represents as a 
+// LoroMap with a `"attributes": LoroMap` and a `"children": LoroList` inside 
+// of it. Both that attributes and children are just part of the parent LoroMap 
+// structure, which is mapped to an actual node.
+//
+// See also: https://prosemirror.net/docs/guide/#doc.data_structures
 export type LoroNodeMapping = Map<ContainerID, Node | Node[]>;
 
 export const ROOT_DOC_KEY = "doc";
 export const ATTRIBUTES_KEY = "attributes";
 export const CHILDREN_KEY = "children";
 
-export function updateDoc(
+export function updateLoroOnPmChange(
   doc: Loro,
   mapping: LoroNodeMapping,
   oldEditorState: EditorState,
@@ -81,7 +100,7 @@ export function createNodeFromLoroObj(
     try {
       retval = schema.node(nodeName, attributes.toJson(), mappedChildren);
     } catch (e) {
-      // An error occured while creating the node.
+      // An error occurred while creating the node.
       // This is probably a result of a concurrent action.
       console.error(e);
     }
@@ -99,7 +118,7 @@ export function createNodeFromLoroObj(
         }
         retval.push(schema.text(delta.insert, marks));
       } catch (e) {
-        // An error occured while creating the node.
+        // An error occurred while creating the node.
         // This is probably a result of a concurrent action.
         console.error(e);
       }
@@ -209,10 +228,14 @@ function eqLoroTextNodes(obj: LoroText, nodes: Node[]) {
   );
 }
 
+
+// TODO: extract code about equality into a single file
+/**
+ * Whether the loro object is equal to the node.
+ */
 function eqLoroObjNode(
   obj: LoroType,
   node: Node | Node[],
-  mapping: LoroNodeMapping,
 ): boolean {
   if (obj instanceof LoroMap) {
     if (Array.isArray(node) || !eqNodeName(obj, node)) {
@@ -225,7 +248,7 @@ function eqLoroObjNode(
       loroChildren.length === normalizedContent.length &&
       eqAttrs(getLoroMapAttributes(obj).toJson(), node.attrs) &&
       normalizedContent.every((childNode, i) =>
-        eqLoroObjNode(loroChildren.get(i)!, childNode, mapping),
+        eqLoroObjNode(loroChildren.get(i)!, childNode),
       )
     );
   }
@@ -326,7 +349,7 @@ function computeChildEqualityFactor(
     } else if (
       leftLoro == null ||
       leftNode == null ||
-      !eqLoroObjNode(leftLoro, leftNode, mapping)
+      !eqLoroObjNode(leftLoro, leftNode)
     ) {
       break;
     }
@@ -346,7 +369,7 @@ function computeChildEqualityFactor(
     } else if (
       rightLoro == null ||
       rightNode == null ||
-      !eqLoroObjNode(rightLoro, rightNode, mapping)
+      !eqLoroObjNode(rightLoro, rightNode)
     ) {
       break;
     }
@@ -417,15 +440,11 @@ export function updateLoroMapAttributes(
   const pAttrs = node.attrs;
   for (const [key, value] of Object.entries(node.attrs)) {
     if (value !== null) {
-      // TODO: Will calling `set` without `get` generate diffs if the content is the same?
-      if (attrs.get(key) !== value) {
+      if (!deepEq(attrs.get(key), value)) {
         attrs.set(key, value);
       }
     } else {
-      // TODO: Can we just call delete without checking this here?
-      if (keys.has(key)) {
-        attrs.delete(key);
-      }
+      attrs.delete(key);
     }
     keys.delete(key);
   }
@@ -469,8 +488,9 @@ export function updateLoroMapChildren(
         leftLoro != null &&
         leftNode != null &&
         isContainer(leftLoro) &&
-        eqLoroObjNode(leftLoro, leftNode, mapping)
+        eqLoroObjNode(leftLoro, leftNode)
       ) {
+        // If they actually equal but have different pointers, update the mapping
         // update mapping
         mapping.set(leftLoro.id, leftNode);
       } else {
@@ -493,8 +513,9 @@ export function updateLoroMapChildren(
         rightLoro != null &&
         rightNode != null &&
         isContainer(rightLoro) &&
-        eqLoroObjNode(rightLoro, rightNode, mapping)
+        eqLoroObjNode(rightLoro, rightNode)
       ) {
+        // If they actually equal but have different pointers, update the mapping
         // update mapping
         mapping.set(rightLoro.id, rightNode);
       } else {
@@ -558,6 +579,7 @@ export function updateLoroMapChildren(
         updateLoroMap(rightLoro as LoroMap, rightNode as Node, mapping);
         right += 1;
       } else {
+        // recreate the element at left
         const child = loroChildren.get(left);
         if (isContainer(child)) {
           mapping.delete(child.id);
@@ -576,6 +598,7 @@ export function updateLoroMapChildren(
     loroChildren.get(0) instanceof LoroText
   ) {
     // Only delete the content of the LoroText to retain remote changes on the same LoroText object
+    // Otherwise, the LoroText object will be deleted and all the concurrent edits to the same LoroText object will be lost
     const loroText = loroChildren.get(0) as LoroText;
     mapping.delete(loroText.id);
     loroText.delete(0, loroText.length);

diff --git a/src/sync-plugin.ts b/src/sync-plugin.ts
@@ -12,19 +12,19 @@ import {
   LoroNodeMapping,
   clearChangedNodes,
   createNodeFromLoroObj,
-  updateDoc,
+  updateLoroOnPmChange,
 } from "./lib";
 
-export const loroSyncPluginKey = new PluginKey("loro-sync");
+export const loroSyncPluginKey = new PluginKey<LoroSyncPluginState>("loro-sync");
 
 type PluginTransactionType =
   | {
-      type: "doc-changed";
-    }
+    type: "doc-changed";
+  }
   | {
-      type: "update-state";
-      state: Partial<LoroSyncPluginState>;
-    };
+    type: "update-state";
+    state: Partial<LoroSyncPluginState>;
+  };
 
 export interface LoroSyncPluginProps {
   doc: Loro;
@@ -44,7 +44,7 @@ export const LoroSyncPlugin = (props: LoroSyncPluginProps): Plugin => {
     props: {
       editable: (state) => {
         const syncState = loroSyncPluginKey.getState(state);
-        return syncState.snapshot == null;
+        return syncState?.snapshot == null;
       },
     },
     state: {
@@ -58,7 +58,7 @@ export const LoroSyncPlugin = (props: LoroSyncPluginProps): Plugin => {
         ) as PluginTransactionType | null;
         switch (meta?.type) {
           case "doc-changed":
-            updateDoc(state.doc, state.mapping, oldEditorState, newEditorState);
+            updateLoroOnPmChange(state.doc, state.mapping, oldEditorState, newEditorState);
             break;
           case "update-state":
             state = { ...state, ...meta.state };
@@ -80,7 +80,7 @@ export const LoroSyncPlugin = (props: LoroSyncPluginProps): Plugin => {
     view: (view: EditorView) => {
       const timeoutId = setTimeout(() => init(view), 0);
       return {
-        update: (view: EditorView, prevState: EditorState) => {},
+        update: (view: EditorView, prevState: EditorState) => { },
         destroy: () => {
           clearTimeout(timeoutId);
         },
@@ -89,14 +89,15 @@ export const LoroSyncPlugin = (props: LoroSyncPluginProps): Plugin => {
   });
 };
 
+// This is called when the plugin's state is associated with an editor view
 function init(view: EditorView) {
   const state = loroSyncPluginKey.getState(view.state) as LoroSyncPluginState;
 
   let docSubscription = state.docSubscription;
   if (docSubscription != null) {
     state.doc.unsubscribe(docSubscription);
   }
-  docSubscription = state.doc.subscribe((event) => update(view, event));
+  docSubscription = state.doc.subscribe((event) => updateNodeOnLoroEvent(view, event));
 
   const innerDoc = state.doc.getMap("doc");
   const mapping: LoroNodeMapping = new Map();
@@ -114,7 +115,7 @@ function init(view: EditorView) {
   view.dispatch(tr);
 }
 
-function update(view: EditorView, event: LoroEventBatch) {
+function updateNodeOnLoroEvent(view: EditorView, event: LoroEventBatch) {
   if (event.local) {
     return;
   }

diff --git a/tests/basic.test.ts b/tests/basic.test.ts
@@ -11,7 +11,7 @@ import {
   createNodeFromLoroObj,
   getLoroMapAttributes,
   getLoroMapChildren,
-  updateDoc,
+  updateLoroOnPmChange,
 } from "../src";
 
 import { schema } from "./schema";
@@ -197,7 +197,7 @@ describe("updateDoc", () => {
     const editorState = createEditorState(schema, examplePmContent.doc);
     const loroDoc = new Loro();
     const mapping: LoroNodeMapping = new Map();
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual(exampleLoroContent);
   });
 
@@ -211,7 +211,7 @@ describe("updateDoc", () => {
     pmContent["content"] = [];
     let editorState = createEditorState(schema, pmContent);
 
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual({
       [ROOT_DOC_KEY]: {
         nodeName: ROOT_DOC_KEY,
@@ -227,7 +227,7 @@ describe("updateDoc", () => {
     });
     editorState = createEditorState(schema, pmContent);
 
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual({
       [ROOT_DOC_KEY]: {
         nodeName: ROOT_DOC_KEY,
@@ -249,7 +249,7 @@ describe("updateDoc", () => {
     });
     editorState = createEditorState(schema, pmContent);
 
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual({
       [ROOT_DOC_KEY]: {
         nodeName: ROOT_DOC_KEY,
@@ -286,7 +286,7 @@ describe("updateDoc", () => {
     });
     editorState = createEditorState(schema, pmContent);
 
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual({
       [ROOT_DOC_KEY]: {
         nodeName: ROOT_DOC_KEY,
@@ -342,7 +342,7 @@ describe("updateDoc", () => {
     });
     editorState = createEditorState(schema, pmContent);
 
-    updateDoc(loroDoc, mapping, editorState, editorState);
+    updateLoroOnPmChange(loroDoc, mapping, editorState, editorState);
     expect(loroDoc.toJson()).toEqual({
       [ROOT_DOC_KEY]: {
         nodeName: ROOT_DOC_KEY,
@@ -394,7 +394,7 @@ describe("createNodeFromLoroObj", () => {
     const _editorState = createEditorState(schema, examplePmContent.doc);
     const loroDoc = new Loro();
     const mapping: LoroNodeMapping = new Map();
-    updateDoc(loroDoc, mapping, _editorState, _editorState);
+    updateLoroOnPmChange(loroDoc, mapping, _editorState, _editorState);
 
     const node = createNodeFromLoroObj(
       schema,