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