From 97aa3fc67293736a9e24d79e07a462603ff02e0e Mon Sep 17 00:00:00 2001 From: Saul-Mirone Date: Fri, 9 May 2025 06:19:57 +0000 Subject: [PATCH] fix(editor): array proxy splice will cause too large yjs update (#12201) Splice will produce large yjs updates for splice because it will call insert and delete operation for every item in yarray. ## Summary by CodeRabbit - **New Features** - Added support for `splice`, `shift`, and `unshift` methods on reactive arrays, enabling enhanced dynamic array operations with synchronization. - **Tests** - Expanded test coverage for array operations including `push`, `splice`, `shift`, and `unshift`, verifying complete array state after each change. --- .../store/src/__tests__/yjs.unit.spec.ts | 57 ++++++++++++++--- .../framework/store/src/reactive/proxy.ts | 63 +++++++++++++++++++ 2 files changed, 112 insertions(+), 8 deletions(-) diff --git a/blocksuite/framework/store/src/__tests__/yjs.unit.spec.ts b/blocksuite/framework/store/src/__tests__/yjs.unit.spec.ts index db6d9b46d9..54faa44247 100644 --- a/blocksuite/framework/store/src/__tests__/yjs.unit.spec.ts +++ b/blocksuite/framework/store/src/__tests__/yjs.unit.spec.ts @@ -7,23 +7,64 @@ import type { Text } from '../reactive/index.js'; import { Boxed, createYProxy, popProp, stashProp } from '../reactive/index.js'; describe('array', () => { - test('proxy', () => { + test('push and splice', () => { const ydoc = new Y.Doc(); const arr = ydoc.getArray('arr'); - arr.push([0]); + arr.push([0, 1, 2, 3, 4, 5]); const proxy = createYProxy(arr) as unknown[]; - expect(arr.get(0)).toBe(0); + expect(arr.toJSON()).toEqual([0, 1, 2, 3, 4, 5]); + expect(proxy).toEqual([0, 1, 2, 3, 4, 5]); - proxy.push(1); - expect(arr.get(1)).toBe(1); - expect(arr.length).toBe(2); + proxy.push(6); + expect(arr.toJSON()).toEqual([0, 1, 2, 3, 4, 5, 6]); + expect(proxy).toEqual([0, 1, 2, 3, 4, 5, 6]); proxy.splice(1, 1); - expect(arr.length).toBe(1); + expect(arr.toJSON()).toEqual([0, 2, 3, 4, 5, 6]); + expect(proxy).toEqual([0, 2, 3, 4, 5, 6]); proxy[0] = 2; - expect(arr.length).toBe(1); + expect(arr.toJSON()).toEqual([2, 2, 3, 4, 5, 6]); + expect(proxy).toEqual([2, 2, 3, 4, 5, 6]); + }); + + test('shift and unshift', () => { + const ydoc = new Y.Doc(); + const arr = ydoc.getArray('arr'); + arr.push([0, 1, 2, 3, 4, 5]); + + const proxy = createYProxy(arr) as unknown[]; + expect(arr.toJSON()).toEqual([0, 1, 2, 3, 4, 5]); + + // Test shift + const shifted = proxy.shift(); + expect(shifted).toBe(0); + expect(arr.toJSON()).toEqual([1, 2, 3, 4, 5]); + expect(proxy).toEqual([1, 2, 3, 4, 5]); + + const shifted2 = proxy.shift(); + expect(shifted2).toBe(1); + expect(arr.toJSON()).toEqual([2, 3, 4, 5]); + expect(proxy).toEqual([2, 3, 4, 5]); + + // Test shift on empty array + const emptyArr = ydoc.getArray('empty'); + const emptyProxy = createYProxy(emptyArr) as unknown[]; + const emptyShifted = emptyProxy.shift(); + expect(emptyShifted).toBeUndefined(); + expect(emptyArr.toJSON()).toEqual([]); + expect(emptyProxy).toEqual([]); + + // Test unshift + proxy.unshift(-1, 0, 1); + expect(arr.toJSON()).toEqual([-1, 0, 1, 2, 3, 4, 5]); + expect(proxy).toEqual([-1, 0, 1, 2, 3, 4, 5]); + + // Test unshift with multiple items + proxy.unshift(-3, -2); + expect(arr.toJSON()).toEqual([-3, -2, -1, 0, 1, 2, 3, 4, 5]); + expect(proxy).toEqual([-3, -2, -1, 0, 1, 2, 3, 4, 5]); }); }); diff --git a/blocksuite/framework/store/src/reactive/proxy.ts b/blocksuite/framework/store/src/reactive/proxy.ts index d60188ad28..43fbb3c09b 100644 --- a/blocksuite/framework/store/src/reactive/proxy.ts +++ b/blocksuite/framework/store/src/reactive/proxy.ts @@ -92,6 +92,69 @@ export class ReactiveYArray extends BaseReactiveYData< return Reflect.set(target, p, data, receiver); }, get: (target, p, receiver) => { + if (p === 'splice') { + return (start: number, deleteCount?: number, ...items: unknown[]) => { + const doc = this._ySource.doc; + if (!doc) { + throw new BlockSuiteError( + ErrorCode.ReactiveProxyError, + 'YData is not bound to a Y.Doc' + ); + } + const count = deleteCount ?? target.length - start; + const yItems = items.map(item => native2Y(item)); + this._transact(doc, () => { + this._ySource.delete(start, count); + this._ySource.insert(start, yItems); + }); + + const result = Array.prototype.splice.apply(target, [ + start, + count, + ...yItems.map(yItem => createYProxy(yItem, this._options)), + ]); + + return result; + }; + } + if (p === 'shift') { + return () => { + const doc = this._ySource.doc; + if (!doc) { + throw new BlockSuiteError( + ErrorCode.ReactiveProxyError, + 'YData is not bound to a Y.Doc' + ); + } + if (target.length === 0) { + return undefined; + } + const result = Array.prototype.shift.call(target); + this._transact(doc, () => { + this._ySource.delete(0, 1); + }); + return result; + }; + } + if (p === 'unshift') { + return (...items: unknown[]) => { + const doc = this._ySource.doc; + if (!doc) { + throw new BlockSuiteError( + ErrorCode.ReactiveProxyError, + 'YData is not bound to a Y.Doc' + ); + } + const yItems = items.map(item => native2Y(item)); + this._transact(doc, () => { + this._ySource.insert(0, yItems); + }); + return Array.prototype.unshift.apply( + target, + yItems.map(yItem => createYProxy(yItem, this._options)) + ); + }; + } return Reflect.get(target, p, receiver); }, deleteProperty: (target, p): boolean => {