diff --git a/.changeset/neat-queens-smile.md b/.changeset/neat-queens-smile.md new file mode 100644 index 000000000..1632c33b7 --- /dev/null +++ b/.changeset/neat-queens-smile.md @@ -0,0 +1,6 @@ +--- +"@tanstack/electric-db-collection": patch +"@tanstack/db": patch +--- + +fix disabling of gc by setting `gcTime: 0` on the collection options diff --git a/packages/db/src/collection.ts b/packages/db/src/collection.ts index 4bfe6296a..9fa939f1b 100644 --- a/packages/db/src/collection.ts +++ b/packages/db/src/collection.ts @@ -746,6 +746,12 @@ export class CollectionImpl< } const gcTime = this.config.gcTime ?? 300000 // 5 minutes default + + // If gcTime is 0, GC is disabled + if (gcTime === 0) { + return + } + this.gcTimeoutId = setTimeout(() => { if (this.activeSubscribersCount === 0) { this.cleanup() @@ -784,7 +790,6 @@ export class CollectionImpl< this.activeSubscribersCount-- if (this.activeSubscribersCount === 0) { - this.activeSubscribersCount = 0 this.startGCTimer() } else if (this.activeSubscribersCount < 0) { throw new NegativeActiveSubscribersError() diff --git a/packages/db/tests/collection-lifecycle.test.ts b/packages/db/tests/collection-lifecycle.test.ts index 8f2b8f8f6..62c32dfc7 100644 --- a/packages/db/tests/collection-lifecycle.test.ts +++ b/packages/db/tests/collection-lifecycle.test.ts @@ -239,6 +239,30 @@ describe(`Collection Lifecycle Management`, () => { unsubscribe3() expect((collection as any).activeSubscribersCount).toBe(0) }) + + it(`should handle rapid subscribe/unsubscribe correctly`, () => { + const collection = createCollection<{ id: string; name: string }>({ + id: `rapid-sub-test`, + getKey: (item) => item.id, + gcTime: 1000, // Short GC time for testing + sync: { + sync: () => {}, + }, + }) + + // Subscribe and immediately unsubscribe multiple times + for (let i = 0; i < 5; i++) { + const unsubscribe = collection.subscribeChanges(() => {}) + expect((collection as any).activeSubscribersCount).toBe(1) + unsubscribe() + expect((collection as any).activeSubscribersCount).toBe(0) + + // Should start GC timer each time + expect(mockSetTimeout).toHaveBeenCalledWith(expect.any(Function), 1000) + } + + expect(mockSetTimeout).toHaveBeenCalledTimes(5) + }) }) describe(`Garbage Collection`, () => { @@ -325,6 +349,24 @@ describe(`Collection Lifecycle Management`, () => { // Should use default 5 minutes (300000ms) expect(mockSetTimeout).toHaveBeenCalledWith(expect.any(Function), 300000) }) + + it(`should disable GC when gcTime is 0`, () => { + const collection = createCollection<{ id: string; name: string }>({ + id: `disabled-gc-test`, + getKey: (item) => item.id, + gcTime: 0, // Disabled GC + sync: { + sync: () => {}, + }, + }) + + const unsubscribe = collection.subscribeChanges(() => {}) + unsubscribe() + + // Should not start any timer when GC is disabled + expect(mockSetTimeout).not.toHaveBeenCalled() + expect(collection.status).not.toBe(`cleaned-up`) + }) }) describe(`Manual Preload and Cleanup`, () => { diff --git a/packages/electric-db-collection/tests/electric.test.ts b/packages/electric-db-collection/tests/electric.test.ts index e8167efbb..78b94e005 100644 --- a/packages/electric-db-collection/tests/electric.test.ts +++ b/packages/electric-db-collection/tests/electric.test.ts @@ -1028,5 +1028,105 @@ describe(`Electric Integration`, () => { await expect(testCollection.utils.awaitTxId(300)).resolves.toBe(true) await expect(testCollection.utils.awaitTxId(400)).resolves.toBe(true) }) + + it(`should resync after garbage collection and new subscription`, () => { + // Use fake timers for this test + vi.useFakeTimers() + + const config = { + id: `gc-resync-test`, + shapeOptions: { + url: `http://test-url`, + params: { + table: `test_table`, + }, + }, + getKey: (item: Row) => item.id as number, + startSync: true, + gcTime: 100, // Short GC time for testing + } + + const testCollection = createCollection(electricCollectionOptions(config)) + + // Populate collection with initial data + subscriber([ + { + key: `1`, + value: { id: 1, name: `Initial User` }, + headers: { operation: `insert` }, + }, + { + key: `2`, + value: { id: 2, name: `Another User` }, + headers: { operation: `insert` }, + }, + { + headers: { control: `up-to-date` }, + }, + ]) + + // Verify initial data is present + expect(testCollection.has(1)).toBe(true) + expect(testCollection.has(2)).toBe(true) + expect(testCollection.size).toBe(2) + + // Subscribe and then unsubscribe to trigger GC timer + const unsubscribe = testCollection.subscribeChanges(() => {}) + unsubscribe() + + // Collection should still be ready before GC timer fires + expect(testCollection.status).toBe(`ready`) + expect(testCollection.size).toBe(2) + + // Fast-forward time to trigger GC (past the 100ms gcTime) + vi.advanceTimersByTime(150) + + // Collection should be cleaned up + expect(testCollection.status).toBe(`cleaned-up`) + expect(testCollection.size).toBe(0) + + // Reset mock call count for new subscription + const initialMockCallCount = mockSubscribe.mock.calls.length + + // Subscribe again - this should restart the sync + const newUnsubscribe = testCollection.subscribeChanges(() => {}) + + // Should have created a new stream + expect(mockSubscribe.mock.calls.length).toBe(initialMockCallCount + 1) + expect(testCollection.status).toBe(`loading`) + + // Send new data to simulate resync + subscriber([ + { + key: `3`, + value: { id: 3, name: `Resynced User` }, + headers: { operation: `insert` }, + }, + { + key: `1`, + value: { id: 1, name: `Updated User` }, + headers: { operation: `insert` }, + }, + { + headers: { control: `up-to-date` }, + }, + ]) + + // Verify the collection has resynced with new data + expect(testCollection.status).toBe(`ready`) + expect(testCollection.has(1)).toBe(true) + expect(testCollection.has(3)).toBe(true) + expect(testCollection.get(1)).toEqual({ id: 1, name: `Updated User` }) + expect(testCollection.get(3)).toEqual({ id: 3, name: `Resynced User` }) + expect(testCollection.size).toBe(2) + + // Old data should not be present (collection was cleaned) + expect(testCollection.has(2)).toBe(false) + + newUnsubscribe() + + // Restore real timers + vi.useRealTimers() + }) }) })