imp(batchedOp): improvements

Author: e11sy

packages/collaboration-manager/src/BatchedOperation.ts

@@ -1,16 +1,11 @@
-import { InvertedOperationType, Operation, OperationType, SerializedOperation } from './Operation.js';
-
-/**
- * Batch debounce time
- */
-const DEBOUNCE_TIMEOUT = 500;
+import { InvertedOperationType, Operation, OperationType, type SerializedOperation } from './Operation.js';
 
 /**
  * Class to batch Text operations (maybe others in the future) for Undo/Redo purposes
  *
  * Operations are batched on timeout basis or if batch is terminated from the outside
  */
-export class BatchedOperation<T extends OperationType> extends Operation<T> {
+export class BatchedOperation<T extends OperationType = OperationType> extends Operation<T> {
   /**
    * Array of operations to batch
    */
@@ -29,16 +24,6 @@ export class BatchedOperation<T extends OperationType> extends Operation<T> {
     }
   }
 
-  /**
-   * Adds an operation to the batch
-   * Make sure, that operation could be added to the batch
-   *
-   * @param op - operation to add
-   */
-  public add(op: Operation<T> | Operation<OperationType.Neutral>): void {
-    this.operations.push(op);
-  }
-
   /**
    * Create a new operation batch from an array of operations
    * 
@@ -63,7 +48,7 @@ export class BatchedOperation<T extends OperationType> extends Operation<T> {
       /**
        * Every batch should have at least one operation
        */
-      const batch = new BatchedOperation(opBatchOrJSON.operations.shift()!);
+      const batch = new BatchedOperation(Operation.from(opBatchOrJSON.operations.shift()!));
 
       opBatchOrJSON.operations.forEach((op) => {
         /**
@@ -80,6 +65,16 @@ export class BatchedOperation<T extends OperationType> extends Operation<T> {
     }
   }
 
+  /**
+   * Adds an operation to the batch
+   * Make sure, that operation could be added to the batch
+   *
+   * @param op - operation to add
+   */
+  public add(op: Operation<T> | Operation<OperationType.Neutral>): void {
+    this.operations.push(op);
+  }
+
   /**
    * Method that inverses all of the operations in the batch
    *
@@ -91,11 +86,7 @@ export class BatchedOperation<T extends OperationType> extends Operation<T> {
      */
     const newBatchedOperation = new BatchedOperation<InvertedOperationType<T> | OperationType.Neutral>(this.operations.pop()!.inverse())
 
-    while (this.operations.length > 0) {
-      const op = this.operations.pop()!.inverse();
-
-      newBatchedOperation.add(op);
-    }
+    this.operations.toReversed().map(op => newBatchedOperation.add(op.inverse()));
 
     return newBatchedOperation as BatchedOperation<InvertedOperationType<T>>;
   }
@@ -110,15 +101,8 @@ export class BatchedOperation<T extends OperationType> extends Operation<T> {
     const transformedOp = this.operations.shift()!.transform(againstOp);
 
     const newBatchedOperation = new BatchedOperation(transformedOp);
-  
-    /**
-     * We either have a new operations batch or all operations were not transformable
-     */
-    for (const op of this.operations) {
-      const transformedOp = op.transform(againstOp);
 
-      newBatchedOperation.add(transformedOp);
-    }
+    this.operations.map(op => newBatchedOperation.add(op.transform(againstOp)));
 
     return newBatchedOperation;
   }

packages/collaboration-manager/src/CollaborationManager.ts

@@ -91,11 +91,7 @@ export class CollaborationManager {
    * Undo last operation in the local stack
    */
   public undo(): void {
-    if (this.#currentBatch !== null) {
-      this.#undoRedoManager.put(this.#currentBatch);
-
-      this.#currentBatch = null;
-    }
+    this.#emptyBatch();
 
     const operation = this.#undoRedoManager.undo();
 
@@ -106,13 +102,7 @@ export class CollaborationManager {
     // Disable  handling
     this.#shouldHandleEvents = false;
 
-    if (operation instanceof BatchedOperation) {
-      operation.operations.forEach((op) => {
-        this.applyOperation(op);
-      });
-    } else {
-      this.applyOperation(operation);
-    }
+    this.applyOperation(operation);
 
     // Re-enable event handling
     this.#shouldHandleEvents = true;
@@ -122,11 +112,7 @@ export class CollaborationManager {
    * Redo last undone operation in the local stack
    */
   public redo(): void {
-    if (this.#currentBatch !== null) {
-      this.#undoRedoManager.put(this.#currentBatch);
-
-      this.#currentBatch = null;
-    }
+    this.#emptyBatch();
 
     const operation = this.#undoRedoManager.redo();
 
@@ -154,7 +140,16 @@ export class CollaborationManager {
    *
    * @param operation - operation to apply
    */
-  public applyOperation(operation: Operation): void {
+  public applyOperation(operation: Operation | BatchedOperation): void {
+    /**
+     * If operation is a batcher operation, apply all operations in the batch
+     */
+    if (operation instanceof BatchedOperation) {
+      operation.operations.forEach(op => this.applyOperation(op));
+
+      return;
+    }
+
     if (operation.type === OperationType.Neutral) {
       return;
     }
@@ -244,7 +239,7 @@ export class CollaborationManager {
 
       /**
        * If we got a new remote operation - transform current batch
-       * If batch is not transormable - clear it
+       * If batch is empty leave it as it is
        */
       this.#currentBatch = this.#currentBatch?.transform(operation) ?? null;
 
@@ -266,6 +261,7 @@ export class CollaborationManager {
 
     /**
      * If current operation could not be added to the batch, then terminate current batch and create a new one with current operation
+     * @todo - add debounce timeout 500ms
      */
     if (!this.#currentBatch.canAdd(operation)) {
       this.#undoRedoManager.put(this.#currentBatch);
@@ -277,4 +273,15 @@ export class CollaborationManager {
 
     this.#currentBatch.add(operation);
   }
+
+  /**
+   * Puts current batch to the undo stack and clears the batch
+   */
+  #emptyBatch(): void {
+    if (this.#currentBatch !== null) {
+      this.#undoRedoManager.put(this.#currentBatch);
+
+      this.#currentBatch = null;
+    }
+  }
 }

packages/collaboration-manager/src/OperationsTransformer.ts

@@ -155,10 +155,20 @@ export class OperationsTransformer {
    * @returns New operation
    */
   #transformAgainstTextOperation<T extends OperationType>(operation: Operation<T>, againstOp: Operation<OperationType>): Operation<T> | Operation<OperationType.Neutral> {
+    const index = operation.index;
+    const againstIndex = againstOp.index;
+    
     /**
      * Cover case 1
      */
-    if (operation.index.isBlockIndex) {
+    if (index.isBlockIndex) {
+      return Operation.from(operation);
+    }
+
+    /**
+     * Check that againstOp affects current operation
+     */
+    if (index.dataKey === againstIndex.dataKey && index.blockIndex === againstIndex.blockIndex && againstIndex.textRange![0] > index.textRange![1]) {
       return Operation.from(operation);
     }