Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .changeset/fix-collection-cleanup-order.md
Original file line number Diff line number Diff line change
@@ -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<string, ReturnType<typeof createCollection>>()

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)!
}
```
10 changes: 7 additions & 3 deletions packages/db/src/collection/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
47 changes: 47 additions & 0 deletions packages/db/tests/collection-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
})
})
})
Loading