Key collection subscriptions by query identity#22
Merged
Conversation
… keying Approach v1 (caller-supplied key). When provided, the subscription is keyed on queryKey instead of the queryConstraints array reference, so upstream reference churn (e.g. arrays read from deep-cloned documents) does not tear down and reload the listener. Latest constraints are read through a ref at rebuild time. - queryKey?: string on UseCollectionOptions - regression tests in src/hooks.test.ts (react-test-renderer) - README / docs / AGENTS.md contract updates
… drop queryKey Approach v2 (auto-stabilization, no public API). Replaces the caller-supplied queryKey with automatic stabilization: the hook builds the query and compares it with Firestore's own queryEqual, keeping the previous constraints reference when the query is unchanged. Reference churn (e.g. constraint inputs read from a deep-cloned document) no longer tears down the listener, and a genuine query change still rebuilds it — with no caller key and no silent-staleness footgun. - removes queryKey from UseCollectionOptions (public API) - queryConstraintsEqual()/buildCollectionQuery() helpers in hooks.ts - tests rewritten to assert semantic-identity behavior (real Firestore + real query/queryEqual, onSnapshot mocked) - README / docs / AGENTS.md updated; callers no longer need to memoize
V2's queryEqual subscription stabilization duplicated collection.ts's constraint merge order (definition constraints first, then hook-level, bare collection ref when empty). The two copies had to stay in sync by hand — drift would let the comparator keep a stale listener or reintroduce the resubscribe flash, with nothing to catch it. Extract one exported buildCollectionQuery() in collection.ts and route both the live subscription and useCollection's queryEqual comparison through it. The merge order now exists in exactly one place. No behavior change. Typecheck clean; 142/142 tests pass.
…constraints queryConstraintsEqual built both the prior and incoming constraint arrays to compare them with queryEqual. For lazy collections, the hook becomes active (enabled + resolved path) before load() attaches a listener, so a render carrying a gated placeholder like where(documentId(), 'in', []) followed by real IDs would build the stale empty-array query during the identity compare and throw, crashing the render before load() ran. Wrap the build in try/catch: if the prior snapshot can't be built, no live listener could have run it, so treat the snapshots as unequal and adopt the new constraints. Defends against any unbuildable constraint shape, not just the lazy empty-in case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useCollectionlistener reuse by Firestore query identity withqueryEqual, while preserving rebuilds for real query changes.buildCollectionQueryso comparison and subscription paths stay aligned.inqueries.Tests
pnpm typecheckpnpm test