diff --git a/CHANGELOG.md b/CHANGELOG.md index f84725b..39bd866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Observer**: Fixed dependencies being cleared when reactive objects are created during a computed's execution. Previously, writing to a new atom would clear all dependencies sharing the same key, including the original source. + ## [1.1.4] - 2025-12-19 ### Fixed diff --git a/src/reactivity/Observer.ts b/src/reactivity/Observer.ts index a1ac11a..aaf629a 100644 --- a/src/reactivity/Observer.ts +++ b/src/reactivity/Observer.ts @@ -13,6 +13,7 @@ import { } from './global'; const GLOBAL_STACK: Array> = []; +const RUNNING_OBSERVERS = new WeakSet>(); /** * Returns the currently running Observer, if any. @@ -27,6 +28,20 @@ export function getCurrentObserver(): ObservableItem | undefined { return GLOBAL_STACK.at(-1); } +/** + * Checks if an Observer is currently running (on the execution stack). + * + * Used to prevent clearing dependencies for an observer that is still + * collecting dependencies during its getter execution. + * + * @param obs - The observer to check + * @returns true if the observer is currently running + * @internal + */ +export function isObserverRunning(obs: ObservableItem): boolean { + return RUNNING_OBSERVERS.has(obs); +} + /** * Lifecycle hooks that can be called on an Observer. */ @@ -152,11 +167,13 @@ class ObservableItem { private wrap(cb: () => CBType): CBType { GLOBAL_STACK.push(this); + RUNNING_OBSERVERS.add(this); try { return cb(); } finally { GLOBAL_STACK.pop(); + RUNNING_OBSERVERS.delete(this); } } diff --git a/src/reactivity/__spec__/observer.spec.ts b/src/reactivity/__spec__/observer.spec.ts index c4ef8f4..42366ea 100644 --- a/src/reactivity/__spec__/observer.spec.ts +++ b/src/reactivity/__spec__/observer.spec.ts @@ -606,4 +606,160 @@ describe('Observer', () => { expect(obs.name).toEqual('myComputed'); }); }); + + describe('creating reactive objects during computation', () => { + it('maintains dependency tracking when atoms with same key are modified during computation', () => { + // This test verifies a bug fix where setting properties on reactive objects + // created during a computed's execution would clear unrelated dependencies + // that share the same tracking key. + // + // The bug occurred because: + // 1. A computed runs and clears its dependencies + // 2. It accesses an atom's 'value' property - registers dependency under key 'value' + // 3. It creates new atoms and READS from them (registering more 'value' deps) + // 4. It then WRITES to those atoms, triggering trackerChanged(atom, 'value') + // 5. trackerChanged called observerClear(obs, 'value') which cleared ALL 'value' deps + // 6. This cleared the dependency on the ORIGINAL atom + // + // This pattern mirrors how @reactive decorators work - each decorated property + // uses an atom {value: T} internally, all sharing 'value' as the tracking key. + + // Create an "atom" - a reactive wrapper with a 'value' property + // This is how @reactive properties work internally + const sourceAtom = Reactive({ value: [1, 2, 3] }); + + let computeCount = 0; + + const obs = Observer({ + get: () => { + computeCount += 1; + + // Access the atom's value - registers dependency under key 'value' + const items = sourceAtom.value; + + // Create new atoms during computation + return items.map((item) => { + const wrapperAtom = Reactive({ value: item }); + + // READ from the atom first - this registers a dependency on wrapperAtom + // under key 'value'. Now the observer depends on BOTH sourceAtom.value + // AND wrapperAtom.value, both under the same key 'value'. + const currentValue = wrapperAtom.value; + + // WRITE to the atom - this triggers trackerChanged(wrapperAtom, 'value') + // which notifies all observers of wrapperAtom.value (including us). + // The bug: observerClear(obs, 'value') was called unconditionally, + // clearing ALL 'value' dependencies including sourceAtom.value! + wrapperAtom.value = currentValue * 2; + + return wrapperAtom; + }); + }, + }); + + // First access + const result1 = obs.value; + + expect(result1).toHaveLength(3); + expect(result1[0].value).toEqual(2); + expect(computeCount).toEqual(1); + + // Change the ORIGINAL atom's value + // This should trigger recomputation because we depend on sourceAtom.value + sourceAtom.value = [10, 20]; + + // The computed MUST rerun - this is the critical assertion + const result2 = obs.value; + + expect(result2).toHaveLength(2); + expect(result2[0].value).toEqual(20); + expect(computeCount).toEqual(2); + }); + + it('preserves dependencies when multiple atoms share the same key', () => { + // Simulates the @reactive decorator pattern where multiple properties + // all use atoms with 'value' as the key + + const atom1 = Reactive({ value: 'hello' }); + const atom2 = Reactive({ value: 'world' }); + + let computeCount = 0; + + const obs = Observer({ + get: () => { + computeCount += 1; + + // Depend on atom1.value + const greeting = atom1.value; + + // Create a new atom during computation + const tempAtom = Reactive({ value: greeting }); + + // READ then WRITE - the read registers dependency, the write triggers change + const current = tempAtom.value; + + tempAtom.value = current.toUpperCase(); // This SET was clearing atom1 dependency + + return `${tempAtom.value} ${atom2.value}`; + }, + }); + + expect(obs.value).toEqual('HELLO world'); + expect(computeCount).toEqual(1); + + // Changing atom1 should trigger recomputation + atom1.value = 'goodbye'; + + expect(obs.value).toEqual('GOODBYE world'); + expect(computeCount).toEqual(2); + + // Changing atom2 should also trigger recomputation + atom2.value = 'everyone'; + + expect(obs.value).toEqual('GOODBYE everyone'); + expect(computeCount).toEqual(3); + }); + + it('handles nested computations with atom creation correctly', () => { + const sourceAtom = Reactive({ value: 5 }); + + let innerComputeCount = 0; + let outerComputeCount = 0; + + const innerObs = Observer({ + get: () => { + innerComputeCount += 1; + + const val = sourceAtom.value; + + // Create atom during computation + const temp = Reactive({ value: val }); + + // READ then WRITE pattern that triggers the bug + const current = temp.value; + + temp.value = current * 2; + return temp.value; + }, + }); + + const outerObs = Observer({ + get: () => { + outerComputeCount += 1; + return innerObs.value + 1; + }, + }); + + expect(outerObs.value).toEqual(11); // (5 * 2) + 1 + expect(innerComputeCount).toEqual(1); + expect(outerComputeCount).toEqual(1); + + // Change source - both should recompute + sourceAtom.value = 10; + + expect(outerObs.value).toEqual(21); // (10 * 2) + 1 + expect(innerComputeCount).toEqual(2); + expect(outerComputeCount).toEqual(2); + }); + }); }); diff --git a/src/reactivity/global.ts b/src/reactivity/global.ts index 153d46b..e0ab4a7 100644 --- a/src/reactivity/global.ts +++ b/src/reactivity/global.ts @@ -11,7 +11,7 @@ * @internal */ -import { getCurrentObserver, ObservableItem } from './Observer'; +import { getCurrentObserver, isObserverRunning, ObservableItem } from './Observer'; // constants /** Symbol used to track when an object's keys change */ @@ -230,9 +230,13 @@ export const trackerChanged = (trk: object, key: DependencyKey): void => { for (const obs of observers.get(key)) { // tell the observer it needs to run again obs.setDirty(); - // the observer is dirty, so we don't need to track it - // anymore until the observer runs again - observerClear(obs, key); + + // Only clear the dependency if the observer is NOT currently running. + // If it's running, it's in the middle of collecting new dependencies + // and we don't want to clear dependencies that were just registered. + if (!isObserverRunning(obs)) { + observerClear(obs, key); + } } } };