Skip to content

Conversation

samwillis
Copy link
Collaborator

Fixes #633 - a regression introduced in #462

Summary of fix from Claude:

Analysis: Our Fix Preserves the Intended Behavior ✅

The Original PR #462 Intention

The PR aimed to fix staleTime behavior by:

  1. Not subscribing to TanStack Query when startSync=false AND subscriberCount=0
  2. Dynamically subscribing/unsubscribing based on subscriber count changes
  3. Allowing the QueryObserver to go inactive, restoring proper staleTime/gcTime semantics

The Bug in the Implementation

The buggy code checked the subscription condition inside the sync function:

// Inside the sync function (WRONG LOCATION)
if (config.startSync || collection.subscriberCount > 0) {
  subscribeToQuery()
}

Problem: When preload() is called, it starts sync but has:

  • config.startSync = undefined/false
  • collection.subscriberCount = 0
  • Result: Never subscribes → queryFn never runs → preload() never resolves ❌

Our Fix is Correct ✅

The key insight: The decision to start sync happens BEFORE the sync function is called

The sync function is only invoked in these cases:

  1. Constructor with startSync: true (line 278-280, collection/index.ts)
  2. First subscriber added when status is idle (line 123-128, collection/changes.ts)
  3. preload() called (through startSync() in collection/sync.ts)
  4. Operations on cleaned-up collection (line 131-133, collection/lifecycle.ts)

Our fix:

// Always subscribe when sync starts
subscribeToQuery()

// Dynamically unsubscribe when no subscribers
collection.on('subscribers:change', ({ subscriberCount }) => {
  if (subscriberCount > 0) subscribeToQuery()
  else if (subscriberCount === 0) unsubscribeFromQuery()
})

Why This Preserves the Intended Behavior

Scenario Sync Function Called? Our Fix Behavior Intended Behavior
startSync=false, no subscribers ❌ No N/A (never called) Should not subscribe
startSync=true, no subscribers ✅ Yes Subscribes, unsubscribes when count=0 Should subscribe initially
preload() called ✅ Yes Subscribes Should subscribe
Subscriber added to idle collection ✅ Yes Subscribes Should subscribe
All subscribers removed Sync running Unsubscribes via event Should unsubscribe

Test Evidence

All 58 tests pass, including:

  • should not auto-subscribe when startSync=false and no subscribers (line 1886)
  • should subscribe/unsubscribe based on subscriber count transitions (line 1911)
  • should resolve preload() even without startSync or subscribers (our new test, line 2275)
  • should manage startSync vs subscriber count priority correctly (line 1120)

Conclusion: Our fix correctly addresses the preload() bug while fully preserving the staleTime behavior improvements from PR #462. The staleTime fix is maintained because the sync function is only called when sync should actually be active, and we properly unsubscribe when there are no subscribers.

@samwillis samwillis requested a review from KyleAMathews October 3, 2025 16:02
Copy link

changeset-bot bot commented Oct 3, 2025

🦋 Changeset detected

Latest commit: 88d12a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@tanstack/query-db-collection Patch
@tanstack/db-example-react-todo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Oct 3, 2025

More templates

@tanstack/angular-db

npm i https://pkg.pr.new/@tanstack/angular-db@635

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@635

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@635

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@635

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@635

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@635

@tanstack/rxdb-db-collection

npm i https://pkg.pr.new/@tanstack/rxdb-db-collection@635

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@635

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@635

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@635

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@635

commit: 88d12a4

Copy link
Contributor

github-actions bot commented Oct 3, 2025

Size Change: 0 B

Total Size: 74.1 kB

ℹ️ View Unchanged
Filename Size
./packages/db/dist/esm/collection/change-events.js 958 B
./packages/db/dist/esm/collection/changes.js 1.01 kB
./packages/db/dist/esm/collection/events.js 683 B
./packages/db/dist/esm/collection/index.js 3.14 kB
./packages/db/dist/esm/collection/indexes.js 1.16 kB
./packages/db/dist/esm/collection/lifecycle.js 1.8 kB
./packages/db/dist/esm/collection/mutations.js 2.59 kB
./packages/db/dist/esm/collection/state.js 3.81 kB
./packages/db/dist/esm/collection/subscription.js 1.69 kB
./packages/db/dist/esm/collection/sync.js 1.32 kB
./packages/db/dist/esm/deferred.js 230 B
./packages/db/dist/esm/errors.js 3.1 kB
./packages/db/dist/esm/index.js 1.56 kB
./packages/db/dist/esm/indexes/auto-index.js 745 B
./packages/db/dist/esm/indexes/base-index.js 605 B
./packages/db/dist/esm/indexes/btree-index.js 1.82 kB
./packages/db/dist/esm/indexes/lazy-index.js 1.25 kB
./packages/db/dist/esm/local-only.js 827 B
./packages/db/dist/esm/local-storage.js 2.02 kB
./packages/db/dist/esm/optimistic-action.js 294 B
./packages/db/dist/esm/proxy.js 3.87 kB
./packages/db/dist/esm/query/builder/functions.js 615 B
./packages/db/dist/esm/query/builder/index.js 3.93 kB
./packages/db/dist/esm/query/builder/ref-proxy.js 938 B
./packages/db/dist/esm/query/compiler/evaluators.js 1.56 kB
./packages/db/dist/esm/query/compiler/expressions.js 631 B
./packages/db/dist/esm/query/compiler/group-by.js 2.11 kB
./packages/db/dist/esm/query/compiler/index.js 2.04 kB
./packages/db/dist/esm/query/compiler/joins.js 2.54 kB
./packages/db/dist/esm/query/compiler/order-by.js 1.23 kB
./packages/db/dist/esm/query/compiler/select.js 1.28 kB
./packages/db/dist/esm/query/ir.js 785 B
./packages/db/dist/esm/query/live-query-collection.js 340 B
./packages/db/dist/esm/query/live/collection-config-builder.js 2.68 kB
./packages/db/dist/esm/query/live/collection-subscriber.js 1.91 kB
./packages/db/dist/esm/query/optimizer.js 3.1 kB
./packages/db/dist/esm/SortedMap.js 1.24 kB
./packages/db/dist/esm/transactions.js 3.03 kB
./packages/db/dist/esm/utils.js 943 B
./packages/db/dist/esm/utils/browser-polyfills.js 365 B
./packages/db/dist/esm/utils/btree.js 6.02 kB
./packages/db/dist/esm/utils/comparison.js 754 B
./packages/db/dist/esm/utils/index-optimization.js 1.62 kB

compressed-size-action::db-package-size

Copy link
Contributor

github-actions bot commented Oct 3, 2025

Size Change: 0 B

Total Size: 1.44 kB

ℹ️ View Unchanged
Filename Size
./packages/react-db/dist/esm/index.js 152 B
./packages/react-db/dist/esm/useLiveQuery.js 1.29 kB

compressed-size-action::react-db-package-size

Copy link
Collaborator

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@samwillis samwillis merged commit b38bd76 into main Oct 3, 2025
6 checks passed
@samwillis samwillis deleted the samwillis/fix-qery-subscribe-633 branch October 3, 2025 16:11
@github-actions github-actions bot mentioned this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collection.preload() stops working
2 participants