From a96461622dbb16802b3fdf6979372232e809be4d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 13:52:19 +0000 Subject: [PATCH] Observe ReactiveNodes a tracked run reads through a parent (#123) Tracked selectors subscribe only to their read covers, nodes whose parent the run did not read. A ReactiveNode reached through a read parent therefore had no listener of its own and never got onObserved, whether it existed before the run or was created during it. Query nodes created lazily inside a selector never subscribed. Covered ReactiveNodes now become observe-only sources. Retree.select and Retree.effect hold a no-op nodeChanged listener on them, and useSelect holds an observed hub entry that never notifies, so changes still arrive once through the cover. Observation makes synchronous onObserved writes common, so two re-entrancy gaps are closed too: a tracked select re-runs after its subscriptions settle instead of replacing them mid-update, and useSelect subscribes moved sources before releasing the old ones so a kept node never loses its last listener. Sources are now resolved lazily, so @select getter runs skip the cover pass. Claude-Session: https://claude.ai/code/session_01Us9f7SPLJUYScpdDnA8oM4 --- .changeset/observe-covered-reactive-nodes.md | 8 ++ packages/retree-core/src/ReactiveNode.spec.ts | 82 +++++++++++++++++++ .../src/internals/dependency-tracking.ts | 29 ++++++- packages/retree-core/src/internals/select.ts | 53 ++++++++++-- .../src/internals/externalStore.ts | 14 +++- .../src/internals/subscriptionHub.ts | 12 ++- packages/retree-react/src/useSelect.spec.tsx | 82 +++++++++++++++++++ packages/retree-react/src/useSelect.ts | 13 ++- 8 files changed, 272 insertions(+), 21 deletions(-) create mode 100644 .changeset/observe-covered-reactive-nodes.md diff --git a/.changeset/observe-covered-reactive-nodes.md b/.changeset/observe-covered-reactive-nodes.md new file mode 100644 index 0000000..c2693f0 --- /dev/null +++ b/.changeset/observe-covered-reactive-nodes.md @@ -0,0 +1,8 @@ +--- +"@retreejs/core": patch +"@retreejs/react": patch +--- + +Selector-only `Retree.select(selector, callback)`, `Retree.effect`, and `useSelect` now observe every `ReactiveNode` they read, so `onObserved` and `onUnobserved` run for it. + +Before, a node reached through a parent the selector also read was never observed, whether it existed before the run or was created during it. A query node created lazily inside a selector, such as `vm.snapshotsFor(id).state`, never subscribed and stayed loading forever. A selector also lost a dependency when `onObserved` wrote state while it subscribed, and `useSelect` re-ran a kept node's `onUnobserved` and `onObserved` when its other dependencies moved. diff --git a/packages/retree-core/src/ReactiveNode.spec.ts b/packages/retree-core/src/ReactiveNode.spec.ts index 44409c7..f6fc567 100644 --- a/packages/retree-core/src/ReactiveNode.spec.ts +++ b/packages/retree-core/src/ReactiveNode.spec.ts @@ -1002,3 +1002,85 @@ it("retains active dependency records while refreshing values and removing stale expect(changed).not.toHaveBeenCalled(); off(); }); + +describe("ReactiveNode observation through tracked reads", () => { + class Query extends ReactiveNode { + public state: number | undefined = undefined; + public observed = 0; + public unobserved = 0; + get dependencies() { + return []; + } + protected onObserved(): void { + this.observed++; + } + protected onUnobserved(): void { + this.unobserved++; + } + } + class Owner extends ReactiveNode { + public queries: Record = {}; + get dependencies() { + return []; + } + queryFor(id: string): Query { + const existing = this.queries[id]; + if (existing !== undefined) return existing; + Retree.runSilent(() => { + this.queries[id] = new Query(); + }); + return this.queries[id]; + } + } + type TTrack = (read: () => unknown) => () => void; + const select: TTrack = (read) => Retree.select(read, () => {}); + const effect: TTrack = (read) => Retree.effect(() => void read()); + + it.each([ + ["Retree.select", "created during the run", select, false], + ["Retree.effect", "created during the run", effect, false], + ["Retree.select", "read through its owner", select, true], + ])("%s observes a node %s", (_, __, track, createFirst) => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const owner = trackRoot(Retree.root(new Owner())); + if (createFirst) owner.queryFor("a"); + const seen: unknown[] = []; + const stop = track(() => seen.push(owner.queryFor("a").state)); + const query = owner.queries.a; + + query.state = 5; + expect(seen.at(-1)).toBe(5); + expect(query.observed).toBe(1); + stop(); + + expect(query.observed).toBe(1); + expect(query.unobserved).toBe(1); + }); + + it("keeps every dependency when onObserved writes while subscribing", () => { + class Cached extends ReactiveNode { + public state: number | undefined = undefined; + get dependencies() { + return []; + } + protected onObserved(): void { + this.state = 5; + } + } + const owner = trackRoot(Retree.root({ query: new Cached() })); + const other = trackRoot(Retree.root({ value: 1 })); + const seen: string[] = []; + const stop = Retree.select( + () => + owner.query.state === undefined + ? "loading" + : `${owner.query.state}:${other.value}`, + (next) => seen.push(next) + ); + + other.value = 2; + stop(); + + expect(seen).toEqual(["5:1", "5:2"]); + }); +}); diff --git a/packages/retree-core/src/internals/dependency-tracking.ts b/packages/retree-core/src/internals/dependency-tracking.ts index 0e663b5..5b19456 100644 --- a/packages/retree-core/src/internals/dependency-tracking.ts +++ b/packages/retree-core/src/internals/dependency-tracking.ts @@ -718,11 +718,17 @@ export enum DependencySubscriptionKind { Node, /** The node and every descendant, delivered per changed node. */ Subtree, + /** + * A `ReactiveNode` read through a cover: held only to run its observed + * lifecycle, since the cover already delivers its changes. + */ + Observe, } /** * One node a tracked run must subscribe to: a cover whose subtree holds - * every other node the run read through it. + * every other node the run read through it, or a covered `ReactiveNode` the + * run observes. */ export interface ITrackedDependencySource { rawNode: TreeNode; @@ -741,9 +747,11 @@ export interface ITrackedSelectionAccesses { /** * The covers of the run's reads: nodes whose parent the run did not * read. A cover with covered children subscribes to its subtree; a - * lone node subscribes to `nodeChanged`. + * lone node subscribes to `nodeChanged`. Covered `ReactiveNode`s follow + * as observe-only sources. Resolved on first access: `@select` + * collection never subscribes. */ - sources: readonly ITrackedDependencySource[]; + readonly sources: readonly ITrackedDependencySource[]; /** Every node the run read, keyed by raw node, in first-read order. */ reads: ReadonlyMap; /** @@ -763,6 +771,7 @@ export interface ITrackedSelectionAccesses { /** * Resolve which records subscribe. A record created before its parent was * read is re-checked here; the rest were settled as they were created. + * Covered `ReactiveNode`s are observed too, so their `onObserved` runs. */ function resolveCovers( frame: DependencyAccessFrame, @@ -788,6 +797,15 @@ function resolveCovers( : DependencySubscriptionKind.Node, }); } + for (const record of reads.values()) { + if (!record.covered) continue; + if (!(record.rawNode instanceof ReactiveNode)) continue; + covers.push({ + rawNode: record.rawNode, + baseProxy: record.baseProxy, + kind: DependencySubscriptionKind.Observe, + }); + } return covers; } @@ -801,11 +819,14 @@ export function collectTrackedSelectionAccesses( ): ITrackedSelectionAccesses { const { value, frame } = runDependenciesFrame(callback); const reads = frame.reads ?? new Map(); + let sources: readonly ITrackedDependencySource[] | undefined; return { value, coverage: frame.coverage, getDependencies: () => toDependencyValues(reads), - sources: resolveCovers(frame, reads), + get sources() { + return (sources ??= resolveCovers(frame, reads)); + }, reads, subtreeReads: frame.subtreeReads ?? [], writeInvalidatedReads: frame.writeInvalidatedReads ?? [], diff --git a/packages/retree-core/src/internals/select.ts b/packages/retree-core/src/internals/select.ts index 84e4a46..af2b339 100644 --- a/packages/retree-core/src/internals/select.ts +++ b/packages/retree-core/src/internals/select.ts @@ -87,9 +87,10 @@ interface DependencySubscription { } /** - * The subscriptions a selector or effect holds on the covers of its last - * run. `update` diffs against the previous run so unchanged covers keep - * their subscription and only added, removed or re-kinded covers churn. + * The subscriptions a selector or effect holds on the sources of its last + * run: its covers, plus the covered `ReactiveNode`s it observes. `update` + * diffs against the previous run so unchanged sources keep their + * subscription and only added, removed or re-kinded sources churn. */ function createDependencySubscriptionSet( subscribeToNode: SubscribeToNode, @@ -104,6 +105,17 @@ function createDependencySubscriptionSet( source: ITrackedDependencySource ): DependencySubscription => { const rawNode = source.rawNode; + if (source.kind === DependencySubscriptionKind.Observe) { + // The node's cover delivers its changes; this only observes it. + return { + kind: source.kind, + unsubscribe: subscribeToNode( + source.baseProxy, + "nodeChanged", + () => undefined + ), + }; + } if (source.kind === DependencySubscriptionKind.Node) { return { kind: source.kind, @@ -448,11 +460,30 @@ export function createRetreeTrackedSelectionObserver(options: { options.subscribeToSubtree, (rawNode, changes) => evaluateForDependency(rawNode, changes) ); + // Subscribing can run `onObserved`, which may write a dependency. A + // nested evaluate would replace the subscriptions mid-update, so changes + // during an update re-run once it settles. + let updating = false; + let changedDuringUpdate = false; + const updateSubscriptions = ( + sources: readonly ITrackedDependencySource[] + ) => { + updating = true; + try { + subscriptions.update(sources); + } finally { + updating = false; + } + }; const evaluateForDependency = ( changedRawNode: TreeNode, changes?: INodeFieldChanges[] ) => { + if (updating) { + changedDuringUpdate = true; + return; + } // Subtree subscriptions deliver every descendant emission; a node // the run never read cannot change what it selected. const record = resolveChangedReadRecord( @@ -475,7 +506,7 @@ export function createRetreeTrackedSelectionObserver(options: { previous.selected, next.selected ); - subscriptions.update(next.sources); + updateSubscriptions(next.sources); const changed = hasTrackedSelectionChanged( previous, { selected: nextSelected, reads: next.reads }, @@ -488,13 +519,19 @@ export function createRetreeTrackedSelectionObserver(options: { reads: next.reads, subtreeReads: next.subtreeReads, }; - if (!changed) { - return; + if (changed) { + options.onChange(nextSelected, previousToEmit.selected); } - options.onChange(nextSelected, previousToEmit.selected); + rerunIfChangedDuringUpdate(); + }; + const rerunIfChangedDuringUpdate = () => { + if (!changedDuringUpdate) return; + changedDuringUpdate = false; + evaluate(); }; - subscriptions.update(previous.sources); + updateSubscriptions(previous.sources); + rerunIfChangedDuringUpdate(); return () => subscriptions.dispose(); } diff --git a/packages/retree-react/src/internals/externalStore.ts b/packages/retree-react/src/internals/externalStore.ts index 911d616..bdb6be4 100644 --- a/packages/retree-react/src/internals/externalStore.ts +++ b/packages/retree-react/src/internals/externalStore.ts @@ -98,6 +98,10 @@ export function getRetreeExternalStoreSource( baseProxy, listenerType, getVersion() { + // An observed source never notifies, so its version never moves. + if (listenerType === "observed") { + return 0; + } // A subtree source moves with any descendant, like a tree source. if (listenerType === "nodeChanged") { return getNodeSnapshotVersion(baseProxy); @@ -285,14 +289,16 @@ export function createRetreeSwappableCompositeExternalStore( }, swapSources: (nextSources) => { current = createRetreeCompositeExternalStore(nextSources); - // Rewire synchronously: no other code runs between the old - // unsubscribe and the new subscribe, so the swap cannot miss a - // write. A swap with no live wirings (before mount, after + // Rewire synchronously so the swap cannot miss a write, and + // subscribe before unsubscribing so a node both lists hold never + // loses its last listener (which would re-run its observed + // lifecycle). A swap with no live wirings (before mount, after // unmount) only replaces the inner store; `subscribe` reads // `current` at call time. for (const wiring of wirings) { - wiring.unsubscribe(); + const unsubscribePrevious = wiring.unsubscribe; wiring.unsubscribe = current.subscribe(wiring.onStoreChange); + unsubscribePrevious(); } }, }; diff --git a/packages/retree-react/src/internals/subscriptionHub.ts b/packages/retree-react/src/internals/subscriptionHub.ts index ef63e7e..f2fc9c1 100644 --- a/packages/retree-react/src/internals/subscriptionHub.ts +++ b/packages/retree-react/src/internals/subscriptionHub.ts @@ -13,9 +13,13 @@ import { SUBSCRIBE_SUBTREE_CHANGED_SYMBOL } from "@retreejs/core/internal"; /** * Listener kinds a hook can hold: the public change events plus the internal - * subtree subscription tracked `useSelect` uses for its read covers. + * subtree subscription tracked `useSelect` uses for its read covers, and the + * observe-only subscription it holds on `ReactiveNode`s read through them. */ -export type RetreeStoreListenerType = TRetreeChangedEvents | "subtreeChanged"; +export type RetreeStoreListenerType = + | TRetreeChangedEvents + | "subtreeChanged" + | "observed"; /** * For `nodeChanged`/`treeChanged` the first argument is the node's reproxy; @@ -46,6 +50,10 @@ function subscribeRetree( listenerType: RetreeStoreListenerType, notify: (node: T, changes: INodeFieldChanges[]) => void ): () => void { + if (listenerType === "observed") { + // Runs the node's observed lifecycle; its cover delivers changes. + return Retree.on(baseProxy, "nodeChanged", () => undefined); + } if (listenerType === "subtreeChanged") { return Retree[SUBSCRIBE_SUBTREE_CHANGED_SYMBOL]( baseProxy, diff --git a/packages/retree-react/src/useSelect.spec.tsx b/packages/retree-react/src/useSelect.spec.tsx index 681ea30..a74103a 100644 --- a/packages/retree-react/src/useSelect.spec.tsx +++ b/packages/retree-react/src/useSelect.spec.tsx @@ -1219,3 +1219,85 @@ describe("useSelect", () => { consoleError.mockRestore(); }); }); + +describe("useSelect observation", () => { + class Query extends ReactiveNode { + public state: number | undefined = undefined; + public observed = 0; + public unobserved = 0; + get dependencies() { + return []; + } + protected onObserved(): void { + this.observed++; + } + protected onUnobserved(): void { + this.unobserved++; + } + } + class Owner extends ReactiveNode { + public queries: Record = {}; + get dependencies() { + return []; + } + queryFor(id: string): Query { + const existing = this.queries[id]; + if (existing !== undefined) return existing; + Retree.runSilent(() => { + this.queries[id] = new Query(); + }); + return this.queries[id]; + } + } + + it("observes a node it reads through its owner", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const owner = trackRoot(Retree.root(new Owner())); + let renders = 0; + function View() { + renders++; + const state = useSelect(() => owner.queryFor("a").state); + return {String(state)}; + } + const view = render(); + const query = owner.queries.a; + expect(query.observed).toBe(1); + + act(() => { + query.state = 5; + }); + expect(screen.getByTestId("state").textContent).toBe("5"); + expect(renders).toBe(2); + + view.unmount(); + expect(query.observed).toBe(1); + expect(query.unobserved).toBe(1); + }); + + it("keeps observing a node while its other dependencies move", () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const owner = trackRoot(Retree.root(new Owner())); + const flag = trackRoot(Retree.root({ on: false, inner: { value: 1 } })); + function View() { + const value = useSelect( + () => + `${owner.queryFor("a").state}:${ + flag.on ? flag.inner.value : 0 + }` + ); + return {value}; + } + render(); + const query = owner.queries.a; + + act(() => { + flag.on = true; + }); + act(() => { + flag.on = false; + }); + + expect(query.observed).toBe(1); + expect(query.unobserved).toBe(0); + }); +}); diff --git a/packages/retree-react/src/useSelect.ts b/packages/retree-react/src/useSelect.ts index 86723c9..4e37774 100644 --- a/packages/retree-react/src/useSelect.ts +++ b/packages/retree-react/src/useSelect.ts @@ -37,6 +37,7 @@ import { RetreeSwappableCompositeExternalStore, } from "./internals/externalStore.js"; import { useNodeFactoryResetWarning } from "./internals/factoryWarning.js"; +import { RetreeStoreListenerType } from "./internals/subscriptionHub.js"; import { NodeFactory } from "./types.js"; export type UseSelectOptions = RetreeSelectOptions; @@ -173,15 +174,21 @@ function getTrackedSelectionSources( sources.push( getRetreeExternalStoreSource( source.baseProxy, - source.kind === DependencySubscriptionKind.Subtree - ? "subtreeChanged" - : "nodeChanged" + getTrackedListenerType(source.kind) ) ); } return sources; } +function getTrackedListenerType( + kind: DependencySubscriptionKind +): RetreeStoreListenerType { + if (kind === DependencySubscriptionKind.Subtree) return "subtreeChanged"; + if (kind === DependencySubscriptionKind.Observe) return "observed"; + return "nodeChanged"; +} + /** * True when a moved source can still affect the selection. With the changed * nodes known, only their records validate (nodes without a record are