Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions src/reactivity/Observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from './global';

const GLOBAL_STACK: Array<ObservableItem<unknown>> = [];
const RUNNING_OBSERVERS = new WeakSet<ObservableItem<unknown>>();

/**
* Returns the currently running Observer, if any.
Expand All @@ -27,6 +28,20 @@ export function getCurrentObserver(): ObservableItem<unknown> | 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<unknown>): boolean {
return RUNNING_OBSERVERS.has(obs);
}

/**
* Lifecycle hooks that can be called on an Observer.
*/
Expand Down Expand Up @@ -152,11 +167,13 @@ class ObservableItem<T> {

private wrap<CBType>(cb: () => CBType): CBType {
GLOBAL_STACK.push(this);
RUNNING_OBSERVERS.add(this);

try {
return cb();
} finally {
GLOBAL_STACK.pop();
RUNNING_OBSERVERS.delete(this);
}
}

Expand Down
156 changes: 156 additions & 0 deletions src/reactivity/__spec__/observer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
12 changes: 8 additions & 4 deletions src/reactivity/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
}
}
}
};
Expand Down
Loading