13.5.49

Make encodeStateAsUpdate deterministic

package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "yjs",
-  "version": "13.5.45",
+  "version": "13.5.49",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
       "name": "yjs",
-      "version": "13.5.45",
+      "version": "13.5.49",
       "license": "MIT",
       "dependencies": {
         "lib0": "^0.2.49"

package.json

@@ -1,6 +1,6 @@
 {
   "name": "yjs",
-  "version": "13.5.45",
+  "version": "13.5.49",
   "description": "Shared Editing Library",
   "main": "./dist/yjs.cjs",
   "module": "./dist/yjs.mjs",

src/types/YText.js

@@ -363,33 +363,48 @@ const formatText = (transaction, parent, currPos, length, attributes) => {
  * @function
  */
 const cleanupFormattingGap = (transaction, start, curr, startAttributes, currAttributes) => {
-  let end = curr
-  const endAttributes = map.copy(currAttributes)
+  /**
+   * @type {Item|null}
+   */
+  let end = start
+  /**
+   * @type {Map<string,ContentFormat>}
+   */
+  const endFormats = map.create()
   while (end && (!end.countable || end.deleted)) {
     if (!end.deleted && end.content.constructor === ContentFormat) {
-      updateCurrentAttributes(endAttributes, /** @type {ContentFormat} */ (end.content))
+      const cf = /** @type {ContentFormat} */ (end.content)
+      endFormats.set(cf.key, cf)
     }
     end = end.right
   }
   let cleanups = 0
-  let reachedEndOfCurr = false
+  let reachedCurr = false
   while (start !== end) {
     if (curr === start) {
-      reachedEndOfCurr = true
+      reachedCurr = true
     }
     if (!start.deleted) {
       const content = start.content
       switch (content.constructor) {
         case ContentFormat: {
           const { key, value } = /** @type {ContentFormat} */ (content)
-          if ((endAttributes.get(key) || null) !== value || (startAttributes.get(key) || null) === value) {
+          const startAttrValue = startAttributes.get(key) || null
+          if (endFormats.get(key) !== content || startAttrValue === value) {
             // Either this format is overwritten or it is not necessary because the attribute already existed.
             start.delete(transaction)
             cleanups++
-            if (!reachedEndOfCurr && (currAttributes.get(key) || null) === value && (startAttributes.get(key) || null) !== value) {
-              currAttributes.delete(key)
+            if (!reachedCurr && (currAttributes.get(key) || null) === value && startAttrValue !== value) {
+              if (startAttrValue === null) {
+                currAttributes.delete(key)
+              } else {
+                currAttributes.set(key, startAttrValue)
+              }
             }
           }
+          if (!reachedCurr && !start.deleted) {
+            updateCurrentAttributes(currAttributes, /** @type {ContentFormat} */ (content))
+          }
           break
         }
       }

src/types/YXmlFragment.js

@@ -21,6 +21,7 @@ import {
 } from '../internals.js'
 
 import * as error from 'lib0/error'
+import * as array from 'lib0/array'
 
 /**
  * Define the elements to which a set of CSS queries apply.
@@ -237,7 +238,7 @@ export class YXmlFragment extends AbstractType {
   querySelectorAll (query) {
     query = query.toUpperCase()
     // @ts-ignore
-    return Array.from(new YXmlTreeWalker(this, element => element.nodeName && element.nodeName.toUpperCase() === query))
+    return array.from(new YXmlTreeWalker(this, element => element.nodeName && element.nodeName.toUpperCase() === query))
   }
 
   /**
@@ -256,7 +257,8 @@ export class YXmlFragment extends AbstractType {
    * @return {string} The string representation of all children.
    */
   toString () {
-    return typeListMap(this, xml => xml.toString()).join('')
+    // toString can result in many cleanup transactions. We wrap all cleanup transactions here to reduce the work
+    return transact(/** @type {Doc} */ (this.doc), () => typeListMap(this, xml => xml.toString()).join(''))
   }
 
   /**

src/utils/DeleteSet.js

@@ -219,17 +219,21 @@ export const createDeleteSetFromStructStore = ss => {
  */
 export const writeDeleteSet = (encoder, ds) => {
   encoding.writeVarUint(encoder.restEncoder, ds.clients.size)
-  ds.clients.forEach((dsitems, client) => {
-    encoder.resetDsCurVal()
-    encoding.writeVarUint(encoder.restEncoder, client)
-    const len = dsitems.length
-    encoding.writeVarUint(encoder.restEncoder, len)
-    for (let i = 0; i < len; i++) {
-      const item = dsitems[i]
-      encoder.writeDsClock(item.clock)
-      encoder.writeDsLen(item.len)
-    }
-  })
+
+  // Ensure that the delete set is written in a deterministic order
+  array.from(ds.clients.entries())
+    .sort((a, b) => b[0] - a[0])
+    .forEach(([client, dsitems]) => {
+      encoder.resetDsCurVal()
+      encoding.writeVarUint(encoder.restEncoder, client)
+      const len = dsitems.length
+      encoding.writeVarUint(encoder.restEncoder, len)
+      for (let i = 0; i < len; i++) {
+        const item = dsitems[i]
+        encoder.writeDsClock(item.clock)
+        encoder.writeDsLen(item.len)
+      }
+    })
 }
 
 /**

src/utils/Doc.js

@@ -147,7 +147,7 @@ export class Doc extends Observable {
   }
 
   getSubdocGuids () {
-    return new Set(Array.from(this.subdocs).map(doc => doc.guid))
+    return new Set(array.from(this.subdocs).map(doc => doc.guid))
   }
 
   /**

src/utils/Transaction.js

@@ -376,15 +376,21 @@ const cleanupTransactions = (transactionCleanups, i) => {
 /**
  * Implements the functionality of `y.transact(()=>{..})`
  *
+ * @template T
  * @param {Doc} doc
- * @param {function(Transaction):void} f
+ * @param {function(Transaction):T} f
  * @param {any} [origin=true]
+ * @return {T}
  *
  * @function
  */
 export const transact = (doc, f, origin = null, local = true) => {
   const transactionCleanups = doc._transactionCleanups
   let initialCall = false
+  /**
+   * @type {any}
+   */
+  let result = null
   if (doc._transaction === null) {
     initialCall = true
     doc._transaction = new Transaction(doc, origin, local)
@@ -395,7 +401,7 @@ export const transact = (doc, f, origin = null, local = true) => {
     doc.emit('beforeTransaction', [doc._transaction, doc])
   }
   try {
-    f(doc._transaction)
+    result = f(doc._transaction)
   } finally {
     if (initialCall) {
       const finishCleanup = doc._transaction === transactionCleanups[0]
@@ -413,4 +419,5 @@ export const transact = (doc, f, origin = null, local = true) => {
       }
     }
   }
+  return result
 }

src/utils/encoding.js

@@ -45,6 +45,7 @@ import * as decoding from 'lib0/decoding'
 import * as binary from 'lib0/binary'
 import * as map from 'lib0/map'
 import * as math from 'lib0/math'
+import * as array from 'lib0/array'
 
 /**
  * @param {UpdateEncoderV1 | UpdateEncoderV2} encoder
@@ -96,7 +97,7 @@ export const writeClientsStructs = (encoder, store, _sm) => {
   encoding.writeVarUint(encoder.restEncoder, sm.size)
   // Write items with higher client ids first
   // This heavily improves the conflict algorithm.
-  Array.from(sm.entries()).sort((a, b) => b[0] - a[0]).forEach(([client, clock]) => {
+  array.from(sm.entries()).sort((a, b) => b[0] - a[0]).forEach(([client, clock]) => {
     // @ts-ignore
     writeStructs(encoder, store.clients.get(client), client, clock)
   })
@@ -231,7 +232,7 @@ const integrateStructs = (transaction, store, clientsStructRefs) => {
    */
   const stack = []
   // sort them so that we take the higher id first, in case of conflicts the lower id will probably not conflict with the id from the higher user.
-  let clientsStructRefsIds = Array.from(clientsStructRefs.keys()).sort((a, b) => a - b)
+  let clientsStructRefsIds = array.from(clientsStructRefs.keys()).sort((a, b) => a - b)
   if (clientsStructRefsIds.length === 0) {
     return null
   }
@@ -601,7 +602,7 @@ export const decodeStateVector = decodedState => readStateVector(new DSDecoderV1
  */
 export const writeStateVector = (encoder, sv) => {
   encoding.writeVarUint(encoder.restEncoder, sv.size)
-  Array.from(sv.entries()).sort((a, b) => b[0] - a[0]).forEach(([client, clock]) => {
+  array.from(sv.entries()).sort((a, b) => b[0] - a[0]).forEach(([client, clock]) => {
     encoding.writeVarUint(encoder.restEncoder, client) // @todo use a special client decoder that is based on mapping
     encoding.writeVarUint(encoder.restEncoder, clock)
   })

tests/doc.tests.js

@@ -2,6 +2,24 @@
 import * as Y from '../src/index.js'
 import * as t from 'lib0/testing'
 
+/**
+ * @param {t.TestCase} _tc
+ */
+export const testAfterTransactionRecursion = _tc => {
+  const ydoc = new Y.Doc()
+  const yxml = ydoc.getXmlFragment('')
+  ydoc.on('afterTransaction', tr => {
+    if (tr.origin === 'test') {
+      yxml.toJSON()
+    }
+  })
+  ydoc.transact(_tr => {
+    for (let i = 0; i < 15000; i++) {
+      yxml.push([new Y.XmlText('a')])
+    }
+  }, 'test')
+}
+
 /**
  * @param {t.TestCase} _tc
  */