From 5f2da341657e291d29df953175a187eae2befe75 Mon Sep 17 00:00:00 2001 From: Will Eastcott Date: Sat, 10 Jan 2026 10:08:36 +0000 Subject: [PATCH] [FIX] Fix nested array isolation in Observer constructor --- src/observer.ts | 4 +-- src/utils.ts | 25 ++++++++++++- test/observer.test.mjs | 79 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/observer.ts b/src/observer.ts index eb8b52b..95a6d22 100644 --- a/src/observer.ts +++ b/src/observer.ts @@ -1,6 +1,6 @@ import { Events } from './events'; import { ObserverHistory } from './observer-history'; -import { arrayEquals } from './utils'; +import { arrayEquals, deepCopyArray } from './utils'; /** * The ObserverSync class is used to construct an interface for synchronizing changes from Observer @@ -202,7 +202,7 @@ class Observer extends Events { for (i = 0; i < target._data[key].length; i++) { if (typeof target._data[key][i] === 'object' && target._data[key][i] !== null) { if (target._data[key][i] instanceof Array) { - target._data[key][i].slice(0); + target._data[key][i] = deepCopyArray(target._data[key][i]); } else { target._data[key][i] = new Observer(target._data[key][i], { parent: this, diff --git a/src/utils.ts b/src/utils.ts index 2d0b158..8094203 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -38,4 +38,27 @@ const arrayEquals = (a: any[], b: any[]) => { return true; }; -export { arrayEquals }; +/** + * Creates a deep copy of an array, recursively copying any nested arrays. + * Non-array objects within the array are not deep copied (they remain references). + * + * @param arr - The array to copy. + * @returns A deep copy of the array with all nested arrays also copied. + * + * @example + * const original = [[1, 2], [3, 4]]; + * const copy = deepCopyArray(original); + * copy[0][0] = 99; + * console.log(original[0][0]); // 1 (unchanged) + */ +const deepCopyArray = (arr: any[]): any[] => { + const copy = arr.slice(0); + for (let i = 0; i < copy.length; i++) { + if (copy[i] instanceof Array) { + copy[i] = deepCopyArray(copy[i]); + } + } + return copy; +}; + +export { arrayEquals, deepCopyArray }; diff --git a/test/observer.test.mjs b/test/observer.test.mjs index 6a51308..7f9508a 100644 --- a/test/observer.test.mjs +++ b/test/observer.test.mjs @@ -238,4 +238,83 @@ describe('Observer', () => { observer.destroy(); }); + describe('nested array isolation', () => { + it('isolates nested arrays from source data modifications', () => { + // Simulates the ShareDB scenario: source data is modified externally + const sourceData = { + vectors: [[1, 2, 3], [4, 5, 6]] + }; + + const observer = new Observer(sourceData); + + // Verify initial values + expect(observer.get('vectors.0')).to.deep.equal([1, 2, 3]); + expect(observer.get('vectors.0.0')).to.equal(1); + + // Simulate external modification (like ShareDB updating its document) + sourceData.vectors[0][0] = 999; + + // Observer's data should NOT be affected - it should have its own copy + expect(observer.get('vectors.0.0')).to.equal(1); + expect(observer.get('vectors.0')).to.deep.equal([1, 2, 3]); + + observer.destroy(); + }); + + it('fires set event when updating nested array element after source modification', () => { + // This is the exact bug scenario from GitHub issue #684 + const sourceData = { + arrayVec3: [[1, 0, 0], [0, 1, 0]] + }; + + const observer = new Observer(sourceData); + + // Simulate external modification (ShareDB updates its document) + sourceData.arrayVec3[0][0] = -1.94; + + // Track if set event fires + let setEventFired = false; + let eventPath = null; + let eventValue = null; + + observer.on('*:set', (path, value) => { + setEventFired = true; + eventPath = path; + eventValue = value; + }); + + // Now set the value through the observer (like sync.write does) + observer.set('arrayVec3.0.0', -1.94); + + // The set event SHOULD fire because the observer's internal data + // should still be 1, not -1.94 + expect(setEventFired).to.be.true; + expect(eventPath).to.equal('arrayVec3.0.0'); + expect(eventValue).to.equal(-1.94); + + observer.destroy(); + }); + + it('maintains independent copies of deeply nested arrays', () => { + const sourceData = { + matrix: [ + [[1, 2], [3, 4]], + [[5, 6], [7, 8]] + ] + }; + + const observer = new Observer(sourceData); + + // Modify source at various levels + sourceData.matrix[0][0][0] = 100; + sourceData.matrix[1][1] = [99, 99]; + + // Observer should have independent data + expect(observer.get('matrix.0.0.0')).to.equal(1); + expect(observer.get('matrix.1.1')).to.deep.equal([7, 8]); + + observer.destroy(); + }); + }); + });