From e0b342585ae1679f661c37149036f12b6c5672b9 Mon Sep 17 00:00:00 2001 From: Eric Maciel Date: Wed, 27 Jan 2021 18:13:23 -0500 Subject: [PATCH] fix: index error and set of root children error --- packages/bridge/src/convert/set.ts | 72 ++++++++++++++++++------------ packages/bridge/src/path/index.ts | 6 ++- packages/client/src/client.spec.ts | 45 ++++++++++++++++++- 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/packages/bridge/src/convert/set.ts b/packages/bridge/src/convert/set.ts index a7172c6..946374c 100644 --- a/packages/bridge/src/convert/set.ts +++ b/packages/bridge/src/convert/set.ts @@ -1,4 +1,5 @@ import * as Automerge from 'automerge' +import { Operation, Node } from 'slate' import { toSlatePath, toJS } from '../utils' import { rootKey } from './constants' @@ -19,44 +20,57 @@ const setDataOp = ( } } +const setChildren = (op: Automerge.Diff, doc: any) => (map: any) => { + const { value } = op + + const ops: Operation[] = [] + + let newValue = map[value] + if (!newValue) { + return ops + } + + // First remove all existing child nodes + for (let i = doc.children.length - 1; i >= 0; i--) { + ops.push({ + type: 'remove_node', + path: [i], + node: doc.children[i] + }) + } + + // Then add all the newly defined nodes + const newChildren: Node[] = newValue + newChildren.forEach((child, index) => { + ops.push({ + type: 'insert_node', + path: [index], + node: child + }) + }) + + return ops +} + const opSet = (op: Automerge.Diff, [map, ops]: any, doc: any) => { const { link, value, path, obj, key } = op try { - // We can ignore any root level cursor updates since those - // will not correspond to any slate operations - if (obj === rootKey && key === 'cursors') { + if (map[obj]) { + map[obj][key as any] = link ? map[value] : value + } + + // ignore all cursor updates since those do not need to translate into + // slate operations + if (path && path.includes('cursors')) { return [map, ops] } // Handle updates received for the root children array - if (obj === rootKey && key === 'children' && map[value]) { - // First remove all existing child nodes - for (let i = doc.children.length - 1; i >= 0; i--) { - ops.push((map: any) => ({ - type: 'remove_node', - path: [i], - node: doc.children[i] - })) - } - - // Then add all the newly defined nodes - const newChildren: Node[] = map[value] - newChildren.forEach((child, index) => { - ops.push((map: any) => ({ - type: 'insert_node', - path: [index], - node: child - })) - }) - - return [map, ops] - } - - if (path && path[0] !== 'cursors') { + if (obj === rootKey && key === 'children') { + ops.push(setChildren(op, doc)) + } else if (path && obj !== rootKey && !path.includes('cursors')) { ops.push(setDataOp(op, doc)) - } else if (map[obj]) { - map[obj][key as any] = link ? map[value] : value } return [map, ops] diff --git a/packages/bridge/src/path/index.ts b/packages/bridge/src/path/index.ts index 2d7bc0f..024e269 100644 --- a/packages/bridge/src/path/index.ts +++ b/packages/bridge/src/path/index.ts @@ -6,7 +6,11 @@ export const isTree = (node: Node): boolean => Boolean(node?.children) export const getTarget = (doc: SyncValue | Element, path: Path) => { const iterate = (current: any, idx: number) => { - if (current === null || !(isTree(current) || current[idx])) { + if (current === null || current === undefined) { + return null + } + + if (!(isTree(current) || current[idx])) { return null } diff --git a/packages/client/src/client.spec.ts b/packages/client/src/client.spec.ts index bb2a300..3924cf8 100644 --- a/packages/client/src/client.spec.ts +++ b/packages/client/src/client.spec.ts @@ -2,8 +2,8 @@ import Automerge from 'automerge' import { createServer } from 'http' import fs from 'fs' import isEqual from 'lodash/isEqual' -import { createEditor, Node, Transforms } from 'slate' -import { SyncDoc, toJS, toSlateOp } from '@hiveteams/collab-bridge' +import { createEditor, Editor, Node, Transforms } from 'slate' +import { createDoc, SyncDoc, toJS, toSlateOp } from '@hiveteams/collab-bridge' import AutomergeCollaboration from '@hiveteams/collab-backend/lib/AutomergeCollaboration' import withIOCollaboration from './withIOCollaboration' import { AutomergeOptions, SocketIOPluginOptions } from './interfaces' @@ -213,6 +213,23 @@ describe('automerge editor client tests', () => { toSlateOp(operations, currentDoc) }) + it('should not throw index error', () => { + // Read from our test json file for the deep tree error + // This allows us to easily reproduce real production errors + // and create test cases that resolve those errors + const rawData = fs.readFileSync( + `${__dirname}/test-json/index-error.json`, + 'utf-8' + ) + const parsedData = JSON.parse(rawData) + const { current, operations } = parsedData + const currentDoc = Automerge.load(current) + + // ensure no errors throw when removing a deep tree node + // that has already been removed + toSlateOp(operations, currentDoc) + }) + it('should update children for a root level children operation', async () => { const editor = await createCollabEditor() @@ -233,6 +250,30 @@ describe('automerge editor client tests', () => { expect(Node.string(editor.children[1])).toEqual('nodes') }) + it('set node for children with missing value should not throw error', () => { + const operations: Automerge.Diff[] = [ + { + action: 'set', + type: 'map', + obj: '00000000-0000-0000-0000-000000000000', + key: 'children', + path: [], + value: '6c7bf8a5-d0e0-4b08-a4a2-32df65b807e5', + link: true, + conflicts: [ + { + actor: '8c5d5ada-3db9-4189-9e04-2e7c101d057d', + value: 'e198d171-a00a-4d5c-a597-c0ff35a7f639', + link: true + } + ] + } + ] + + const slateOps = toSlateOp(operations, createDoc()) + expect(slateOps.length).toEqual(0) + }) + afterAll(() => { collabBackend.destroy() server.close()