diff --git a/.changeset/fix-collection-cleanup-order.md b/.changeset/fix-collection-cleanup-order.md new file mode 100644 index 000000000..f81aa84d2 --- /dev/null +++ b/.changeset/fix-collection-cleanup-order.md @@ -0,0 +1,34 @@ +--- +"@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`) + }) }) })