moved linkedBy reference out of Item into StructStore

src/structs/ContentType.js

@@ -9,7 +9,8 @@ import {
   readYXmlHook,
   readYXmlText,
   UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, StructStore, Transaction, Item, YEvent, AbstractType, // eslint-disable-line
-  readYWeakLink
+  readYWeakLink,
+  unlinkFrom
 } from '../internals.js'
 
 import * as error from 'lib0/error'
@@ -114,8 +115,8 @@ export class ContentType {
       const type = /** @type {WeakLink<any>} */ (this.type);
       if (type._linkedItem !== null && !type._linkedItem.deleted) {
         const item = /** @type {Item} */ (type._linkedItem)
-        if (item.linkedBy !== null) {
-          item.linkedBy.delete(type)
+        if (item.linked) {
+          unlinkFrom(transaction, item, type)
         }
         type._linkedItem = null
       }

src/structs/Item.js

@@ -296,13 +296,6 @@ export class Item extends AbstractStruct {
      * @type {ID | null}
      */
     this.redone = null
-    /**
-     * If this item was referenced by other weak links, here we keep the references
-     * to these weak refs.
-     * 
-     * @type {Set<YWeakLink<any>> | null}
-     */
-    this.linkedBy = null
     /**
      * @type {AbstractContent}
      */
@@ -312,11 +305,28 @@ export class Item extends AbstractStruct {
      * bit2: countable
      * bit3: deleted
      * bit4: mark - mark node as fast-search-marker
+     * bit5: linked - this item is linked by Weak Link references
      * @type {number} byte
      */
     this.info = this.content.isCountable() ? binary.BIT2 : 0
   }
 
+  /**
+   * This is used to mark the item as linked by weak link references.
+   * Reference dependencies are being kept in StructStore.
+   *
+   * @type {boolean}
+   */
+  set linked(isLinked) {
+    if (((this.info & binary.BIT9) > 0) !== isLinked) {
+      this.info ^= binary.BIT9
+    }
+  }
+
+  get linked() {
+    return (this.info & binary.BIT9) > 0
+  }
+
   /**
    * This is used to mark the item as an indexed fast-search marker
    *
@@ -524,8 +534,16 @@ export class Item extends AbstractStruct {
         // set as current parent value if right === null and this is parentSub
         /** @type {AbstractType<any>} */ (this.parent)._map.set(this.parentSub, this)
         if (this.left !== null) {
-          // inherit links from block we're overriding
-          this.linkedBy = this.left.linkedBy
+          // move links from block we're overriding
+          this.linked = this.left.linked
+          this.left.linked = false
+          const allLinks = transaction.doc.store.linkedBy
+          const links = allLinks.get(this.left)
+          if (links !== undefined) {
+            allLinks.set(this, links)
+            // since left is being deleted, it will remove
+            // its links from store.linkedBy anyway
+          }
           // this is the current attribute value of parent. delete right
           this.left.delete(transaction)
         }
@@ -538,9 +556,13 @@ export class Item extends AbstractStruct {
       this.content.integrate(transaction, this)
       // add parent to transaction.changed
       addChangedTypeToTransaction(transaction, /** @type {AbstractType<any>} */ (this.parent), this.parentSub)
-      if (this.linkedBy !== null) {
-        for (let link of this.linkedBy) {
-          addChangedTypeToTransaction(transaction, link, this.parentSub)
+      if (this.linked) {
+        // notify links about changes
+        const linkedBy = transaction.doc.store.linkedBy.get(this)
+        if (linkedBy !== undefined) {
+          for (let link of linkedBy) {
+            addChangedTypeToTransaction(transaction, link, this.parentSub)
+          }
         }
       }
       if ((/** @type {AbstractType<any>} */ (this.parent)._item !== null && /** @type {AbstractType<any>} */ (this.parent)._item.deleted) || (this.parentSub !== null && this.right !== null)) {
@@ -600,8 +622,7 @@ export class Item extends AbstractStruct {
       this.deleted === right.deleted &&
       this.redone === null &&
       right.redone === null &&
-      this.linkedBy === null &&
-      right.linkedBy === null &&
+      !this.linked && !right.linked && // linked items cannot be merged
       this.content.constructor === right.content.constructor &&
       this.content.mergeWith(right.content)
     ) {
@@ -647,11 +668,18 @@ export class Item extends AbstractStruct {
       addToDeleteSet(transaction.deleteSet, this.id.client, this.id.clock, this.length)
       addChangedTypeToTransaction(transaction, parent, this.parentSub)
       this.content.delete(transaction)
-      if (this.linkedBy !== null) {
-        for (let link of this.linkedBy) {
-          addChangedTypeToTransaction(transaction, link, this.parentSub)
+
+      if (this.linked) {
+        // notify links that current element has been removed
+        const allLinks = transaction.doc.store.linkedBy
+        const linkedBy = allLinks.get(this)
+        if (linkedBy !== undefined) {
+          for (let link of linkedBy) {
+            addChangedTypeToTransaction(transaction, link, this.parentSub)
+          }
+          allLinks.delete(this)
         }
-        this.linkedBy = null
+        this.linked = false
       }
     }
   }

src/types/AbstractType.js

@@ -243,13 +243,16 @@ export const callTypeObservers = (type, transaction, event, visitedLinks = null)
     map.setIfUndefined(changedParentTypes, type, () => []).push(event)
     if (type._item === null) {
       break
-    } else if (type._item.linkedBy !== null) {
-      for (let link of type._item.linkedBy) {
-        if (visitedLinks === null || !visitedLinks.has(link)) {
-          visitedLinks = visitedLinks !== null ? visitedLinks : new Set()
-          visitedLinks.add(link)
-          // recursive call
-          callTypeObservers(link, transaction, /** @type {any} */ (event), visitedLinks)
+    } else if (type._item.linked) {
+      const linkedBy = transaction.doc.store.linkedBy.get(type._item)
+      if (linkedBy !== undefined) {
+        for (let link of linkedBy) {
+          if (visitedLinks === null || !visitedLinks.has(link)) {
+            visitedLinks = visitedLinks !== null ? visitedLinks : new Set()
+            visitedLinks.add(link)
+            // recursive call
+            callTypeObservers(link, transaction, /** @type {any} */ (event), visitedLinks)
+          }
         }
       }
     }

src/types/YWeakLink.js

@@ -1,4 +1,6 @@
 import { decoding, encoding, error } from "lib0"
+import * as map from 'lib0/map'
+import * as set from 'lib0/set'
 import { 
   YEvent, Transaction, ID, GC, AbstractType, UpdateDecoderV1, UpdateDecoderV2, UpdateEncoderV1, UpdateEncoderV2, Doc, Item,
   transact,
@@ -92,11 +94,7 @@ export class YWeakLink extends AbstractType {
         }
         this._linkedItem = sourceItem
         if (!sourceItem.deleted) {
-          const src = /** @type {Item} */ (sourceItem)
-          if (src.linkedBy === null) {
-            src.linkedBy = new Set()
-          }
-          src.linkedBy.add(this)
+          createLink(transaction, /** @type {Item} */ (sourceItem), this)
         }
       })
     }
@@ -171,10 +169,12 @@ export const arrayWeakLink = (transaction, parent, index) => {
             item = getItemCleanEnd(transaction, transaction.doc.store, createID(item.id.client, item.id.clock))
         }
         const link = new YWeakLink(item.id, item)
-        if (item.linkedBy === null) {
-          item.linkedBy = new Set()
+        if (parent.doc !== null) {
+          const source = /** @type {Item} */ (item)
+          transact(parent.doc, (transaction) => {
+            createLink(transaction, source, link)
+          })
         }
-        item.linkedBy.add(link)
         return link
       }
       index -= item.length
@@ -195,12 +195,51 @@ export const mapWeakLink = (parent, key) => {
   const item = parent._map.get(key)
   if (item !== undefined) {
     const link = new YWeakLink(item.id, item)
-    if (item.linkedBy === null) {
-      item.linkedBy = new Set()
+    if (parent.doc !== null) {
+      transact(parent.doc, (transaction) => {
+        createLink(transaction, item, link)
+      })
     }
-    item.linkedBy.add(link)
     return link
   } else {
     return undefined
   }
+}
+
+/**
+ * Establishes a link between source and weak link reference.
+ * It assumes that source has already been split if necessary.
+ * 
+ * @param {Transaction} transaction 
+ * @param {Item} source
+ * @param {YWeakLink<any>} linkRef
+ */
+export const createLink = (transaction, source, linkRef) => {
+  const allLinks = transaction.doc.store.linkedBy
+  map.setIfUndefined(allLinks, source, set.create).add(linkRef)
+  source.linked = true
+}
+
+/**
+ * Deletes the link between source and a weak link reference.
+ * 
+ * @param {Transaction} transaction 
+ * @param {Item} source
+ * @param {YWeakLink<any>} linkRef
+ */
+export const unlinkFrom = (transaction, source, linkRef) => {
+  const allLinks = transaction.doc.store.linkedBy
+  const linkedBy = allLinks.get(source)
+  if (linkedBy !== undefined) {
+    linkedBy.delete(linkRef)
+    if (linkedBy.size === 0) {
+      allLinks.delete(source)
+      source.linked = false
+      if (source.countable) {
+        // since linked property is blocking items from merging,
+        // it may turn out that source item can be merged now
+        transaction._mergeStructs.push(source)
+      }
+    }
+  }
 }

src/utils/StructStore.js

@@ -2,7 +2,8 @@
 import {
   GC,
   splitItem,
-  Transaction, ID, Item, DSDecoderV2 // eslint-disable-line
+  Transaction, ID, Item, DSDecoderV2, // eslint-disable-line
+  YWeakLink
 } from '../internals.js'
 
 import * as math from 'lib0/math'
@@ -14,6 +15,13 @@ export class StructStore {
      * @type {Map<number,Array<GC|Item>>}
      */
     this.clients = new Map()
+    /**
+     * If this item was referenced by other weak links, here we keep the references
+     * to these weak refs.
+     * 
+     * @type {Map<Item, Set<YWeakLink<any>>>}
+     */
+    this.linkedBy = new Map()
     /**
      * @type {null | { missing: Map<number, number>, update: Uint8Array }}
      */

tests/index.js

@@ -21,8 +21,7 @@ if (isBrowser) {
   log.createVConsole(document.body)
 }
 runTests({
-  //doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, 
-  weakLinks
+  doc, map, array, text, xml, encoding, undoredo, compatibility, snapshot, updates, relativePositions, weakLinks
 }).then(success => {
   /* istanbul ignore next */
   if (isNode) {

tests/y-weak-links.tests.js

@@ -488,9 +488,7 @@ export const testRemoteMapUpdate = tc => {
   map0.set('key', 3)
 
   // apply updated content first, link second
-  console.log('update U0 -> U2')
   Y.applyUpdate(users[2], Y.encodeStateAsUpdate(users[0])) 
-  console.log('update U1 -> U2')
   Y.applyUpdate(users[2], Y.encodeStateAsUpdate(users[1]))
 
   // make sure that link can find the most recent block