Skip to content

fix(useTransactions): include limit in txKeys.list's query key - #70

Merged
abayomicornelius merged 8 commits into
StellarSend:mainfrom
davidishere1:fix/recent-transactions-query-key-missing-limit
Aug 21, 2026
Merged

fix(useTransactions): include limit in txKeys.list's query key#70
abayomicornelius merged 8 commits into
StellarSend:mainfrom
davidishere1:fix/recent-transactions-query-key-missing-limit

Conversation

@davidishere1

@davidishere1 davidishere1 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Summary

useRecentTransactions(limit)'s React Query cache key never included limit, so every call for the same wallet — regardless of what limit was passed — resolved to the identical query key. React Query treats those as one shared query and deduplicates/shares results between them.

Dashboard.tsx alone has three simultaneous callers with different limits (RecentTransactions(5), QuickStats(50), ActivityChart(50)); History.tsx has two more (HistoryChart(100), HistorySummary(50)). Whichever queryFn happened to run for the shared key determined the data every subscriber received — QuickStats's totals and ActivityChart's 7-day chart could silently under-report real activity with no error and no visible sign anything was wrong.

Fix: txKeys.list now takes limit as an explicit parameter and includes it in the returned key tuple, exactly as the issue's suggested fix describes.

  1. fix: include limit in txKeys.list's query key, plus direct unit tests of the key factory's shape
  2. test: regression test mirroring the issue's exact reproduction — two different limits for the same wallet now fetch and receive independent, correctly-sized datasets (verified this fails against the pre-fix key: only 1 fetch call instead of 2)
  3. test: fixed the regression test's mock Transaction fixtures to satisfy the real type (vitest doesn't type-check, but tsc/npm run build do)
  4. test: Dashboard's exact 3-caller scenario (5, 50, 50) — confirms the fix doesn't overcorrect: the two limit=50 callers still correctly dedupe to one shared fetch/data reference, while limit=5 gets its own
  5. test: History's exact 2-caller scenario (100, 50)
  6. test: identical limits across two hook instances still share one fetch (general caching-efficiency sanity check)
  7. test: useInvalidateTransactions (which invalidates via the broader txKeys.all prefix) still correctly invalidates every distinct-limit query after the key shape change
  8. docs: note the fix in CHANGELOG, matching this repo's existing convention

Test plan

  • npx vitest run — 113/113 passing across 27 test files
  • npx tsc --noEmit — clean
  • npm run lint — 0 warnings (--max-warnings 0)
  • npm run build — succeeds
  • Manually verified the core regression test fails against the pre-fix txKeys.list (fetcher called once instead of twice) and passes against the fix

Closes #51

txKeys.list's query key never included limit, so every useRecentTransactions
call for the same wallet — regardless of what limit was passed — collided
on one shared React Query cache entry (StellarSend#51). Dashboard.tsx alone has three
callers with different limits (5, 50, 50); History.tsx has two (100, 50).
Whichever queryFn happened to run for the shared key determined the data
every subscriber received, silently under- or over-reporting activity.

Adds txKeys.list's limit param and two direct unit tests of the key
factory's shape.
…ding on one cache entry

Mirrors Dashboard.tsx's exact scenario: RecentTransactions calls
useRecentTransactions(5) while QuickStats/ActivityChart call
useRecentTransactions(50) for the same connected wallet at the same time.
Asserts fetchTransactionsFromHorizon is invoked once per distinct limit and
each hook instance receives a dataset sized for its own limit.

Verified this fails against the pre-fix txKeys.list (fetcher called only
once, with whichever limit happened to run first for the shared key) and
passes against the fix.
Confirms the fix doesn't overcorrect into never sharing a cache entry:
RecentTransactions(5), QuickStats(50), and ActivityChart(50) mounted
together produce exactly two fetches, not three — the two limit=50 callers
correctly dedupe to one shared query and the same data reference, while
limit=5 gets its own independent fetch.
Independent collision from Dashboard's — HistoryChart(100) and
HistorySummary(50) on a separate page, same class of bug. Confirms both
fetch independently and each receives its own limit-sized dataset.
General correctness check that the fix didn't overcorrect into always
fetching separately: two useRecentTransactions(50) instances for the same
wallet still dedupe to a single fetch and the same data reference.
…ery distinct-limit query

Now that txKeys.list()'s key includes limit, this confirms the broader
txKeys.all-prefix invalidation useInvalidateTransactions relies on still
reaches every limit variant under a wallet, not just one — both a
limit=5 and a limit=50 query refetch after a single invalidate() call.
…eal type

The regression tests' mock transactions only set {id}, which happened to
work for vitest (no type-checking at runtime) but failed tsc/npm run build
— Transaction requires hash, createdAt, type, status, direction,
counterparty, and more. Adds a makeTransaction/makeTransactions fixture
builder satisfying the full type and uses it everywhere the tests fabricate
transaction data.
@davidishere1

davidishere1 commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

All checks passed. please review

@abayomicornelius
abayomicornelius merged commit 7e74b0c into StellarSend:main Aug 21, 2026
4 checks passed
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.

useRecentTransactions(limit)'s query key omits limit — Dashboard/History components requesting different limits collide on one shared cache entry

2 participants