From a662c46f1acf4025a548e36904494aef4f6e0bb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Oct 2025 15:05:03 +0000 Subject: [PATCH 1/2] fix(collection): fire status:change event before cleaning up event handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Event handlers are now cleaned up after the status is changed to 'cleaned-up', allowing status:change listeners to properly detect the cleaned-up state. The cleanup process now: 1. Cleans up sync, state, changes, and indexes 2. Sets status to 'cleaned-up' (fires the event) 3. Finally cleans up event handlers This fixes the collection factory pattern where collections listen for the 'cleaned-up' status to remove themselves from the cache. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .changeset/fix-collection-cleanup-order.md | 33 +++++++++++++ packages/db/src/collection/lifecycle.ts | 10 ++-- .../db/tests/collection-lifecycle.test.ts | 47 +++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-collection-cleanup-order.md diff --git a/.changeset/fix-collection-cleanup-order.md b/.changeset/fix-collection-cleanup-order.md new file mode 100644 index 000000000..f0c132eee --- /dev/null +++ b/.changeset/fix-collection-cleanup-order.md @@ -0,0 +1,33 @@ +--- +"@tanstack/db": patch +--- + +Fix collection cleanup to fire status:change event with 'cleaned-up' status + +Previously, when a collection was garbage collected, event handlers were removed before the status was changed to 'cleaned-up'. This prevented listeners from receiving the status:change event, breaking the collection factory pattern where collections listen for cleanup to remove themselves from a cache. + +Now, the cleanup process: +1. Cleans up sync, state, changes, and indexes +2. Sets status to 'cleaned-up' (fires the event) +3. Finally cleans up event handlers + +This enables the collection factory pattern: + +```typescript +const cache = new Map>(); + +const getTodoCollection = (id: string) => { + if (!cache.has(id)) { + const collection = createCollection(/* ... */); + + collection.on('status:change', ({ status }) => { + if (status === 'cleaned-up') { + cache.delete(id); // This now works! + } + }); + + cache.set(id, collection); + } + return cache.get(id)!; +}; +``` diff --git a/packages/db/src/collection/lifecycle.ts b/packages/db/src/collection/lifecycle.ts index be607a3c9..5181ab216 100644 --- a/packages/db/src/collection/lifecycle.ts +++ b/packages/db/src/collection/lifecycle.ts @@ -250,8 +250,7 @@ export class CollectionLifecycleManager< !deadline || deadline.timeRemaining() > 0 || deadline.didTimeout if (hasTime) { - // Perform all cleanup operations - this.events.cleanup() + // Perform all cleanup operations except events this.sync.cleanup() this.state.cleanup() this.changes.cleanup() @@ -265,8 +264,13 @@ export class CollectionLifecycleManager< this.hasBeenReady = false this.onFirstReadyCallbacks = [] - // Set status to cleaned-up + // Set status to cleaned-up after everything is cleaned up + // This fires the status:change event to notify listeners this.setStatus(`cleaned-up`) + + // Finally, cleanup event handlers after the event has been fired + this.events.cleanup() + return true } else { // If we don't have time, reschedule for the next idle period diff --git a/packages/db/tests/collection-lifecycle.test.ts b/packages/db/tests/collection-lifecycle.test.ts index 5af642c08..4094ee813 100644 --- a/packages/db/tests/collection-lifecycle.test.ts +++ b/packages/db/tests/collection-lifecycle.test.ts @@ -474,5 +474,52 @@ describe(`Collection Lifecycle Management`, () => { subscription.unsubscribe() }) + + it(`should fire status:change event with 'cleaned-up' status before clearing event handlers`, () => { + const collection = createCollection<{ id: string; name: string }>({ + id: `cleanup-event-test`, + getKey: (item) => item.id, + gcTime: 1000, + sync: { + sync: () => {}, + }, + }) + + // Track status changes + const statusChanges: Array<{ status: string; previousStatus: string }> = + [] + + // Add event listener for status changes + collection.on(`status:change`, ({ status, previousStatus }) => { + statusChanges.push({ status, previousStatus }) + }) + + // Subscribe and unsubscribe to trigger GC + const subscription = collection.subscribeChanges(() => {}) + subscription.unsubscribe() + + expect(statusChanges).toHaveLength(1) + expect(statusChanges[0]).toEqual({ + status: `loading`, + previousStatus: `idle`, + }) + + // Trigger GC timeout to schedule cleanup + const gcTimerId = mockSetTimeout.mock.results[0]?.value + if (gcTimerId) { + triggerTimeout(gcTimerId) + } + + // Trigger all remaining timeouts to handle the idle callback + triggerAllTimeouts() + + // Verify that the listener received the 'cleaned-up' status change event + expect(statusChanges).toHaveLength(2) + expect(statusChanges[1]).toEqual({ + status: `cleaned-up`, + previousStatus: `loading`, + }) + expect(collection.status).toBe(`cleaned-up`) + }) }) }) From 59d67e1810f8aeab58a0209862344a77fb78ee5e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Oct 2025 01:33:37 +0000 Subject: [PATCH 2/2] style: format changeset with prettier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .changeset/fix-collection-cleanup-order.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.changeset/fix-collection-cleanup-order.md b/.changeset/fix-collection-cleanup-order.md index f0c132eee..f81aa84d2 100644 --- a/.changeset/fix-collection-cleanup-order.md +++ b/.changeset/fix-collection-cleanup-order.md @@ -7,6 +7,7 @@ Fix collection cleanup to fire status:change event with 'cleaned-up' status Previously, when a collection was garbage collected, event handlers were removed before the status was changed to 'cleaned-up'. This prevented listeners from receiving the status:change event, breaking the collection factory pattern where collections listen for cleanup to remove themselves from a cache. Now, the cleanup process: + 1. Cleans up sync, state, changes, and indexes 2. Sets status to 'cleaned-up' (fires the event) 3. Finally cleans up event handlers @@ -14,20 +15,20 @@ Now, the cleanup process: This enables the collection factory pattern: ```typescript -const cache = new Map>(); +const cache = new Map>() const getTodoCollection = (id: string) => { if (!cache.has(id)) { - const collection = createCollection(/* ... */); + const collection = createCollection(/* ... */) - collection.on('status:change', ({ status }) => { - if (status === 'cleaned-up') { - cache.delete(id); // This now works! + collection.on("status:change", ({ status }) => { + if (status === "cleaned-up") { + cache.delete(id) // This now works! } - }); + }) - cache.set(id, collection); + cache.set(id, collection) } - return cache.get(id)!; -}; + return cache.get(id)! +} ```