[FIX] Fix circular reference error when serializing Observer data

eslint.config.mjs

@@ -52,6 +52,11 @@ export default [
     },
     {
         files: ['**/*.test.js', '**/*.test.mjs'],
+        languageOptions: {
+            globals: {
+                ...globals.mocha
+            }
+        },
         rules: {
             'no-unused-expressions': 'off'
         }

package.json

@@ -63,8 +63,8 @@
     "build:umd": "rollup -c --environment target:umd",
     "build:types": "tsc -p tsconfig.build.json",
     "docs": "typedoc",
-    "lint": "eslint src rollup.config.mjs eslint.config.mjs",
-    "lint:fix": "eslint src rollup.config.mjs eslint.config.mjs --fix",
+    "lint": "eslint src test rollup.config.mjs eslint.config.mjs",
+    "lint:fix": "eslint src test rollup.config.mjs eslint.config.mjs --fix",
     "publint": "publint",
     "test": "mocha",
     "type:check": "tsc --noEmit",

src/events.ts

@@ -35,26 +35,26 @@ export type HandleEvent = (arg1?: any, arg2?: any, arg3?: any, arg4?: any, arg5?
  * events.unbind('testEvent');
  */
 class Events {
-    private _suspendEvents: boolean = false;
+    private _additionalEmitters!: Events[];
 
-    private _additionalEmitters: Events[] = [];
+    private _events!: Record<string, HandleEvent[]>;
 
-    private _events: Record<string, HandleEvent[]> = {};
+    private _suspendEvents!: boolean;
 
     /**
      * Creates a new Events instance.
      */
     constructor() {
-        // _world
-        Object.defineProperty(
-            this,
-            '_events', {
-                enumerable: false,
-                configurable: false,
-                writable: true,
-                value: { }
-            }
-        );
+        // Make internal properties non-enumerable so they don't get serialized
+        // when the object is converted to JSON (e.g., for ShareDB sync)
+        const props: [string, any][] = [
+            ['_additionalEmitters', []],
+            ['_events', {}],
+            ['_suspendEvents', false]
+        ];
+        for (const [name, value] of props) {
+            Object.defineProperty(this, name, { enumerable: false, writable: true, value });
+        }
     }
 
     /**

src/observer-list.ts

@@ -7,7 +7,7 @@ import { Observer } from './observer';
 class ObserverList extends Events {
     data: any[] = [];
 
-    private _indexed: Record<number, Observer> = {};
+    private _indexed!: Record<number, Observer>;
 
     sorted: ((arg0: any, arg1: any) => number) | null = null;
 
@@ -20,6 +20,9 @@ class ObserverList extends Events {
     constructor(options: { sorted?: (arg0: any, arg1: any) => number, index?: string } = {}) {
         super();
 
+        // Make internal property non-enumerable so it doesn't get serialized
+        Object.defineProperty(this, '_indexed', { enumerable: false, writable: true, value: {} });
+
         this.sorted = options.sorted || null;
         this.index = options.index || null;
     }

src/observer.ts

@@ -36,27 +36,27 @@ export type ObserverSync = Events & {
  * observer.set('name', 'Jane'); // Logs: Name changed from John to Jane
  */
 class Observer extends Events {
-    private _destroyed: boolean = false;
+    private _destroyed!: boolean;
 
-    private _path: string = '';
+    private _path!: string;
 
-    private _keys: string[] = [];
+    private _keys!: string[];
 
-    private _data: any = { };
+    private _data!: any;
 
-    private _pathsWithDuplicates: any = null;
+    private _pathsWithDuplicates!: Set<string> | null;
 
-    private _parent: Observer = null;
+    private _parent!: Observer | null;
 
-    private _parentPath: string = '';
+    private _parentPath!: string;
 
-    private _parentField: any = null;
+    private _parentField!: any;
 
-    private _parentKey: any = null;
+    private _parentKey!: any;
 
-    private _latestFn: Function = null;
+    private _latestFn!: Function | null;
 
-    private _silent: boolean = false;
+    private _silent!: boolean;
 
     history: ObserverHistory;
 
@@ -80,28 +80,27 @@ class Observer extends Events {
     } = {}) {
         super();
 
-        // array paths where duplicate entries are allowed - normally
-        // we check if an array already has a value before inserting it
-        // but if the array path is in here we'll allow it
-        this._pathsWithDuplicates = null;
-        if (options.pathsWithDuplicates) {
-            this._pathsWithDuplicates = {};
-            for (let i = 0; i < options.pathsWithDuplicates.length; i++) {
-                this._pathsWithDuplicates[options.pathsWithDuplicates[i]] = true;
-            }
+        // Make internal properties non-enumerable so they don't get serialized
+        // when the object is converted to JSON (e.g., for ShareDB sync)
+        const props: [string, any][] = [
+            ['_destroyed', false],
+            ['_path', ''],
+            ['_keys', []],
+            ['_data', {}],
+            ['_pathsWithDuplicates', options.pathsWithDuplicates ? new Set(options.pathsWithDuplicates) : null],
+            ['_parent', options.parent || null],
+            ['_parentPath', options.parentPath || ''],
+            ['_parentField', options.parentField || null],
+            ['_parentKey', options.parentKey || null],
+            ['_latestFn', options.latestFn || null],
+            ['_silent', false]
+        ];
+        for (const [name, value] of props) {
+            Object.defineProperty(this, name, { enumerable: false, writable: true, value });
         }
 
         this.patch(data);
 
-        this._parent = options.parent || null;
-        this._parentPath = options.parentPath || '';
-        this._parentField = options.parentField || null;
-        this._parentKey = options.parentKey || null;
-
-        this._latestFn = options.latestFn || null;
-
-        this._silent = false;
-
         const propagate = function (evt: string) {
             return function (path: string, arg1: any, arg2: any, arg3: any) {
                 if (!this._parent) {
@@ -893,7 +892,7 @@ class Observer extends Events {
         }
 
         const path = node._path ? `${node._path}.${key}` : key;
-        if (value !== null && !allowDuplicates && (!this._pathsWithDuplicates || !this._pathsWithDuplicates[path])) {
+        if (value !== null && !allowDuplicates && !this._pathsWithDuplicates?.has(path)) {
             if (arr.indexOf(value) !== -1) {
                 return;
             }

test/events.test.mjs

@@ -0,0 +1,29 @@
+import { expect } from 'chai';
+
+import { Events } from '../dist/index.mjs';
+
+describe('Events', () => {
+
+    it('has non-enumerable internal properties', () => {
+        const events = new Events();
+        const keys = Object.keys(events);
+
+        expect(keys).to.not.include('_events');
+        expect(keys).to.not.include('_suspendEvents');
+        expect(keys).to.not.include('_additionalEmitters');
+    });
+
+    it('can be serialized to JSON without circular reference errors', () => {
+        const events1 = new Events();
+        const events2 = new Events();
+
+        // Create a scenario that would cause circular reference if _additionalEmitters was enumerable
+        events1.addEmitter(events2);
+        events2.addEmitter(events1);
+
+        // This should not throw
+        expect(() => JSON.stringify(events1)).to.not.throw();
+        expect(() => JSON.stringify(events2)).to.not.throw();
+    });
+
+});

test/observer-list.test.mjs

@@ -0,0 +1,28 @@
+import { expect } from 'chai';
+
+import { Observer, ObserverList } from '../dist/index.mjs';
+
+describe('ObserverList', () => {
+
+    it('has non-enumerable internal properties', () => {
+        const list = new ObserverList();
+        const keys = Object.keys(list);
+
+        expect(keys).to.not.include('_indexed');
+        expect(keys).to.not.include('_events');
+        expect(keys).to.not.include('_suspendEvents');
+        expect(keys).to.not.include('_additionalEmitters');
+    });
+
+    it('can be serialized to JSON without circular reference errors', () => {
+        const list = new ObserverList();
+        const observer = new Observer({ id: 1, name: 'test' });
+        list.add(observer);
+
+        // This should not throw
+        expect(() => JSON.stringify(list.json())).to.not.throw();
+
+        observer.destroy();
+    });
+
+});

test/observer.test.mjs

@@ -2,7 +2,6 @@ import { expect } from 'chai';
 
 import { Observer } from '../dist/index.mjs';
 
-
 const getData = () => {
     return {
         name: 'Will',
@@ -150,4 +149,93 @@ describe('Observer', () => {
 
         observer.destroy();
     });
+
+    it('has non-enumerable internal properties', () => {
+        const observer = new Observer(getData());
+        const keys = Object.keys(observer);
+
+        // Events properties
+        expect(keys).to.not.include('_events');
+        expect(keys).to.not.include('_suspendEvents');
+        expect(keys).to.not.include('_additionalEmitters');
+
+        // Observer's own properties
+        expect(keys).to.not.include('_destroyed');
+        expect(keys).to.not.include('_path');
+        expect(keys).to.not.include('_keys');
+        expect(keys).to.not.include('_data');
+        expect(keys).to.not.include('_parent');
+        expect(keys).to.not.include('_parentPath');
+        expect(keys).to.not.include('_parentField');
+        expect(keys).to.not.include('_parentKey');
+        expect(keys).to.not.include('_latestFn');
+        expect(keys).to.not.include('_silent');
+        expect(keys).to.not.include('_pathsWithDuplicates');
+
+        observer.destroy();
+    });
+
+    it('can be serialized to JSON without including internal properties', () => {
+        const observer = new Observer(getData());
+        const jsonString = JSON.stringify(observer.json());
+
+        expect(jsonString).to.not.include('_events');
+        expect(jsonString).to.not.include('_suspendEvents');
+        expect(jsonString).to.not.include('_additionalEmitters');
+        expect(jsonString).to.not.include('_parent');
+        expect(jsonString).to.not.include('_data');
+
+        observer.destroy();
+    });
+
+    it('can be serialized when nested Observers have parent references', () => {
+        // This tests the exact scenario from the bug: nested Observers with _parent references
+        const parent = new Observer({
+            entries: []
+        });
+
+        // Insert an object which becomes a nested Observer with _parent pointing back to parent
+        // This creates a true circular reference:
+        // - parent._data.entries[0] -> child Observer
+        // - child._parent -> parent
+        parent.insert('entries', { name: 'child' });
+
+        // The nested Observer has a reference back to parent - this would cause circular reference
+        // if _parent was enumerable
+        expect(() => JSON.stringify(parent.json())).to.not.throw();
+
+        // Verify the structure is correct
+        const json = parent.json();
+        expect(json.entries).to.have.lengthOf(1);
+        expect(json.entries[0].name).to.equal('child');
+
+        parent.destroy();
+    });
+
+    it('prevents duplicate array insertions by default', () => {
+        const observer = new Observer({ items: [] });
+
+        observer.insert('items', 'a');
+        observer.insert('items', 'b');
+        observer.insert('items', 'a'); // duplicate - should be ignored
+
+        expect(observer.get('items')).to.deep.equal(['a', 'b']);
+
+        observer.destroy();
+    });
+
+    it('allows duplicate array insertions when path is in pathsWithDuplicates', () => {
+        const observer = new Observer({ items: [] }, {
+            pathsWithDuplicates: ['items']
+        });
+
+        observer.insert('items', 'a');
+        observer.insert('items', 'b');
+        observer.insert('items', 'a'); // duplicate - should be allowed
+
+        expect(observer.get('items')).to.deep.equal(['a', 'b', 'a']);
+
+        observer.destroy();
+    });
+
 });