From f6f40360a2dc16d337d70b7b8ffe14a89c7a2218 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 21:31:58 +0000 Subject: [PATCH] Defer ReactiveNode hub teardown across React resubscribes When an inline useSelect selector's sources move with a prop, the render-phase recompute builds a new store handle and React swaps the subscription on commit by unsubscribing the old store before subscribing the new one. A ReactiveNode both stores held briefly had no hub listener, so it re-ran onUnobserved and onObserved, which costs query nodes a backend resubscribe. The subscription hub now defers a ReactiveNode hub's teardown to a microtask and keeps it if a listener resubscribes first. Hubs for other nodes still tear down synchronously. Claude-Session: https://claude.ai/code/session_01Soy3br8g2C5cCzNF5H54DF --- .../keep-observed-across-resubscribe.md | 7 ++++ .../src/internals/subscriptionHub.ts | 39 +++++++++++++++++-- packages/retree-react/src/useSelect.spec.tsx | 25 +++++++++++- .../04.convex-react-nextjs/app/page.spec.tsx | 1 + 4 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 .changeset/keep-observed-across-resubscribe.md diff --git a/.changeset/keep-observed-across-resubscribe.md b/.changeset/keep-observed-across-resubscribe.md new file mode 100644 index 0000000..0bb9a5b --- /dev/null +++ b/.changeset/keep-observed-across-resubscribe.md @@ -0,0 +1,7 @@ +--- +"@retreejs/react": patch +--- + +`useSelect` no longer re-runs `onUnobserved` and `onObserved` on a `ReactiveNode` it keeps reading when a prop moves the selector's other dependencies. Query nodes such as `ConvexQueryNode` no longer close and reopen their backend subscription on those re-renders. + +React hooks now release a `ReactiveNode` one microtask after its last listener unsubscribes, so `onUnobserved` runs just after unmount rather than during it. diff --git a/packages/retree-react/src/internals/subscriptionHub.ts b/packages/retree-react/src/internals/subscriptionHub.ts index f2fc9c1..ea48228 100644 --- a/packages/retree-react/src/internals/subscriptionHub.ts +++ b/packages/retree-react/src/internals/subscriptionHub.ts @@ -5,6 +5,7 @@ import { INodeFieldChanges, + ReactiveNode, Retree, TRetreeChangedEvents, TreeNode, @@ -38,6 +39,8 @@ interface SubscriptionHubEntry { */ listeners: Map, number>; unsubscribeRetree: () => void; + /** A `ReactiveNode` hub lost its last listener and tears down next microtask. */ + teardownQueued: boolean; } const subscriptionHubs: WeakMap< @@ -89,6 +92,7 @@ export function subscribeToNode( hub = { listeners, unsubscribeRetree, + teardownQueued: false, }; nodeHubs.set(listenerType, hub as SubscriptionHubEntry); } @@ -117,10 +121,37 @@ export function subscribeToNode( if (hub.listeners.size > 0) { return; } - hub.unsubscribeRetree(); - nodeHubs.delete(listenerType); - if (nodeHubs.size === 0) { - subscriptionHubs.delete(baseProxy); + if (!(baseProxy instanceof ReactiveNode)) { + teardownHub(baseProxy, nodeHubs, listenerType, hub); + return; + } + // React swaps a moved store's subscription by unsubscribing the old + // one before subscribing the new one in the same commit. Deferring + // lets a resubscribe adopt this hub, so a node both stores hold keeps + // its listener instead of re-running `onUnobserved`/`onObserved`. + if (hub.teardownQueued) { + return; } + hub.teardownQueued = true; + queueMicrotask(() => { + hub.teardownQueued = false; + if (hub.listeners.size > 0) { + return; + } + teardownHub(baseProxy, nodeHubs, listenerType, hub); + }); }; } + +function teardownHub( + baseProxy: T, + nodeHubs: Map, + listenerType: RetreeStoreListenerType, + hub: SubscriptionHubEntry +): void { + hub.unsubscribeRetree(); + nodeHubs.delete(listenerType); + if (nodeHubs.size === 0) { + subscriptionHubs.delete(baseProxy); + } +} diff --git a/packages/retree-react/src/useSelect.spec.tsx b/packages/retree-react/src/useSelect.spec.tsx index a74103a..5a41879 100644 --- a/packages/retree-react/src/useSelect.spec.tsx +++ b/packages/retree-react/src/useSelect.spec.tsx @@ -1250,7 +1250,7 @@ describe("useSelect observation", () => { } } - it("observes a node it reads through its owner", () => { + it("observes a node it reads through its owner", async () => { vi.spyOn(console, "warn").mockImplementation(() => undefined); const owner = trackRoot(Retree.root(new Owner())); let renders = 0; @@ -1270,6 +1270,8 @@ describe("useSelect observation", () => { expect(renders).toBe(2); view.unmount(); + expect(query.unobserved).toBe(0); + await Promise.resolve(); expect(query.observed).toBe(1); expect(query.unobserved).toBe(1); }); @@ -1300,4 +1302,25 @@ describe("useSelect observation", () => { expect(query.observed).toBe(1); expect(query.unobserved).toBe(0); }); + + it("keeps observing a node when a prop moves its other dependencies", async () => { + vi.spyOn(console, "warn").mockImplementation(() => undefined); + const owner = trackRoot(Retree.root(new Owner())); + const other = trackRoot(Retree.root({ value: 1 })); + function View({ on }: { on: boolean }) { + const value = useSelect( + () => `${owner.queryFor("a").state}:${on ? other.value : 0}` + ); + return {value}; + } + const view = render(); + const query = owner.queries.a; + + view.rerender(); + view.rerender(); + await Promise.resolve(); + + expect(query.observed).toBe(1); + expect(query.unobserved).toBe(0); + }); }); diff --git a/samples/04.convex-react-nextjs/app/page.spec.tsx b/samples/04.convex-react-nextjs/app/page.spec.tsx index 873b50c..d0af505 100644 --- a/samples/04.convex-react-nextjs/app/page.spec.tsx +++ b/samples/04.convex-react-nextjs/app/page.spec.tsx @@ -128,6 +128,7 @@ describe("compiled React sample", () => { }); await screen.findByLabelText("Edit Updated task"); view.unmount(); + await Promise.resolve(); expect(unsubscribe).toHaveBeenCalled(); }); });