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 <[email protected]>
yorkie-team · Jun 8, 2023 · 8151e8a · 8151e8a
1 parent d38c853
commit 8151e8a
Show file tree

Hide file tree

Showing 10 changed files with 98 additions and 143 deletions.
diff --git 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
@@ -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
@@ -161,19 +161,17 @@ <h2>yorkie document</h2>
           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 @@ <h2>yorkie document</h2>
           // 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
@@ -235,30 +235,28 @@ <h2>Yorkie LinkedList</h2>
             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
@@ -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
@@ -149,7 +149,7 @@ export interface LocalChangeEvent extends BaseDocEvent {
   /**
    * LocalChangeEvent type
    */
-  value: Array<ChangeInfo>;
+  value: ChangeInfo;
 }
 
 /**
@@ -166,7 +166,7 @@ export interface RemoteChangeEvent extends BaseDocEvent {
   /**
    * RemoteChangeEvent type
    */
-  value: Array<ChangeInfo>;
+  value: ChangeInfo;
 }
 
 /**
@@ -258,15 +258,13 @@ export class Document<T> {
       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<T> {
             return;
           }
 
-          const changeInfos: Array<ChangeInfo> = [];
-          for (const { message, operations, actor } of event.value) {
-            const targetOps: Array<OperationInfo> = [];
-            for (const op of operations) {
-              if (this.isSameElementOrChildOf(op.path, target)) {
-                targetOps.push(op);
-              }
+          const { message, operations, actor } = event.value;
+          const targetOps: Array<OperationInfo> = [];
+          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<T> {
       // 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
@@ -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
@@ -123,9 +123,8 @@ describe('Document', function () {
     let expectedEvents2: Array<OperationInfo> = [];
     const pushEvent = (event: DocEvent, events: Array<OperationInfo>) => {
       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<OperationInfo> = [];
     const pushEvent = (event: DocEvent, events: Array<OperationInfo>) => {
       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<OperationInfo> = [];
     const pushEvent = (event: DocEvent, events: Array<OperationInfo>) => {
       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
@@ -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);
         }
       });