diff --git a/.changeset/happy-parks-invite.md b/.changeset/happy-parks-invite.md new file mode 100644 index 000000000..8871910e4 --- /dev/null +++ b/.changeset/happy-parks-invite.md @@ -0,0 +1,7 @@ +--- +"@tanstack/query-db-collection": patch +--- + +Fix `staleTime` behavior by automatically subscribing/unsubscribing from TanStack Query based on collection subscriber count. + +Previously, query collections kept a QueryObserver permanently subscribed, which broke TanStack Query's `staleTime` and window-focus refetch behavior. Now the QueryObserver properly goes inactive when the collection has no subscribers, restoring normal `staleTime`/`gcTime` semantics. diff --git a/packages/db/src/collection/events.ts b/packages/db/src/collection/events.ts index 1b6c31be5..d0f56e744 100644 --- a/packages/db/src/collection/events.ts +++ b/packages/db/src/collection/events.ts @@ -70,7 +70,7 @@ export class CollectionEventsManager { this.listeners.get(event)!.add(callback) return () => { - this.listeners.get(event)!.delete(callback) + this.listeners.get(event)?.delete(callback) } } diff --git a/packages/db/src/types.ts b/packages/db/src/types.ts index 38ae672ce..5ca19854d 100644 --- a/packages/db/src/types.ts +++ b/packages/db/src/types.ts @@ -334,8 +334,14 @@ export interface BaseCollectionConfig< */ gcTime?: number /** - * Whether to start syncing immediately when the collection is created. - * Defaults to false for lazy loading. Set to true to immediately sync. + * Whether to eagerly start syncing on collection creation. + * When true, syncing begins immediately. When false, syncing starts when the first subscriber attaches. + * + * Note: Even with startSync=true, collections will pause syncing when there are no active + * subscribers (typically when components querying the collection unmount), resuming when new + * subscribers attach. This preserves normal staleTime/gcTime behavior. + * + * @default false */ startSync?: boolean /** diff --git a/packages/query-db-collection/src/query.ts b/packages/query-db-collection/src/query.ts index 09380cd28..902d81390 100644 --- a/packages/query-db-collection/src/query.ts +++ b/packages/query-db-collection/src/query.ts @@ -338,6 +338,7 @@ export function queryCollectionOptions( throw new QueryClientRequiredError() } + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (!getKey) { throw new GetKeyRequiredError() } @@ -379,8 +380,11 @@ export function queryCollectionOptions( any >(queryClient, observerOptions) + let isSubscribed = false + let actualUnsubscribeFn: (() => void) | null = null + type UpdateHandler = Parameters[0] - const handleUpdate: UpdateHandler = (result) => { + const handleQueryResult: UpdateHandler = (result) => { if (result.isSuccess) { // Clear error state lastError = undefined @@ -472,14 +476,45 @@ export function queryCollectionOptions( } } - const actualUnsubscribeFn = localObserver.subscribe(handleUpdate) + const subscribeToQuery = () => { + if (!isSubscribed) { + actualUnsubscribeFn = localObserver.subscribe(handleQueryResult) + isSubscribed = true + } + } + + const unsubscribeFromQuery = () => { + if (isSubscribed && actualUnsubscribeFn) { + actualUnsubscribeFn() + actualUnsubscribeFn = null + isSubscribed = false + } + } + + // If startSync=true or there are subscribers to the collection, subscribe to the query straight away + if (config.startSync || collection.subscriberCount > 0) { + subscribeToQuery() + } + + // Set up event listener for subscriber changes + const unsubscribeFromCollectionEvents = collection.on( + `subscribers:change`, + ({ subscriberCount }) => { + if (subscriberCount > 0) { + subscribeToQuery() + } else if (subscriberCount === 0) { + unsubscribeFromQuery() + } + } + ) // Ensure we process any existing query data (QueryObserver doesn't invoke its callback automatically with initial // state) - handleUpdate(localObserver.getCurrentResult()) + handleQueryResult(localObserver.getCurrentResult()) return async () => { - actualUnsubscribeFn() + unsubscribeFromCollectionEvents() + unsubscribeFromQuery() await queryClient.cancelQueries({ queryKey }) queryClient.removeQueries({ queryKey }) } diff --git a/packages/query-db-collection/tests/query.test-d.ts b/packages/query-db-collection/tests/query.test-d.ts index b6f7aefaa..7d97d21e5 100644 --- a/packages/query-db-collection/tests/query.test-d.ts +++ b/packages/query-db-collection/tests/query.test-d.ts @@ -1,5 +1,6 @@ import { describe, expectTypeOf, it } from "vitest" import { + and, createCollection, createLiveQueryCollection, eq, @@ -166,7 +167,7 @@ describe(`Query collection type resolution tests`, () => { query: (q) => q .from({ user: usersCollection }) - .where(({ user }) => eq(user.active, true) && gt(user.age, 18)) + .where(({ user }) => and(eq(user.active, true), gt(user.age, 18))) .select(({ user }) => ({ id: user.id, name: user.name, diff --git a/packages/query-db-collection/tests/query.test.ts b/packages/query-db-collection/tests/query.test.ts index ce6dc45ad..c7085a857 100644 --- a/packages/query-db-collection/tests/query.test.ts +++ b/packages/query-db-collection/tests/query.test.ts @@ -661,6 +661,16 @@ describe(`QueryCollection`, () => { expect(collection.size).toBe(1) }) + // Verify initial subscriber state - startSync=true, so even with no subscribers of the collection, there should + // be an active subscription to the query + expect(collection.subscriberCount).toBe(0) + expect(collection.status).toBe(`ready`) + + // Add explicit subscribers to test cleanup with active subscribers + const subscription1 = collection.subscribeChanges(() => {}) + const subscription2 = collection.subscribeChanges(() => {}) + expect(collection.subscriberCount).toBe(2) + // Cleanup the collection which should trigger sync cleanup await collection.cleanup() @@ -670,10 +680,15 @@ describe(`QueryCollection`, () => { // Verify collection status expect(collection.status).toBe(`cleaned-up`) - // Verify that the TanStack Query cleanup methods were called + // Verify that cleanup methods are called regardless of subscriber state expect(cancelQueriesSpy).toHaveBeenCalledWith({ queryKey }) expect(removeQueriesSpy).toHaveBeenCalledWith({ queryKey }) + // Verify subscribers can be safely cleaned up after collection cleanup + subscription1.unsubscribe() + subscription2.unsubscribe() + expect(collection.subscriberCount).toBe(0) + // Restore spies cancelQueriesSpy.mockRestore() removeQueriesSpy.mockRestore() @@ -701,15 +716,28 @@ describe(`QueryCollection`, () => { expect(collection.size).toBe(1) }) - // Call cleanup multiple times + // Add subscribers to test consistency during multiple cleanups + const subscription1 = collection.subscribeChanges(() => {}) + const subscription2 = collection.subscribeChanges(() => {}) + expect(collection.subscriberCount).toBe(2) + + // Call cleanup multiple times - subscriber count should remain consistent await collection.cleanup() expect(collection.status).toBe(`cleaned-up`) + expect(collection.subscriberCount).toBe(2) // Subscribers still tracked await collection.cleanup() await collection.cleanup() - // Should handle multiple cleanups gracefully + // Should handle multiple cleanups gracefully with consistent subscriber state expect(collection.status).toBe(`cleaned-up`) + expect(collection.subscriberCount).toBe(2) // Still consistent + + // Verify subscribers can be safely unsubscribed after multiple cleanups + subscription1.unsubscribe() + expect(collection.subscriberCount).toBe(1) + subscription2.unsubscribe() + expect(collection.subscriberCount).toBe(0) }) it(`should restart sync when collection is accessed after cleanup`, async () => { @@ -735,17 +763,31 @@ describe(`QueryCollection`, () => { expect(collection.size).toBe(1) }) - // Cleanup + // Verify initial subscriber state + expect(collection.subscriberCount).toBe(0) // startSync: true with no explicit subscribers + + // Add a subscriber before cleanup + const preCleanupSubscription = collection.subscribeChanges(() => {}) + expect(collection.subscriberCount).toBe(1) + + // Cleanup - should handle active subscribers gracefully await collection.cleanup() expect(collection.status).toBe(`cleaned-up`) - // Access collection data to restart sync - const subscription = collection.subscribeChanges(() => {}) + // Subscriber count should remain tracked even after cleanup + expect(collection.subscriberCount).toBe(1) + preCleanupSubscription.unsubscribe() // Clean up old subscriber + expect(collection.subscriberCount).toBe(0) + + // Access collection data to restart sync with new subscriber + const postCleanupSubscription = collection.subscribeChanges(() => {}) + expect(collection.subscriberCount).toBe(1) // Subscriber count tracking works after restart // Should restart sync (might be ready immediately if query is cached) expect([`loading`, `ready`]).toContain(collection.status) - subscription.unsubscribe() + postCleanupSubscription.unsubscribe() + expect(collection.subscriberCount).toBe(0) }) it(`should handle query lifecycle during restart cycle`, async () => { @@ -944,12 +986,19 @@ describe(`QueryCollection`, () => { expect(collection.size).toBe(1) }) - // Create multiple subscriptions + // Verify initial subscriber count - startSync=true means the query should be active + expect(collection.subscriberCount).toBe(0) + expect(collection.status).toBe(`ready`) + + // Create multiple subscriptions and track count changes const changeHandler1 = vi.fn() const changeHandler2 = vi.fn() const subscription1 = collection.subscribeChanges(changeHandler1) + expect(collection.subscriberCount).toBe(1) // 0 → 1 + const subscription2 = collection.subscribeChanges(changeHandler2) + expect(collection.subscriberCount).toBe(2) // 1 → 2 // Change the data and trigger a refetch items = [{ id: `1`, name: `Item 1 Updated` }] @@ -964,8 +1013,10 @@ describe(`QueryCollection`, () => { expect(changeHandler1).toHaveBeenCalled() expect(changeHandler2).toHaveBeenCalled() - // Unsubscribe one + // Unsubscribe one and verify count tracking subscription1.unsubscribe() + expect(collection.subscriberCount).toBe(1) // 2 → 1 + changeHandler1.mockClear() changeHandler2.mockClear() @@ -982,8 +1033,10 @@ describe(`QueryCollection`, () => { expect(changeHandler1).not.toHaveBeenCalled() expect(changeHandler2).toHaveBeenCalled() - // Cleanup + // Final cleanup - verify query remains active due to startSync: true subscription2.unsubscribe() + expect(collection.subscriberCount).toBe(0) // 1 → 0 + expect(collection.status).toBe(`ready`) // Still ready due to startSync: true }) it(`should handle query cancellation gracefully`, async () => { @@ -1064,6 +1117,71 @@ describe(`QueryCollection`, () => { const finalItem = collection.get(`1`) expect(finalItem?.name).toMatch(/^Item \d+$/) }) + + it(`should manage startSync vs subscriber count priority correctly`, async () => { + const queryKey1 = [`startSyncTruePriorityTest`] + const queryKey2 = [`startSyncFalsePriorityTest`] + const items = [{ id: `1`, name: `Item 1` }] + const queryFn1 = vi.fn().mockResolvedValue(items) + const queryFn2 = vi.fn().mockResolvedValue(items) + + // Test case 1: startSync=true should keep query active even with 0 subscribers + const config1: QueryCollectionConfig = { + id: `startSyncTrueTest`, + queryClient, + queryKey: queryKey1, + queryFn: queryFn1, + getKey, + startSync: true, + } + + const options1 = queryCollectionOptions(config1) + const collection1 = createCollection(options1) + + await vi.waitFor(() => { + expect(collection1.status).toBe(`ready`) + }) + + expect(collection1.subscriberCount).toBe(0) + expect(queryFn1).toHaveBeenCalled() + expect(collection1.status).toBe(`ready`) // Active due to startSync: true + + // Test case 2: startSync=false should rely purely on subscriber count + const config2: QueryCollectionConfig = { + id: `startSyncFalseTest`, + queryClient, + queryKey: queryKey2, + queryFn: queryFn2, + getKey, + startSync: false, + } + + const options2 = queryCollectionOptions(config2) + const collection2 = createCollection(options2) + + await flushPromises() + + expect(collection2.subscriberCount).toBe(0) + expect(queryFn2).not.toHaveBeenCalled() // Should not be called without subscribers + expect(collection2.status).toBe(`idle`) // Inactive due to startSync: false + no subscribers + + // Add subscriber to collection2 -> should now activate + const subscription = collection2.subscribeChanges(() => {}) + + await vi.waitFor(() => expect(collection2.status).toBe(`ready`)) + + expect(collection2.subscriberCount).toBe(1) + expect(queryFn2).toHaveBeenCalled() // Now called due to subscriber + + // Remove subscriber -> query may still be active but subscriber count drops + subscription.unsubscribe() + expect(collection2.subscriberCount).toBe(0) + + // Verify the core logic: startSync || subscriberCount > 0 + // collection1: startSync=true, subscriberCount=0 -> active + // collection2: startSync=false, subscriberCount=0 -> depends on implementation + expect(collection1.status).toBe(`ready`) // Always active with startSync: true + }) }) describe(`Manual Sync Operations`, () => { @@ -1644,6 +1762,82 @@ describe(`QueryCollection`, () => { ) }) + describe(`subscriber count tracking and auto-subscription`, () => { + it(`should not auto-subscribe when startSync=false and no subscribers`, async () => { + const queryKey = [`noSubscriptionTest`] + const items = [{ id: `1`, name: `Item 1` }] + const queryFn = vi.fn().mockResolvedValue(items) + + const config: QueryCollectionConfig = { + id: `noSubscriptionTest`, + queryClient, + queryKey, + queryFn, + getKey, + startSync: false, + } + + const options = queryCollectionOptions(config) + const collection = createCollection(options) + + // Give it time to potentially subscribe (it shouldn't) + await flushPromises() + + expect(collection.subscriberCount).toBe(0) + expect(collection.status).toBe(`idle`) // Should remain idle without startSync or subscribers + expect(queryFn).not.toHaveBeenCalled() // Query should not be executed + }) + + it(`should subscribe/unsubscribe based on subscriber count transitions`, async () => { + const queryKey = [`countTransitionTest`] + const items = [{ id: `1`, name: `Item 1` }] + const queryFn = vi.fn().mockResolvedValue(items) + + const config: QueryCollectionConfig = { + id: `countTransition`, + queryClient, + queryKey, + queryFn, + getKey, + startSync: false, // Start unsubscribed + } + + const options = queryCollectionOptions(config) + const collection = createCollection(options) + + // Should start unsubscribed + expect(collection.subscriberCount).toBe(0) + expect(collection.status).toBe(`idle`) + + // Add a subscriber -> should subscribe and load data + const subscription1 = collection.subscribeChanges(() => {}) + + await vi.waitFor(() => { + expect(collection.status).toBe(`ready`) + }) + + expect(collection.subscriberCount).toBe(1) + expect(queryFn).toHaveBeenCalled() + + // Add another subscriber - should not trigger additional queries + const initialCallCount = queryFn.mock.calls.length + const subscription2 = collection.subscribeChanges(() => {}) + expect(collection.subscriberCount).toBe(2) + + await flushPromises() + expect(queryFn.mock.calls.length).toBe(initialCallCount) // No additional calls + + // Remove first subscriber - should still be subscribed + subscription1.unsubscribe() + expect(collection.subscriberCount).toBe(1) + expect(collection.status).toBe(`ready`) + + // Remove last subscriber -> query should remain active but collection subscriber count drops to 0 + subscription2.unsubscribe() + expect(collection.subscriberCount).toBe(0) + }) + }) + describe(`Error Handling`, () => { // Helper to create test collection with common configuration const createErrorHandlingTestCollection = ( diff --git a/packages/query-db-collection/tsconfig.json b/packages/query-db-collection/tsconfig.json index 7e586bab3..623d4bd91 100644 --- a/packages/query-db-collection/tsconfig.json +++ b/packages/query-db-collection/tsconfig.json @@ -12,7 +12,9 @@ "forceConsistentCasingInFileNames": true, "jsx": "react", "paths": { - "@tanstack/store": ["../store/src"] + "@tanstack/store": ["../store/src"], + "@tanstack/db": ["../db/src"], + "@tanstack/db-ivm": ["../db-ivm/src"] } }, "include": ["src", "tests", "vite.config.ts"],