Pushing or removing from array causes mobx computed to fire changes = number of array elements + 1

[BradHarris]

Hi,

So far I am loving this library but recently noticed an issue when attempting to sync data between 2 or more clients. Using mobx and adding or removing from an array causes related computed properties to invalidate for every element in the array. I have written some tests and put them in a branch.

Interestingly, the yjs-reactive-bindings don't seem to be at fault. If I don't use SyncedStore with mobx and only use observeYJS from the yjs-reactive-bindings package then the computed properties fire the expected number of times (once per push/remove from the array).

here is the code to reproduce this isssue: https://github.com/YousefED/SyncedStore/compare/main...BradHarris:SyncedStore:bradharris/issue-122?expand=1

I will also inline the code here if that is more convenient:

import { syncedStore, enableMobxBindings } from '@syncedstore/core';
import { observeYJS } from '@syncedstore/yjs-reactive-bindings'
import * as mobx from 'mobx';
import * as Y from 'yjs';

enableMobxBindings(mobx);

type YjsThing = {
  id: string;
};

const createSyncedStore = (doc: Y.Doc) => syncedStore({ things: [] as YjsThing[] }, doc);

class ThingSyncStore {
  doc: Y.Doc;
  store: ReturnType<typeof createSyncedStore>;
  constructor(doc: Y.Doc) {
    this.doc = doc;
    this.store = createSyncedStore(doc);

    mobx.makeAutoObservable(this, {
      doc: false,
      store: false,
    });
  }

  get thingsMap() {
    return this.store.things.reduce((acc, thing) => {
      acc[thing.id] = thing;
      return acc;
    }, {} as Record<string, YjsThing>);
  }

  addPoint(id: string) {
    this.store.things.push({
      id,
    });

    return id;
  }
}

describe('yjs syncedstore with mobx binding', () => {
  it('emits one change for each push to an array', async () => {
    const doc = new Y.Doc();
    const store = new ThingSyncStore(doc);

    const autorunSpy = jest.fn();
    mobx.autorun(() => {
      console.log(Object.keys(store.thingsMap).length);
      autorunSpy();
    });

    // when working on the store locally, we expect the autorun to fire once
    // for the things array being updated
    autorunSpy.mockClear();
    store.addPoint('1');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    autorunSpy.mockClear();
    store.addPoint('2');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    autorunSpy.mockClear();
    store.addPoint('3');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    // create a new doc which we will pretend is another client we wish to sync
    const doc2 = new Y.Doc();
    Y.applyUpdate(doc2, Y.encodeStateAsUpdateV2(doc));
    // apply updates from doc2 to doc
    doc2.on('update', (update) => {
      Y.applyUpdate(doc, update);
    });
    const things = doc2.getArray('things');
    autorunSpy.mockClear();

    // This is where the problem is, the autorun is firing once for the change
    // to the things array, but also once for each element in the array. If you
    // add 10 things, the autorun will fire 11 times. This becomes a huge problem
    // when you want to have multiplayer with a large number of items in the array.
    const thing = new Y.Map();
    thing.set('id', '4');
    things.push([thing]);
    expect(autorunSpy).toHaveBeenCalledTimes(1);
  });
});

class ThingYjsStore {
  doc: Y.Doc;

  constructor(doc: Y.Doc) {
    observeYJS(doc);
    this.doc = doc;
  }

  get things() {
    return this.doc.getArray<Y.Map<any>>('things');
  }

  get thingsMap() {
    const thingMap: Record<string, Y.Map<any>> = {};
    this.things.forEach((thing: Y.Map<any>) => {
      const id = thing.get('id');
      thingMap[id] = thing;
    });

    return thingMap;
  }

  addPoint(id: string) {
    const thing = new Y.Map();
    thing.set('id', id);
    this.things.push([thing]);

    return id;
  }
}

// when use only use the yjs reactive bindings then these tests pass
describe('yjs syncedstore without mobx binding', () => {

  it('emits one change for each push to an array', async () => {
    const doc = new Y.Doc();
    const store = new ThingYjsStore(doc);

    const autorunSpy = jest.fn();
    mobx.autorun(() => {
      console.log(Object.keys(store.thingsMap).length);
      autorunSpy();
    });

    // when working on the store locally, we expect the autorun to fire once
    // for the things array being updated
    autorunSpy.mockClear();
    store.addPoint('1');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    autorunSpy.mockClear();
    store.addPoint('2');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    autorunSpy.mockClear();
    store.addPoint('3');
    expect(autorunSpy).toHaveBeenCalledTimes(1);

    // create a new doc which we will pretend is another client we wish to sync
    const doc2 = new Y.Doc();
    Y.applyUpdate(doc2, Y.encodeStateAsUpdateV2(doc));
    // apply updates from doc2 to doc
    doc2.on('update', (update) => {
      Y.applyUpdate(doc, update);
    });
    const things = doc2.getArray('things');
    autorunSpy.mockClear();

    const thing = new Y.Map();
    thing.set('id', '4');
    things.push([thing]);
    expect(autorunSpy).toHaveBeenCalledTimes(1);
  });
});