From 963bbdd3c023178cd64d85c49adf0aa462cecf48 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 16:54:30 +0000 Subject: [PATCH 1/6] [FIX] Fix circular reference error when serializing Observer data --- src/events.ts | 26 +++++----- src/observer-list.ts | 5 +- src/observer.ts | 59 +++++++++++---------- test/observer.test.mjs | 113 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 158 insertions(+), 45 deletions(-) diff --git a/src/events.ts b/src/events.ts index f906165..7cfe22f 100644 --- a/src/events.ts +++ b/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; - private _events: Record = {}; + 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 }); + } } /** diff --git a/src/observer-list.ts b/src/observer-list.ts index 1bb89e0..6397e90 100644 --- a/src/observer-list.ts +++ b/src/observer-list.ts @@ -7,7 +7,7 @@ import { Observer } from './observer'; class ObserverList extends Events { data: any[] = []; - private _indexed: Record = {}; + private _indexed: Record; 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; } diff --git a/src/observer.ts b/src/observer.ts index 7477101..de8a36e 100644 --- a/src/observer.ts +++ b/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 | null; - private _parent: Observer = null; + private _parent: Observer; - 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; - 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; } diff --git a/test/observer.test.mjs b/test/observer.test.mjs index 0171a79..11df405 100644 --- a/test/observer.test.mjs +++ b/test/observer.test.mjs @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { Observer } from '../dist/index.mjs'; +import { Events, Observer, ObserverList } from '../dist/index.mjs'; const getData = () => { @@ -151,3 +151,114 @@ describe('Observer', () => { observer.destroy(); }); }); + +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(); + }); + +}); + +describe('Observer (non-enumerable properties)', () => { + + it('has non-enumerable internal properties inherited from Events', () => { + 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: [] + }); + + // Simulate what happens when array items become nested Observers + const child = new Observer({ name: 'child' }, { parent: parent, parentPath: 'entries', parentField: [] }); + + // The child has a reference back to parent - this would cause circular reference + // if _parent was enumerable + expect(() => JSON.stringify(parent.json())).to.not.throw(); + expect(() => JSON.stringify(child.json())).to.not.throw(); + + parent.destroy(); + child.destroy(); + }); + +}); + +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(); + }); + +}); From ddccbc1dc5ad9bac40e19a30ae249dbbc38a4463 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 17:02:53 +0000 Subject: [PATCH 2/6] Reorganize tests --- eslint.config.mjs | 5 ++++ package.json | 4 +-- test/events.test.mjs | 29 +++++++++++++++++++ test/observer-list.test.mjs | 28 ++++++++++++++++++ test/observer.test.mjs | 57 +------------------------------------ 5 files changed, 65 insertions(+), 58 deletions(-) create mode 100644 test/events.test.mjs create mode 100644 test/observer-list.test.mjs diff --git a/eslint.config.mjs b/eslint.config.mjs index e036b33..1ac1216 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -52,6 +52,11 @@ export default [ }, { files: ['**/*.test.js', '**/*.test.mjs'], + languageOptions: { + globals: { + ...globals.mocha + } + }, rules: { 'no-unused-expressions': 'off' } diff --git a/package.json b/package.json index 701b83e..e902042 100644 --- a/package.json +++ b/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", diff --git a/test/events.test.mjs b/test/events.test.mjs new file mode 100644 index 0000000..8f62720 --- /dev/null +++ b/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(); + }); + +}); diff --git a/test/observer-list.test.mjs b/test/observer-list.test.mjs new file mode 100644 index 0000000..ffe6df0 --- /dev/null +++ b/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(); + }); + +}); diff --git a/test/observer.test.mjs b/test/observer.test.mjs index 11df405..efba8dc 100644 --- a/test/observer.test.mjs +++ b/test/observer.test.mjs @@ -1,7 +1,6 @@ import { expect } from 'chai'; -import { Events, Observer, ObserverList } from '../dist/index.mjs'; - +import { Observer } from '../dist/index.mjs'; const getData = () => { return { @@ -150,37 +149,8 @@ describe('Observer', () => { observer.destroy(); }); -}); - -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(); - }); - -}); - -describe('Observer (non-enumerable properties)', () => { - - it('has non-enumerable internal properties inherited from Events', () => { const observer = new Observer(getData()); const keys = Object.keys(observer); @@ -237,28 +207,3 @@ describe('Observer (non-enumerable properties)', () => { }); }); - -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(); - }); - -}); From a72c6cfef748245f1d5d3def42f126c935cb2940 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 17:09:20 +0000 Subject: [PATCH 3/6] Use definite assignment assertion operator --- src/events.ts | 6 +++--- src/observer-list.ts | 2 +- src/observer.ts | 22 +++++++++++----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/events.ts b/src/events.ts index 7cfe22f..080f953 100644 --- a/src/events.ts +++ b/src/events.ts @@ -35,11 +35,11 @@ export type HandleEvent = (arg1?: any, arg2?: any, arg3?: any, arg4?: any, arg5? * events.unbind('testEvent'); */ class Events { - private _additionalEmitters: Events[]; + private _additionalEmitters!: Events[]; - private _events: Record; + private _events!: Record; - private _suspendEvents: boolean; + private _suspendEvents!: boolean; /** * Creates a new Events instance. diff --git a/src/observer-list.ts b/src/observer-list.ts index 6397e90..3588801 100644 --- a/src/observer-list.ts +++ b/src/observer-list.ts @@ -7,7 +7,7 @@ import { Observer } from './observer'; class ObserverList extends Events { data: any[] = []; - private _indexed: Record; + private _indexed!: Record; sorted: ((arg0: any, arg1: any) => number) | null = null; diff --git a/src/observer.ts b/src/observer.ts index de8a36e..2bad90a 100644 --- a/src/observer.ts +++ b/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; + private _destroyed!: boolean; - private _path: string; + private _path!: string; - private _keys: string[]; + private _keys!: string[]; - private _data: any; + private _data!: any; - private _pathsWithDuplicates: Set | null; + private _pathsWithDuplicates!: Set | null; - private _parent: Observer; + private _parent!: Observer; - private _parentPath: string; + private _parentPath!: string; - private _parentField: any; + private _parentField!: any; - private _parentKey: any; + private _parentKey!: any; - private _latestFn: Function; + private _latestFn!: Function; - private _silent: boolean; + private _silent!: boolean; history: ObserverHistory; From c18f52bd8f133b016caccee75eec05335f4893ce Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 17:13:02 +0000 Subject: [PATCH 4/6] Typing fixes --- src/observer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/observer.ts b/src/observer.ts index 2bad90a..eb8b52b 100644 --- a/src/observer.ts +++ b/src/observer.ts @@ -46,7 +46,7 @@ class Observer extends Events { private _pathsWithDuplicates!: Set | null; - private _parent!: Observer; + private _parent!: Observer | null; private _parentPath!: string; @@ -54,7 +54,7 @@ class Observer extends Events { private _parentKey!: any; - private _latestFn!: Function; + private _latestFn!: Function | null; private _silent!: boolean; From 5db47d4225fa4265a6cefb27a035ffc4d9c700d2 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 17:19:47 +0000 Subject: [PATCH 5/6] Improve tests --- test/observer.test.mjs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/observer.test.mjs b/test/observer.test.mjs index efba8dc..1e33031 100644 --- a/test/observer.test.mjs +++ b/test/observer.test.mjs @@ -206,4 +206,30 @@ describe('Observer', () => { child.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(); + }); + }); From 4b4c67fcfbe8d62460730620792c7dbe060e59f6 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Mon, 5 Jan 2026 17:21:26 +0000 Subject: [PATCH 6/6] Improve tests --- test/observer.test.mjs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/observer.test.mjs b/test/observer.test.mjs index 1e33031..6a51308 100644 --- a/test/observer.test.mjs +++ b/test/observer.test.mjs @@ -194,16 +194,22 @@ describe('Observer', () => { entries: [] }); - // Simulate what happens when array items become nested Observers - const child = new Observer({ name: 'child' }, { parent: parent, parentPath: 'entries', parentField: [] }); + // 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 child has a reference back to parent - this would cause circular reference + // 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(); - expect(() => JSON.stringify(child.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(); - child.destroy(); }); it('prevents duplicate array insertions by default', () => {