From 34a1c2953edfc404cdf98d63c2bc800ced8d6041 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Wed, 3 Apr 2024 23:26:07 +0800 Subject: [PATCH] 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 * fix: based on suggestions from comments and add a few more notes --------- Co-authored-by: Thiago Bellini Ribeiro --- package.json | 1 + pnpm-lock.yaml | 4 +++- src/lib.ts | 53 ++++++++++++++++++++++++++++++++------------- src/sync-plugin.ts | 25 +++++++++++---------- tests/basic.test.ts | 16 +++++++------- 5 files changed, 63 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index cc228f0..aa4a996 100644 --- 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 index f187f7d..a8ffd94 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -5,6 +5,9 @@ settings: excludeLinksFromLockfile: false dependencies: + fast-deep-equal: + specifier: ^3.1.3 + version: 3.1.3 lib0: specifier: ^0.2.42 version: 0.2.93 @@ -1021,7 +1024,6 @@ packages: /fast-deep-equal@3.1.3: resolution: {integrity: sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==} - dev: true /fast-glob@3.3.2: resolution: {integrity: sha512-oX2ruAFQwf/Orj8m737Y5adxDQO0LAB7/S5MnxCdTNDd4p6BsyIVsv9JQsATbTSq8KHRpLwIHbVlUNatxd+1Ow==} diff --git a/src/lib.ts b/src/lib.ts index 7a07c7a..48f75cb 100644 --- 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; 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 index 2191992..34afbfd 100644 --- 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("loro-sync"); type PluginTransactionType = | { - type: "doc-changed"; - } + type: "doc-changed"; + } | { - type: "update-state"; - state: Partial; - }; + type: "update-state"; + state: Partial; + }; 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,6 +89,7 @@ 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; @@ -96,7 +97,7 @@ function init(view: EditorView) { 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 index 1d7d33b..b9ca5dd 100644 --- 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,