From 79045c944bd23bded33988dd8bee895f2de65e67 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Wed, 3 Apr 2024 19:10:35 +0800 Subject: [PATCH] docs: add some notes and a few refactors --- src/lib.ts | 34 +++++++++++++++++++++++----------- src/sync-plugin.ts | 25 +++++++++++++------------ tests/basic.test.ts | 16 ++++++++-------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/lib.ts b/src/lib.ts index 7a07c7a..23611c4 100644 --- a/src/lib.ts +++ b/src/lib.ts @@ -27,7 +27,7 @@ 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, @@ -209,10 +209,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 +229,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 +330,7 @@ function computeChildEqualityFactor( } else if ( leftLoro == null || leftNode == null || - !eqLoroObjNode(leftLoro, leftNode, mapping) + !eqLoroObjNode(leftLoro, leftNode) ) { break; } @@ -346,7 +350,7 @@ function computeChildEqualityFactor( } else if ( rightLoro == null || rightNode == null || - !eqLoroObjNode(rightLoro, rightNode, mapping) + !eqLoroObjNode(rightLoro, rightNode) ) { break; } @@ -418,14 +422,14 @@ export function updateLoroMapAttributes( 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? + // + // FIXME: Should we use a deep equal here? If the value is an obj, it's always going to be a new object. + // Or maybe we should getting the old value from mapping? if (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); } @@ -434,6 +438,8 @@ export function updateLoroMapAttributes( for (const key of keys) { attrs.delete(key); } + + // FIXME: should we update the mapping of attr here? } export function getLoroMapChildren(obj: LoroMap): LoroList { @@ -469,8 +475,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 +500,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 +566,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); @@ -577,6 +586,7 @@ export function updateLoroMapChildren( ) { // Only delete the content of the LoroText to retain remote changes on the same LoroText object const loroText = loroChildren.get(0) as LoroText; + // NOTE: why do we need to delete it in the mapping? mapping.delete(loroText.id); loroText.delete(0, loroText.length); } else if (loroDelLength > 0) { @@ -595,6 +605,8 @@ export function updateLoroMapChildren( createLoroChild(loroChildren, left + i, nodeChild, mapping), ); } + + // FIXME: should we update the mapping of children here? } export function clearChangedNodes( diff --git a/src/sync-plugin.ts b/src/sync-plugin.ts index 2191992..1d48c96 100644 --- a/src/sync-plugin.ts +++ b/src/sync-plugin.ts @@ -12,19 +12,19 @@ import { LoroNodeMapping, clearChangedNodes, createNodeFromLoroObj, - updateDoc, + updateLoroOnPmChange as 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,