From c3304082777b57c3f505b290242e1ec5a78b067c Mon Sep 17 00:00:00 2001 From: Sheraff Date: Mon, 28 Sep 2026 23:23:44 +0200 Subject: [PATCH 1/2] fix(store): release unobserved computed atoms with retained dependency links --- .changeset/tidy-unobserved-atoms.md | 5 + packages/store/src/alien.ts | 96 ++++--- packages/store/src/atom.ts | 107 +++++-- packages/store/tests/unobserved-atoms.test.ts | 271 ++++++++++++++++++ 4 files changed, 419 insertions(+), 60 deletions(-) create mode 100644 .changeset/tidy-unobserved-atoms.md create mode 100644 packages/store/tests/unobserved-atoms.test.ts diff --git a/.changeset/tidy-unobserved-atoms.md b/.changeset/tidy-unobserved-atoms.md new file mode 100644 index 00000000..50c72137 --- /dev/null +++ b/.changeset/tidy-unobserved-atoms.md @@ -0,0 +1,5 @@ +--- +'@tanstack/store': patch +--- + +Allow computed atoms read without a subscription to be garbage collected while preserving cached snapshots and reconnecting dependencies when subscribed. diff --git a/packages/store/src/alien.ts b/packages/store/src/alien.ts index f41dd140..2088521f 100644 --- a/packages/store/src/alien.ts +++ b/packages/store/src/alien.ts @@ -57,62 +57,76 @@ export function createReactiveSystem({ if (prevDep !== undefined && prevDep.dep === dep) { return } - const nextDep = prevDep !== undefined ? prevDep.nextDep : sub.deps + let nextDep = prevDep !== undefined ? prevDep.nextDep : sub.deps + const prevSub = dep.subsTail if (nextDep !== undefined && nextDep.dep === dep) { nextDep.version = version sub.depsTail = nextDep - return - } - const prevSub = dep.subsTail - if ( - prevSub !== undefined && - prevSub.version === version && - prevSub.sub === sub - ) { - return - } - const newLink = - (sub.depsTail = - dep.subsTail = - { - version, - dep, - sub, - prevDep, - nextDep, - prevSub, - nextSub: undefined, - }) - if (nextDep !== undefined) { - nextDep.prevDep = newLink - } - if (prevDep !== undefined) { - prevDep.nextDep = newLink + // A self pointer marks a retained link absent from the subscriber list. + if (nextDep.prevSub !== nextDep) { + return + } } else { - sub.deps = newLink + if ( + prevSub !== undefined && + prevSub.version === version && + prevSub.sub === sub + ) { + return + } + const newLink = (sub.depsTail = { + version, + dep, + sub, + prevDep, + nextDep, + prevSub, + nextSub: undefined, + }) + if (nextDep !== undefined) { + nextDep.prevDep = newLink + } + if (prevDep !== undefined) { + prevDep.nextDep = newLink + } else { + sub.deps = newLink + } + nextDep = newLink } + dep.subsTail = nextDep + nextDep.prevSub = prevSub if (prevSub !== undefined) { - prevSub.nextSub = newLink + prevSub.nextSub = nextDep } else { - dep.subs = newLink + dep.subs = nextDep } } - function unlink(link: Link, sub = link.sub): Link | undefined { + function unlink( + link: Link, + sub = link.sub, + keepDeps = false, + ): Link | undefined { const dep = link.dep const prevDep = link.prevDep const nextDep = link.nextDep const nextSub = link.nextSub const prevSub = link.prevSub - if (nextDep !== undefined) { - nextDep.prevDep = prevDep - } else { - sub.depsTail = prevDep + // Unobserved atoms retain their forward links for validation and reuse. + if (!keepDeps) { + if (nextDep !== undefined) { + nextDep.prevDep = prevDep + } else { + sub.depsTail = prevDep + } + if (prevDep !== undefined) { + prevDep.nextDep = nextDep + } else { + sub.deps = nextDep + } } - if (prevDep !== undefined) { - prevDep.nextDep = nextDep - } else { - sub.deps = nextDep + if (prevSub === link) { + return nextDep } if (nextSub !== undefined) { nextSub.prevSub = prevSub @@ -124,6 +138,8 @@ export function createReactiveSystem({ } else if ((dep.subs = nextSub) === undefined) { unwatched(dep) } + link.prevSub = link + link.nextSub = undefined return nextDep } diff --git a/packages/store/src/atom.ts b/packages/store/src/atom.ts index 1a29e607..939ce166 100644 --- a/packages/store/src/atom.ts +++ b/packages/store/src/atom.ts @@ -8,7 +8,7 @@ import { createReactiveSystem, } from './alien' -import type { ReactiveNode } from './alien' +import type { Link, ReactiveNode } from './alien' import type { Atom, AtomOptions, @@ -36,6 +36,8 @@ export function toObserver( interface InternalAtom extends ReactiveNode { _snapshot: T + _version: number + _unwatchedDeps?: boolean _update: (getValue?: T | ((snapshot: T) => T)) => boolean get: () => T subscribe: (observerOrFn: Observer | ((value: T) => void)) => Subscription @@ -43,6 +45,7 @@ interface InternalAtom extends ReactiveNode { const queuedEffects: Array = [] let cycle = 0 +let writeVersion = 0 const { link, unlink, propagate, checkDirty, shallowPropagate } = createReactiveSystem({ update(atom: InternalAtom): boolean { @@ -53,13 +56,7 @@ const { link, unlink, propagate, checkDirty, shallowPropagate } = queuedEffects[queuedEffectsLength++] = effect effect.flags &= ~WATCHING }, - unwatched(atom: InternalAtom): void { - if (atom.depsTail !== undefined) { - atom.depsTail = undefined - atom.flags = MUTABLE | DIRTY - purgeDeps(atom) - } - }, + unwatched, }) let notifyIndex = 0 @@ -86,6 +83,22 @@ function purgeDeps(sub: ReactiveNode) { } } +function unwatched(atom: InternalAtom): void { + if (atom.deps === undefined || atom._unwatchedDeps) { + return + } + atom._unwatchedDeps = true + // Preserve pending changes before replacing tracking cycles with snapshots + // of dependency versions. Detached nodes no longer receive invalidations. + if (atom.flags & PENDING) { + atom.flags = (atom.flags & ~PENDING) | DIRTY + } + for (let dep: Link | undefined = atom.deps; dep; dep = dep.nextDep) { + dep.version = (dep.dep as InternalAtom)._version + unlink(dep, atom, true) + } +} + export function flush(): void { if (batchDepth > 0) { return @@ -161,6 +174,7 @@ export function createAtom( // Create plain object atom const atom: InternalAtom = { _snapshot: isComputed ? undefined! : valueOrFn, + _version: 0, subs: undefined, subsTail: undefined, @@ -200,6 +214,7 @@ export function createAtom( const prevSub = activeSub const compare = options?.compare ?? Object.is if (isComputed) { + atom._unwatchedDeps = undefined activeSub = atom ++cycle atom.depsTail = undefined @@ -221,9 +236,15 @@ export function createAtom( : getValue! if (oldValue === undefined || !compare(oldValue, newValue)) { atom._snapshot = newValue + atom._version = ++writeVersion return true } return false + } catch (error) { + if (isComputed) { + atom.flags |= DIRTY + } + throw error } finally { activeSub = prevSub if (isComputed) { @@ -235,23 +256,69 @@ export function createAtom( } if (isComputed) { + let checkedVersion = -1 atom.flags = MUTABLE | DIRTY atom.get = function (): T { - const flags = atom.flags - if (flags & DIRTY || (flags & PENDING && checkDirty(atom.deps!, atom))) { - if (atom._update()) { - const subs = atom.subs - if (subs !== undefined) { - shallowPropagate(subs) + try { + const deps = atom._unwatchedDeps && atom.deps + if ( + deps && + !(atom.flags & DIRTY) && + (activeSub !== undefined || checkedVersion !== writeVersion) + ) { + const prevSub = activeSub + // Subscribing reconnects the cached dependencies without changing the + // snapshot. An untracked read validates them without retaining links. + activeSub = prevSub === undefined ? undefined : atom + if (activeSub !== undefined) { + atom._unwatchedDeps = undefined + atom.depsTail = undefined + } + try { + for ( + let depLink: Link | undefined = deps; + depLink; + depLink = depLink.nextDep + ) { + const dep = depLink.dep as InternalAtom + const version = depLink.version + dep.get() + if (dep._version !== version) { + atom.flags |= DIRTY + break + } + } + } finally { + activeSub = prevSub } } - } else if (flags & PENDING) { - atom.flags = flags & ~PENDING - } - if (activeSub !== undefined) { - link(atom, activeSub, cycle) + const flags = atom.flags + if ( + flags & DIRTY || + (flags & PENDING && checkDirty(atom.deps!, atom)) + ) { + if (atom._update()) { + const subs = atom.subs + if (subs !== undefined) { + shallowPropagate(subs) + } + } + } else if (flags & PENDING) { + atom.flags = flags & ~PENDING + } + if (activeSub !== undefined) { + link(atom, activeSub, cycle) + } + checkedVersion = writeVersion + return atom._snapshot + } catch (error) { + atom.flags |= DIRTY + throw error + } finally { + if (atom.subs === undefined) { + unwatched(atom) + } } - return atom._snapshot } } else { ;(atom as unknown as Atom).set = function ( diff --git a/packages/store/tests/unobserved-atoms.test.ts b/packages/store/tests/unobserved-atoms.test.ts new file mode 100644 index 00000000..b38bcb8e --- /dev/null +++ b/packages/store/tests/unobserved-atoms.test.ts @@ -0,0 +1,271 @@ +import { describe, expect, test, vi } from 'vitest' +import { batch, createAsyncAtom, createAtom } from '../src' +import type { ReactiveNode } from '../src/alien' + +function expectUnwatched(atom: unknown) { + expect((atom as ReactiveNode).subs).toBeUndefined() + expect((atom as ReactiveNode).subsTail).toBeUndefined() +} + +describe('Unobserved computed atoms', () => { + test('reading an atom does not leave references from its source', () => { + const source = createAtom(0) + const derived = createAtom(() => source.get() + 1) + + expect(derived.get()).toBe(1) + expectUnwatched(source) + expectUnwatched(derived) + }) + + test('caches snapshots until a dependency changes', () => { + const source = createAtom(0) + const unrelated = createAtom(0) + const getter = vi.fn(() => ({ value: source.get() })) + const derived = createAtom(getter) + const snapshot = derived.get() + + unrelated.set(1) + expect(derived.get()).toBe(snapshot) + source.set(0) + expect(derived.get()).toBe(snapshot) + expect(getter).toHaveBeenCalledTimes(1) + source.set(1) + expect(derived.get()).toEqual({ value: 1 }) + expect(getter).toHaveBeenCalledTimes(2) + expectUnwatched(source) + }) + + test('validates chains without recomputing unchanged intermediate values', () => { + const source = createAtom(0) + const parity = createAtom(() => source.get() % 2) + const getter = vi.fn(() => ({ parity: parity.get() })) + const derived = createAtom(getter) + const snapshot = derived.get() + + source.set(2) + expect(derived.get()).toBe(snapshot) + expect(getter).toHaveBeenCalledTimes(1) + source.set(3) + expect(derived.get()).toEqual({ parity: 1 }) + expectUnwatched(source) + expectUnwatched(parity) + }) + + test('preserves custom comparisons and previous snapshots', () => { + const source = createAtom(0) + const getter = vi.fn((prev?: { parity: number }) => { + return { parity: source.get() % 2, prev } + }) + const derived = createAtom(getter, { + compare: (a, b) => a.parity === b.parity, + }) + const snapshot = derived.get() + + source.set(2) + expect(derived.get()).toBe(snapshot) + source.set(3) + expect(derived.get()).toEqual({ parity: 1, prev: snapshot }) + expect(getter).toHaveBeenLastCalledWith(snapshot) + expectUnwatched(source) + }) + + test('validates both branches of a diamond before and after subscribing', () => { + const source = createAtom(1) + const left = createAtom(() => source.get() * 2) + const right = createAtom(() => source.get() * 3) + const derived = createAtom(() => left.get() + right.get()) + + expect(derived.get()).toBe(5) + source.set(2) + expect(derived.get()).toBe(10) + expectUnwatched(source) + const observer = vi.fn() + const subscription = derived.subscribe(observer) + source.set(3) + expect(observer.mock.calls).toEqual([[15]]) + subscription.unsubscribe() + expectUnwatched(source) + }) + + test('subscribes to a previously read chain without replacing its snapshot', () => { + const source = createAtom(0) + const intermediate = createAtom(() => source.get() + 1) + const getter = vi.fn(() => ({ value: intermediate.get() })) + const derived = createAtom(getter) + const snapshot = derived.get() + const observer = vi.fn() + const subscription = derived.subscribe(observer) + + expect(derived.get()).toBe(snapshot) + expect(getter).toHaveBeenCalledTimes(1) + source.set(1) + expect(observer).toHaveBeenLastCalledWith({ value: 2 }) + subscription.unsubscribe() + expectUnwatched(source) + expectUnwatched(intermediate) + source.set(2) + expect(derived.get()).toEqual({ value: 3 }) + expectUnwatched(source) + + const secondSubscription = derived.subscribe(observer) + source.set(3) + expect(observer).toHaveBeenLastCalledWith({ value: 4 }) + secondSubscription.unsubscribe() + expectUnwatched(source) + }) + + test('tracks changed conditional dependencies when subscribing after a read', () => { + const condition = createAtom(true) + const left = createAtom(1) + const right = createAtom(2) + const derived = createAtom(() => + condition.get() ? left.get() : right.get(), + ) + expect(derived.get()).toBe(1) + condition.set(false) + const observer = vi.fn() + const subscription = derived.subscribe(observer) + + expect(derived.get()).toBe(2) + expectUnwatched(left) + left.set(3) + expect(observer).not.toHaveBeenCalled() + right.set(4) + expect(observer).toHaveBeenLastCalledWith(4) + subscription.unsubscribe() + expectUnwatched(condition) + expectUnwatched(right) + }) + + test('does not retain an unobserved atom through a subscribed dependency', () => { + const source = createAtom(0) + const intermediate = createAtom(() => source.get() + 1) + const observer = vi.fn() + const subscription = intermediate.subscribe(observer) + const derived = createAtom(() => intermediate.get() + 1) + + expect(derived.get()).toBe(2) + expect( + (intermediate as unknown as ReactiveNode).subs?.nextSub, + ).toBeUndefined() + source.set(1) + expect(observer).toHaveBeenLastCalledWith(2) + expect(derived.get()).toBe(3) + subscription.unsubscribe() + expectUnwatched(source) + }) + + test('preserves pending changes when unsubscribing during a batch', () => { + const source = createAtom(0) + const intermediate = createAtom(() => source.get() + 1) + const derived = createAtom(() => intermediate.get() + 1) + const subscription = derived.subscribe(() => {}) + + batch(() => { + source.set(1) + subscription.unsubscribe() + expect(derived.get()).toBe(3) + }) + expectUnwatched(source) + }) + + test('releases dependencies when a nested getter throws and can retry', () => { + const source = createAtom(0) + const intermediate = createAtom(() => { + if (source.get() === 0) throw new Error('not ready') + return source.get() + }) + const derived = createAtom(() => intermediate.get() + 1) + + expect(() => derived.get()).toThrow('not ready') + expectUnwatched(source) + source.set(1) + expect(derived.get()).toBe(2) + expectUnwatched(source) + }) + + test('retries after a cached dependency throws while subscribing', () => { + const stable = createAtom(0) + const source = createAtom(1) + const intermediate = createAtom(() => { + if (source.get() === 0) throw new Error('not ready') + return source.get() + }) + const derived = createAtom(() => stable.get() + intermediate.get()) + + expect(derived.get()).toBe(1) + source.set(0) + expect(() => derived.subscribe(() => {})).toThrow('not ready') + expectUnwatched(stable) + expectUnwatched(source) + source.set(2) + expect(derived.get()).toBe(2) + expectUnwatched(source) + }) + + test('reconnects dirty dependencies even when their value stays equal', () => { + const source = createAtom(0) + const parity = createAtom(() => source.get() % 2) + const derived = createAtom(() => ({ parity: parity.get() })) + const subscription = derived.subscribe(() => {}) + + batch(() => { + source.set(2) + subscription.unsubscribe() + }) + const observer = vi.fn() + const nextSubscription = derived.subscribe(observer) + expect(derived.get()).toEqual({ parity: 0 }) + source.set(3) + expect(observer.mock.calls).toEqual([[{ parity: 1 }]]) + nextSubscription.unsubscribe() + expectUnwatched(source) + }) + + test('detects writes made inside an unobserved getter', () => { + const source = createAtom(0) + const derived = createAtom(() => { + const value = source.get() + if (value === 1) source.set(2) + return value + }) + + expect(derived.get()).toBe(0) + source.set(1) + expect(derived.get()).toBe(1) + expect(derived.get()).toBe(2) + expectUnwatched(source) + }) + + test('preserves constant snapshots across writes and subscriptions', () => { + const unrelated = createAtom(0) + const getter = vi.fn(() => ({})) + const derived = createAtom(getter) + const snapshot = derived.get() + + unrelated.set(1) + expect(derived.get()).toBe(snapshot) + const subscription = derived.subscribe(() => {}) + expect(derived.get()).toBe(snapshot) + expect(getter).toHaveBeenCalledTimes(1) + subscription.unsubscribe() + }) + + test('observes async completion without restarting a cached request', async () => { + const unrelated = createAtom(0) + const request = vi.fn(() => Promise.resolve(42)) + const asyncAtom = createAsyncAtom(request) + const derived = createAtom(() => asyncAtom.get().status) + + expect(derived.get()).toBe('pending') + await Promise.resolve() + expect(derived.get()).toBe('done') + unrelated.set(1) + const subscription = derived.subscribe(() => {}) + expect(derived.get()).toBe('done') + expect(asyncAtom.get()).toEqual({ status: 'done', data: 42 }) + expect(request).toHaveBeenCalledTimes(1) + subscription.unsubscribe() + expectUnwatched(asyncAtom) + }) +}) From a7cc518cdbbd9ae7d6c60ed579bfaec42c7bce34 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 21:25:35 +0000 Subject: [PATCH 2/2] ci: apply automated fixes and generate docs --- docs/reference/functions/batch.md | 2 +- docs/reference/functions/createAsyncAtom.md | 2 +- docs/reference/functions/createAtom.md | 4 ++-- docs/reference/functions/flush.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/reference/functions/batch.md b/docs/reference/functions/batch.md index 43c3f38e..b3a52a7b 100644 --- a/docs/reference/functions/batch.md +++ b/docs/reference/functions/batch.md @@ -7,7 +7,7 @@ title: batch function batch(fn): void; ``` -Defined in: [atom.ts:70](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L70) +Defined in: [atom.ts:67](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L67) ## Parameters diff --git a/docs/reference/functions/createAsyncAtom.md b/docs/reference/functions/createAsyncAtom.md index 4bf0c602..9e7f3251 100644 --- a/docs/reference/functions/createAsyncAtom.md +++ b/docs/reference/functions/createAsyncAtom.md @@ -7,7 +7,7 @@ title: createAsyncAtom function createAsyncAtom(getValue, options?): ReadonlyAtom>; ``` -Defined in: [atom.ts:108](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L108) +Defined in: [atom.ts:121](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L121) ## Type Parameters diff --git a/docs/reference/functions/createAtom.md b/docs/reference/functions/createAtom.md index eed96287..897837f3 100644 --- a/docs/reference/functions/createAtom.md +++ b/docs/reference/functions/createAtom.md @@ -9,7 +9,7 @@ title: createAtom function createAtom(getValue, options?): ReadonlyAtom; ``` -Defined in: [atom.ts:146](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L146) +Defined in: [atom.ts:159](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L159) ### Type Parameters @@ -37,7 +37,7 @@ Defined in: [atom.ts:146](https://github.com/TanStack/store/blob/main/packages/s function createAtom(initialValue, options?): Atom; ``` -Defined in: [atom.ts:150](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L150) +Defined in: [atom.ts:163](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L163) ### Type Parameters diff --git a/docs/reference/functions/flush.md b/docs/reference/functions/flush.md index 4e1ef6c9..ac7f2c34 100644 --- a/docs/reference/functions/flush.md +++ b/docs/reference/functions/flush.md @@ -7,7 +7,7 @@ title: flush function flush(): void; ``` -Defined in: [atom.ts:89](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L89) +Defined in: [atom.ts:102](https://github.com/TanStack/store/blob/main/packages/store/src/atom.ts#L102) ## Returns