From 8151e8a259e13885c44a299310f1bdf5ae9a7bc8 Mon Sep 17 00:00:00 2001 From: jinho park Date: Thu, 8 Jun 2023 12:49:47 +0900 Subject: [PATCH] Change the value of XXXChange to Change in Document.subscribe (#538) Previously, we passed ChangeInfos as values of RemoteChange and LocalChange types in Document.subscribe, and users should iterate loop to extract Change. This commit simply passes Change instead of Changes to remove changes loop in Document.subscribe. --------- Co-authored-by: Youngteac Hong --- public/editor.html | 15 +++----- public/index.html | 7 ++-- public/multi.html | 31 +++++++--------- public/prosemirror.html | 42 +++++++++++----------- public/quill.html | 7 ++-- src/document/document.ts | 56 ++++++++++++++--------------- test/integration/client_test.ts | 2 +- test/integration/document_test.ts | 15 ++++---- test/integration/text_test.ts | 41 +++++++-------------- test/unit/document/document_test.ts | 25 ++++++------- 10 files changed, 98 insertions(+), 143 deletions(-) diff --git a/public/editor.html b/public/editor.html index fdc1d6c6a..99257a263 100644 --- a/public/editor.html +++ b/public/editor.html @@ -190,18 +190,13 @@ // 02-2. subscribe document event. doc.subscribe('$.text', (event) => { - const changes = event.value; if (event.type === 'remote-change') { - for (const change of changes) { - const { operations } = change; - handleOperations(operations); - } + const { operations } = event.value; + handleOperations(operations); } else if (event.type === 'local-change') { - for (const change of changes) { - const { message } = change; - if (message === 'simulate') { - renderText(); - } + const { message } = event.value; + if (message === 'simulate') { + renderText(); } } }); diff --git a/public/index.html b/public/index.html index 27fe1c177..76c50bebd 100644 --- a/public/index.html +++ b/public/index.html @@ -162,11 +162,8 @@ doc.subscribe('$.content', (event) => { if (event.type === 'remote-change') { - const changes = event.value; - for (const change of changes) { - const { actor, operations } = change; - handleOperations(operations, actor); - } + const { actor, operations } = event.value; + handleOperations(operations, actor); const textLength = codemirror.getValue().length; if ( diff --git a/public/multi.html b/public/multi.html index 9c6e98c15..41569c758 100644 --- a/public/multi.html +++ b/public/multi.html @@ -161,19 +161,17 @@

yorkie document

doc.subscribe('$.todos', (event) => { console.log('🟡 todos event', event); - for (const changeInfo of event.value) { - const { message, operations } = changeInfo; - for (const op of operations) { - const { type, path, index } = op; - switch (type) { - case 'add': - const value = doc.getValueByPath(`${path}.${index}`); - addTodo(value); - break; - default: - displayTodos(); - break; - } + const { message, operations } = event.value; + for (const op of operations) { + const { type, path, index } = op; + switch (type) { + case 'add': + const value = doc.getValueByPath(`${path}.${index}`); + addTodo(value); + break; + default: + displayTodos(); + break; } } @@ -255,11 +253,8 @@

yorkie document

// 05. Quill example doc.subscribe('$.content', (event) => { if (event.type === 'remote-change') { - const changes = event.value; - for (const change of changes) { - const { actor, message, operations } = change; - handleOperations(operations, actor); - } + const { actor, message, operations } = event.value; + handleOperations(operations, actor); } }); diff --git a/public/prosemirror.html b/public/prosemirror.html index fd15c3fce..0f13d6726 100644 --- a/public/prosemirror.html +++ b/public/prosemirror.html @@ -235,30 +235,28 @@

Yorkie LinkedList

return; } - for (const change of event.value) { - const { message, actor, operations } = change; - for (const op of operations) { - if (op.type !== 'tree-edit') { - continue; - } + const { message, actor, operations } = event.value; + for (const op of operations) { + if (op.type !== 'tree-edit') { + continue; + } - const { from, to, value: content } = op; - const transform = view.state.tr; - if (content) { - transform.replaceWith( - from, - to, - Node.fromJSON(mySchema, { - type: content.type, - text: content.value, - }), - ); - } else { - transform.replace(from, to); - } - const newState = view.state.apply(transform); - view.updateState(newState); + const { from, to, value: content } = op; + const transform = view.state.tr; + if (content) { + transform.replaceWith( + from, + to, + Node.fromJSON(mySchema, { + type: content.type, + text: content.value, + }), + ); + } else { + transform.replace(from, to); } + const newState = view.state.apply(transform); + view.updateState(newState); } }); diff --git a/public/quill.html b/public/quill.html index 4ea1429e5..fa7b9fc30 100644 --- a/public/quill.html +++ b/public/quill.html @@ -106,11 +106,8 @@ doc.subscribe('$.content', (event) => { if (event.type === 'remote-change') { - const changes = event.value; - for (const change of changes) { - const { actor, message, operations } = change; - handleOperations(operations, actor); - } + const { actor, message, operations } = event.value; + handleOperations(operations, actor); } }); diff --git a/src/document/document.ts b/src/document/document.ts index 47a52a20a..8ef78e362 100644 --- a/src/document/document.ts +++ b/src/document/document.ts @@ -149,7 +149,7 @@ export interface LocalChangeEvent extends BaseDocEvent { /** * LocalChangeEvent type */ - value: Array; + value: ChangeInfo; } /** @@ -166,7 +166,7 @@ export interface RemoteChangeEvent extends BaseDocEvent { /** * RemoteChangeEvent type */ - value: Array; + value: ChangeInfo; } /** @@ -258,15 +258,13 @@ export class Document { if (this.eventStreamObserver) { this.eventStreamObserver.next({ type: DocEventType.LocalChange, - value: [ - { - message: change.getMessage() || '', - operations: internalOpInfos.map((internalOpInfo) => - this.toOperationInfo(internalOpInfo), - ), - actor: change.getID().getActorID(), - }, - ], + value: { + message: change.getMessage() || '', + operations: internalOpInfos.map((internalOpInfo) => + this.toOperationInfo(internalOpInfo), + ), + actor: change.getID().getActorID(), + }, }); } @@ -317,25 +315,21 @@ export class Document { return; } - const changeInfos: Array = []; - for (const { message, operations, actor } of event.value) { - const targetOps: Array = []; - for (const op of operations) { - if (this.isSameElementOrChildOf(op.path, target)) { - targetOps.push(op); - } + const { message, operations, actor } = event.value; + const targetOps: Array = []; + for (const op of operations) { + if (this.isSameElementOrChildOf(op.path, target)) { + targetOps.push(op); } - targetOps.length && - changeInfos.push({ - message, - operations: targetOps, - actor, - }); } - changeInfos.length && + targetOps.length && callback({ type: event.type, - value: changeInfos, + value: { + message, + operations: targetOps, + actor, + }, }); }, arg3, @@ -623,10 +617,12 @@ export class Document { // with the Document after RemoteChange event is emitted. If the event // is emitted asynchronously, the model can be changed and breaking // consistency. - this.eventStreamObserver.next({ - type: DocEventType.RemoteChange, - value: changeInfos, - }); + for (const changeInfo of changeInfos) { + this.eventStreamObserver.next({ + type: DocEventType.RemoteChange, + value: changeInfo, + }); + } } if (logger.isEnabled(LogLevel.Debug)) { diff --git a/test/integration/client_test.ts b/test/integration/client_test.ts index 268b85690..b89cb5d02 100644 --- a/test/integration/client_test.ts +++ b/test/integration/client_test.ts @@ -824,7 +824,7 @@ describe('Client', function () { await waitStubCallCount(stub1, 3); // local-change await waitStubCallCount(stub2, 3); // local-change - await waitStubCallCount(stub3, 2); + await waitStubCallCount(stub3, 3); assert.equal(d1.toSortedJSON(), '{"c1":1,"c2":0}'); assert.equal(d2.toSortedJSON(), '{"c1":0,"c2":1}'); assert.equal(d3.toSortedJSON(), '{"c1":1,"c2":1}'); diff --git a/test/integration/document_test.ts b/test/integration/document_test.ts index 11eafb8ae..21241e01b 100644 --- a/test/integration/document_test.ts +++ b/test/integration/document_test.ts @@ -123,9 +123,8 @@ describe('Document', function () { let expectedEvents2: Array = []; const pushEvent = (event: DocEvent, events: Array) => { if (event.type !== DocEventType.RemoteChange) return; - for (const { operations } of event.value) { - events.push(...operations); - } + const { operations } = event.value; + events.push(...operations); }; const stub1 = sinon.stub().callsFake((event) => pushEvent(event, events1)); const stub2 = sinon.stub().callsFake((event) => pushEvent(event, events2)); @@ -270,9 +269,8 @@ describe('Document', function () { let counterEvents: Array = []; const pushEvent = (event: DocEvent, events: Array) => { if (event.type !== DocEventType.RemoteChange) return; - for (const { operations } of event.value) { - events.push(...operations); - } + const { operations } = event.value; + events.push(...operations); }; const stub = sinon.stub().callsFake((event) => pushEvent(event, events)); const stubTodo = sinon @@ -377,9 +375,8 @@ describe('Document', function () { let objEvents: Array = []; const pushEvent = (event: DocEvent, events: Array) => { if (event.type !== DocEventType.RemoteChange) return; - for (const { operations } of event.value) { - events.push(...operations); - } + const { operations } = event.value; + events.push(...operations); }; const stub = sinon.stub().callsFake((event) => pushEvent(event, events)); const stubTodo = sinon diff --git a/test/integration/text_test.ts b/test/integration/text_test.ts index fa469c6fb..f315fa0f5 100644 --- a/test/integration/text_test.ts +++ b/test/integration/text_test.ts @@ -96,11 +96,8 @@ describe('Text', function () { doc.update((root) => (root.text = new Text())); doc.subscribe('$.text', (event) => { if (event.type === 'local-change') { - const changes = event.value; - for (const change of changes) { - const { operations } = change; - view.applyOperations(operations); - } + const { operations } = event.value; + view.applyOperations(operations); } }); @@ -123,11 +120,8 @@ describe('Text', function () { doc.update((root) => (root.text = new Text())); doc.subscribe('$.text', (event) => { if (event.type === 'local-change') { - const changes = event.value; - for (const change of changes) { - const { operations } = change; - view.applyOperations(operations); - } + const { operations } = event.value; + view.applyOperations(operations); } }); @@ -158,11 +152,8 @@ describe('Text', function () { doc.update((root) => (root.text = new Text())); doc.subscribe('$.text', (event) => { if (event.type === 'local-change') { - const changes = event.value; - for (const change of changes) { - const { operations } = change; - view.applyOperations(operations); - } + const { operations } = event.value; + view.applyOperations(operations); } }); @@ -196,14 +187,11 @@ describe('Text', function () { doc.subscribe('$.text', (event) => { if (event.type === 'local-change') { - const changes = event.value; - for (const change of changes) { - const { operations } = change; - - if (operations[0].type === 'select') { - assert.equal(operations[0].from, 2); - assert.equal(operations[0].to, 4); - } + const { operations } = event.value; + + if (operations[0].type === 'select') { + assert.equal(operations[0].from, 2); + assert.equal(operations[0].to, 4); } } }); @@ -351,11 +339,8 @@ describe('Text', function () { const view1 = new TextView(); d1.subscribe('$.k1', (event) => { if (event.type === 'local-change') { - const changes = event.value; - for (const change of changes) { - const { operations } = change; - view1.applyOperations(operations); - } + const { operations } = event.value; + view1.applyOperations(operations); } }); diff --git a/test/unit/document/document_test.ts b/test/unit/document/document_test.ts index 3c0124ad8..af8279f36 100644 --- a/test/unit/document/document_test.ts +++ b/test/unit/document/document_test.ts @@ -947,9 +947,8 @@ describe('Document', function () { const ops: Array = []; const stub1 = sinon.stub().callsFake((event: DocEvent) => { if (event.type !== DocEventType.LocalChange) return; - for (const { operations } of event.value) { - ops.push(...operations); - } + const { operations } = event.value; + ops.push(...operations); }); const unsub1 = doc.subscribe(stub1); @@ -988,9 +987,8 @@ describe('Document', function () { const ops: Array = []; const stub1 = sinon.stub().callsFake((event: DocEvent) => { if (event.type !== DocEventType.LocalChange) return; - for (const { operations } of event.value) { - ops.push(...operations); - } + const { operations } = event.value; + ops.push(...operations); }); const unsub1 = doc.subscribe(stub1); @@ -1029,9 +1027,8 @@ describe('Document', function () { const stub1 = sinon.stub().callsFake((event: DocEvent) => { if (event.type !== DocEventType.LocalChange) return; - for (const { operations } of event.value) { - ops.push(...operations); - } + const { operations } = event.value; + ops.push(...operations); }); const unsub1 = doc.subscribe(stub1); @@ -1080,9 +1077,8 @@ describe('Document', function () { const ops: Array = []; const stub1 = sinon.stub().callsFake((event: DocEvent) => { if (event.type !== DocEventType.LocalChange) return; - for (const { operations } of event.value) { - ops.push(...operations); - } + const { operations } = event.value; + ops.push(...operations); }); const unsub1 = doc.subscribe(stub1); @@ -1131,9 +1127,8 @@ describe('Document', function () { const ops: Array = []; const stub1 = sinon.stub().callsFake((event: DocEvent) => { if (event.type !== DocEventType.LocalChange) return; - for (const { operations } of event.value) { - ops.push(...operations); - } + const { operations } = event.value; + ops.push(...operations); }); const unsub1 = doc.subscribe(stub1);