diff --git a/src/ObservableObject.ts b/src/ObservableObject.ts index b4421db8..873b13ac 100644 --- a/src/ObservableObject.ts +++ b/src/ObservableObject.ts @@ -230,9 +230,14 @@ function updateNodes(parent: NodeInfo, obj: Record | Array | unde } else if (prevValue && (!obj || isObject(obj))) { // For keys that have been removed from object, notify and update children recursively const lengthPrev = keysPrev.length; + let keySet: Set | undefined; for (let i = 0; i < lengthPrev; i++) { const key = keysPrev[i]; - if (!keys.includes(key)) { + // Fast path for same-position keys, then Set lookup for large key lists to avoid quadratic scans. + // Map keys can be undefined, so an out-of-bounds read must not count as a match. + if (i < length && keys[i] === key) continue; + if (!keySet && keys.length > 32 && lengthPrev > 32) keySet = new Set(keys); + if (!(keySet ? keySet.has(key) : keys.includes(key))) { hasADiff = true; const child = getChildNode(parent, key); @@ -1479,6 +1484,9 @@ function recursivelyAutoActivateInner(obj: Record, pathStack: strin for (const key in obj) { if (hasOwnProperty.call(obj, key)) { const value = obj[key]; + // Scalars can never be observables or linked functions; skip them before the isObservable check. + const type = typeof value; + if (type !== 'object' && type !== 'function') continue; if (isObservable(value)) { const childNode = getNodeAtPath(); diff --git a/tests/computed.test.ts b/tests/computed.test.ts index 36b631c2..ca1dd486 100644 --- a/tests/computed.test.ts +++ b/tests/computed.test.ts @@ -2441,6 +2441,42 @@ describe('Complex computeds', () => { }); }); describe('Activation', () => { + test('Recursive activation traverses mixed scalar values, objects, and arrays', () => { + const source = observable(1); + const lazyGet = jest.fn(() => 10); + const normalFunction = jest.fn(() => 20); + const scalars = { + text: 'hello', + number: 3, + boolean: true, + nil: null, + missing: undefined, + bigint: BigInt(1), + symbol: Symbol('value'), + }; + const obs = observable({ + ...scalars, + nested: { + ...scalars, + value: source, + auto: linked({ get: () => source.get() + 1, activate: 'auto' }), + lazy: linked({ get: lazyGet, activate: 'lazy' }), + normalFunction, + }, + array: [null, 'hello', { value: source }], + }); + + const value = obs.get(); + + expect(value).toMatchObject(scalars); + expect(value.nested).toMatchObject({ ...scalars, value: 1, auto: 2 }); + expect(value.array).toEqual([null, 'hello', { value: 1 }]); + expect(lazyGet).not.toHaveBeenCalled(); + expect(normalFunction).not.toHaveBeenCalled(); + source.set(2); + expect(obs.get().nested).toMatchObject({ value: 2, auto: 3 }); + expect(obs.get().array).toEqual([null, 'hello', { value: 2 }]); + }); test('linked with function', () => { const obs = observable({ test: 10, test2: 20 }); const comp = observable(linked(() => obs.test.get() + obs.test2.get())); diff --git a/tests/crud.test.ts b/tests/crud.test.ts index 0eefa835..697352f5 100644 --- a/tests/crud.test.ts +++ b/tests/crud.test.ts @@ -1999,7 +1999,8 @@ describe('lastSync', () => { syncState(obs$).sync(); expect(numLists).toEqual(2); - await promiseTimeout(10); + // lastSync is published after the persistence plugin finishes writing metadata. + await when(() => syncState(obs$).lastSync.get() === 2); expect(localStorage.getItem(persistName + '__m')!).toEqual(JSON.stringify({ lastSync: 2 })); }); diff --git a/tests/mapset.test.ts b/tests/mapset.test.ts index df50b428..0cf6f79d 100644 --- a/tests/mapset.test.ts +++ b/tests/mapset.test.ts @@ -1,7 +1,8 @@ import { observe } from '../src/observe'; -import { isObservable, optimized } from '../src/globals'; +import { getChildNode, getNode, isObservable, optimized } from '../src/globals'; import { mergeIntoObservable } from '../src/helpers'; import { observable } from '../src/observable'; +import { getProxy } from '../src/ObservableObject'; import { expectChangeHandler } from './testglobals'; describe('Map default behavior', () => { @@ -24,6 +25,50 @@ describe('Map default behavior', () => { expect(obs.test.get('key3').get()).toEqual('value3'); expect(obs.test.has('key1')).toEqual(false); }); + test.each([32, 33])('Reordering %i Map keys preserves key identity and retained listeners', (size) => { + const objectKey = { id: 1 }; + const replacementKey = { id: 1 }; + const initial = new Map([ + [objectKey, 1], + [NaN, 2], + [0, 3], + ...Array.from({ length: size - 3 }, (_, i): [unknown, number] => ['key' + i, i]), + ]); + const obs = observable(initial); + const onRemoved = expectChangeHandler(obs.get(objectKey)); + const onRetained = jest.fn(); + obs.get(NaN).onChange(onRetained); + obs.get(0).onChange(onRetained); + const next = new Map([...initial].reverse()); + next.delete(objectKey); + next.set(replacementKey, 4); + + obs.set(next); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined, 1, [ + { path: [], pathTypes: [], prevAtPath: 1, valueAtPath: undefined }, + ]); + expect(onRetained).not.toHaveBeenCalled(); + expect(obs.peek()).toEqual(next); + obs.get(NaN).set(5); + obs.get(-0).set(6); + expect(onRetained).toHaveBeenCalledTimes(2); + }); + test.each([0, 1, 33])('Map replacement removes an undefined key after %i retained keys', (size) => { + const retained = Array.from({ length: size }, (_, i): [unknown, number] => ['key' + i, i]); + const obs = observable(new Map([...retained, [undefined, 42]])); + // Access the node directly because get(undefined) currently returns the Map's own proxy. + const child = getProxy(getChildNode(getNode(obs), undefined as any)); + const onRemoved = jest.fn(); + child.onChange(({ value }) => onRemoved(value)); + + obs.set(new Map(retained)); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined); + expect(obs.peek()).toEqual(new Map(retained)); + }); test('Map delete', () => { const obs = observable({ test: new Map([ diff --git a/tests/perf.test.ts b/tests/perf.test.ts index 3b862155..b52123ee 100644 --- a/tests/perf.test.ts +++ b/tests/perf.test.ts @@ -1,4 +1,3 @@ -import { synced } from '../src/sync/synced'; import { ObservableHint } from '../src/ObservableHint'; import { linked } from '../src/linked'; import { observable } from '../src/observable'; @@ -81,34 +80,6 @@ describe('Perf', () => { expect(then - now).toBeLessThan(1); }); - test('Lazy activation perf with synced', () => { - const obj: Record = {}; - const Num = 10000; - for (let i = 0; i < Num; i++) { - obj['key' + i] = { - child: { - grandchild: { - value: 'hi', - }, - }, - }; - } - - const obs = observable( - synced>({ - get: () => null, - subscribe: ({ update }) => { - update({ value: obj }); - }, - }), - ); - - const now = performance.now(); - obs.get(); - const then = performance.now(); - - expect(then - now).toBeLessThan(5); - }); test('Lazy activation perf2', () => { const obj: Record = {}; const Num = 10000; diff --git a/tests/synced.test.ts b/tests/synced.test.ts index 85b6e5e2..5db40fa8 100644 --- a/tests/synced.test.ts +++ b/tests/synced.test.ts @@ -2,6 +2,31 @@ import { observable, observe } from '@legendapp/state'; import { synced } from '@legendapp/state/sync'; import { promiseTimeout } from './testglobals'; +describe('Lazy activation', () => { + test('Reading a synced root does not traverse the subscribed data', () => { + const enumerateChild = jest.fn(Reflect.ownKeys); + const data: Record = {}; + for (let i = 0; i < 10000; i++) { + data['key' + i] = new Proxy<(typeof data)[string]>( + { child: { grandchild: { value: 'hi' } } }, + { ownKeys: enumerateChild }, + ); + } + const get = jest.fn(() => null); + const subscribe = jest.fn(({ update }) => update({ value: data })); + const obs$ = observable(synced({ get, subscribe })); + + expect(get).not.toHaveBeenCalled(); + expect(subscribe).not.toHaveBeenCalled(); + expect(obs$.get()).toBe(data); + expect(get).toHaveBeenCalledTimes(1); + expect(subscribe).toHaveBeenCalledTimes(1); + // Catch eager recursive activation without relying on a wall-clock deadline. + expect(enumerateChild).not.toHaveBeenCalled(); + expect(obs$.key9999.child.grandchild.value.get()).toBe('hi'); + }); +}); + describe('unsubscribe', () => { test('Canceling observe unsubscribes', async () => { let numObserves = 0; diff --git a/tests/tests.test.ts b/tests/tests.test.ts index 0eb3194f..b0cd42a6 100644 --- a/tests/tests.test.ts +++ b/tests/tests.test.ts @@ -35,6 +35,45 @@ describe('Set', () => { expect(obs.test.get()).toEqual({ text: 't2' }); expect(obs.get()).toEqual({ test: { text: 't2' } }); }); + test.each([32, 33, 2000])('Replacing all %i object keys notifies removed descendants', (size) => { + const initial = Object.fromEntries(Array.from({ length: size }, (_, i) => ['key' + i, { value: i }])); + const obs = observable>(initial); + const removed = obs['key' + (size - 1)].value; + const onRemoved = expectChangeHandler(removed); + const next = Object.fromEntries(Array.from({ length: size }, (_, i) => ['other' + i, { value: i }])); + + obs.set(next); + + expect(onRemoved).toHaveBeenCalledTimes(1); + expect(onRemoved).toHaveBeenCalledWith(undefined, size - 1, [ + { path: [], pathTypes: [], prevAtPath: size - 1, valueAtPath: undefined }, + ]); + expect(obs.peek()).toEqual(next); + }); + test.each([32, 33])('Reordering %i object keys preserves retained listeners and deletion order', (size) => { + const initial = Object.fromEntries(Array.from({ length: size }, (_, i) => ['key' + i, i])); + const obs = observable>(initial); + const retained = obs.key1; + const onRetained = jest.fn(); + retained.onChange(onRetained); + const removed: string[] = []; + obs.key0.onChange(() => removed.push('key0')); + obs['key' + (size - 1)].onChange(() => removed.push('key' + (size - 1))); + const next = Object.fromEntries([ + ['added0', -1], + ...Object.entries(initial).slice(1, -1).reverse(), + ['added1', -2], + ]); + + obs.set(next); + + expect(removed).toEqual(['key0', 'key' + (size - 1)]); + expect(onRetained).not.toHaveBeenCalled(); + expect(obs.key1).toBe(retained); + expect(obs.peek()).toEqual(next); + obs.key1.set(100); + expect(onRetained).toHaveBeenCalledTimes(1); + }); test('Set primitive', () => { const obs = observable({ test: { text: 't' } }); obs.test.text.set('t2');