Replace RequestCache with TanStack Query#269
Merged
Merged
Conversation
Add TanStack Query (v5) to replace the hand-rolled RequestCache for mutable
data over time. This commit is plumbing only — no other call sites are
migrated.
Changes:
- Install @tanstack/react-query, react-query-devtools,
react-query-persist-client, query-async-storage-persister
- src/browser/lib/query-client.ts — single QueryClient (staleTime=30s,
gcTime=5min, no refetch-on-focus) + raw IDB persister in a dedicated
'pulldash-rq/data' store, separate from PersistentCache ('pulldash/responses'),
using the same raw IDB pattern already in persistent-cache.ts (no idb-keyval)
- src/browser/lib/github-client.ts — module-level setOctokit/getOctokit so
query functions can reach the authenticated Octokit without being inside the
GitHub context; set by GitHubProvider on auth, cleared on sign-out
- src/browser/lib/queries.ts — queryOptions factory registry with conventions
documented in the header comment; currentUser query as smoke test
(meta: { persist: true }, staleTime: 5 min)
- src/browser/index.tsx — wrap app root with PersistQueryClientProvider;
dehydrateOptions filters to meta.persist===true; ReactQueryDevtools
mounted only in dev builds (__DEV__ define)
- scripts/build-browser.ts — add __DEV__ define (true when --watch)
- src/types/assets.d.ts — declare const __DEV__: boolean
- Migrate user:current from RequestCache to React Query — remove
fetchCurrentUser() and currentUser from GitHubState; useCurrentUser() now
uses useQuery(queries.currentUser()) enabled by ready; useCurrentUserLoader
updated to use the hook
Resolves #260
Six functions in github.tsx that cache content-addressed (SHA-keyed) data
were using RequestCache (in-memory + localStorage) as their primary store,
falling back to PersistentCache only when prKey was provided. Since these
responses are immutable, they belong in IndexedDB permanently.
Changes:
- getCommitFiles, getSingleCommit, getRawGitCommit, getMergeCommitFiles,
getRawCompareDiff, getPRFilesForRange, getCommitsForHeadSha:
- Remove cache.get/set/getPending/setPending calls
- Check PersistentCache.get first (always, not just when prKey given)
- Use a per-store Map<string, Promise> (immutablePending) for in-flight
request deduplication only; cleared on settle
- Write to PersistentCache when prKey is available
- pr-review/index.tsx: four getPRFilesForRange callers that omitted prKey
now pass `${owner}/${repo}/${pr.number}` so results are persisted
Existing cache keys are preserved so on-disk entries from previous sessions
remain valid without migration.
Resolves #261
getPRChecksForSha and getWorkflowRunsForSha were using RequestCache
(in-memory + localStorage) with a 15s TTL. They're mutable (status
transitions pending→success/failure), so they go through React Query.
Changes:
- queries.ts: add checksByCommit(owner, repo, sha) and
workflowRunsByCommit(owner, repo, sha) factories, staleTime: 15_000,
no meta.persist
- github.tsx: replace the RequestCache-based implementations of
getPRChecksForSha / getWorkflowRunsForSha with one-line wrappers
around queryClient.fetchQuery(); all 8 callers (fetchPRChecks plus
7 direct calls in pr-review/index.tsx) transparently go through
React Query now
- approveWorkflowRun: replace cache.invalidate('workflow-runs:...') with
queryClient.invalidateQueries({ queryKey: ['workflow-runs', owner, repo] });
partial key match invalidates all SHAs for this repo, preserving the
previous prefix-match behavior
Resolves #262
Both are slow-changing, read-only from the app, and good first uses of the
meta.persist path (cross-session IndexedDB persistence).
Changes:
- queries.ts: add collaborators(owner, repo) and labels(owner, repo)
factories, staleTime: 5 min, meta: { persist: true }
- github.tsx: getRepoCollaborators and getRepoLabels become one-line
wrappers around queryClient.fetchQuery()
- pr-overview.tsx:
- PROverview: replace useState + fetchCollaborators with
useQuery(queries.collaborators), drop the manual fetch-on-picker-open
pattern; collaborator data is now pre-fetched and cached eagerly
- LabelsSection: replace useState + fetchLabels with
useQuery(queries.labels), same pattern
Resolves #263
Replace the hand-rolled fetchPRList / searchPRs / searchRepos / searchUsers
functions (which all used RequestCache internally) with four new queryOptions
factories in queries.ts:
- queries.prList(queryStrings, page, perPage) — compound query that runs
all search strings in parallel, deduplicates, sorts, and enriches via a
single GraphQL call. placeholderData: keepPreviousData prevents pagination
flicker.
- queries.searchPRs(query, page, perPage) — thin wrapper used by the store
for the fetchInvolvedPRs helper.
- queries.searchRepos(query) — used imperatively in the home repo-search
dropdown.
- queries.searchUsers(query) — used in the markdown @-mention autocomplete.
The prList enrichment logic (GraphQL PR metadata batch) is inlined into the
queryFn in queries.ts, calling getOctokit().graphql() directly rather than
routing through the store's GraphQLBatcher (the batcher was only a 5ms
micro-optimisation, unnecessary inside a queryFn).
home.tsx now derives its data from useQuery instead of useSyncExternalStore:
const { data, isFetching, isPending, dataUpdatedAt } =
useQuery({ ...queries.prList(searchQueries, page, perPage), enabled: ready });
RefreshCountdown receives dataUpdatedAt (a React Query timestamp) instead of
the old PRListState.lastFetchedAt. The manual 60-second setInterval auto-
refresh is removed; React Query's staleTime (30 s) + refetchOnWindowFocus
covers the same job.
invalidatePRCaches() in pr-review/index.tsx now also invalidates
["pr-list"] and ["search", "prs"] so the home page reflects PR state
changes (close / reopen / merge / enqueue) immediately.
GitHubState loses prList, prListQueries, and prListPage; the reset() function
calls queryClient.removeQueries(["pr-list"]) instead. fetchPRList,
refreshPRList, usePRList, and usePRListActions are removed.
Resolves #264
… Query Replace RequestCache implementations for PR object, files, commits, push versions, and user profile lookups with React Query factories. Mutations that change PR header state now invalidate the ["pull-request", owner, repo, number] subtree (which covers files, commits, push versions) and ["pr-list"]; the broad pattern-based cache.invalidate still runs for the remaining RequestCache entries (comments, reviews, conversation, timeline) that move in ticket #267. PATCH /pulls responses (closePR, reopenPR, updatePR) prime the PR query cache before invalidation so the UI shows the new state immediately. Progresses: #259 Resolves #265
Migrates the bulk of mutated PR data to React Query factories under the ["pull-request", ...] and ["reactions", ...] key hierarchies: - pullRequestComments, pullRequestReviews - pullRequestConversation, pullRequestTimeline - issueReactions, commentReactions, reviewCommentReactions Reaction mutations now target their specific query key; comment and conversation mutations invalidate just their subtree. PR-level mutations continue to call invalidatePR, which prefix-matches the entire ["pull-request", owner, repo, number] subtree — so a single invalidate picks up files, commits, push-versions, comments, reviews, conversation, and timeline at once. Progresses: #259 Resolves #266
All call sites pass a SHA ref + prKey, so the RequestCache layer was redundant — every fetched file is content-addressed and immutable. Route the function through PersistentCache directly, using the immutablePending map for in-flight dedup (matching getSingleCommit / getCommitFiles). Last mutable RequestCache reader; the class itself goes away in #268. Progresses: #259 Resolves #267
All mutable reads now flow through React Query; the SHA-keyed
immutable ones live in PersistentCache. Delete the RequestCache
class, its gh_cache:* localStorage hydration, the auth-flow shim
that cleared those keys, and the dead caches.delete("pulldash-v1")
service-worker reference.
The public invalidateCache(pattern) API is replaced by a typed
invalidatePR(owner, repo, number) helper — every caller used the
same pr:owner/repo/number pattern. Logout's full-flush is now
queryClient.clear() in reset(), matching how React Query owns
the cache.
GraphQLBatcher is untouched — it batches at the transport layer.
Resolves #268
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
1 participant
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.
I keep finding little inconsistencies where not all data on the screen updates at once. I think it's better if we just use TanStack Query to handle data fetching and caching.
Add TanStack Query (v5) to replace the hand-rolled RequestCache for mutable
data over time. If a query is marked with
meta: { persist: true }then the key will be persisted between page loads. This is done for the data that RequestCache was previously storing in localStorage.This does not replace immutable data that we were already storing in IndexedDB. That is untouched and remains outside of TanStack Query. It might be worthwhile to fetch even this data via TanStack Query for loading state, etc., but that can be done as a separate effort.
Resolves #259