perf(platform): migrate useQuery to TanStack Query cache#423
Conversation
📝 WalkthroughWalkthroughThis pull request systematically migrates data-fetching throughout the platform from Convex's native React hooks to TanStack React Query integrated via a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
services/platform/app/routes/dashboard/create-organization.tsx (1)
38-44:⚠️ Potential issue | 🟠 MajorInfinite loading for unauthenticated users.
When the query is skipped (user not authenticated), TanStack Query returns
isLoading: falseanddata: undefined. The conditionisOrgsLoading || !organizationsevaluates totruefor skipped queries, causing unauthenticated users to see the loading spinner indefinitely.Consider checking authentication state explicitly before the organizations loading check:
🐛 Proposed fix
- if (isAuthLoading || isOrgsLoading || !organizations) { + if (isAuthLoading) { + return ( + <div className="flex h-screen items-center justify-center"> + <div className="animate-pulse">Loading...</div> + </div> + ); + } + + if (!isAuthenticated || isOrgsLoading || !organizations) { return ( <div className="flex h-screen items-center justify-center"> <div className="animate-pulse">Loading...</div> </div> ); }Alternatively, if unauthenticated users should be redirected, add a redirect to
/log-inwhen!isAuthenticatedafter the auth loading check.services/platform/app/features/settings/teams/components/team-members-dialog.tsx (1)
59-59: 🧹 Nitpick | 🔵 TrivialConsider using TanStack Query's
isPendingfor more robust loading state.Deriving loading state from
teamMembers === undefinedconflates "loading" with "errored" (both leavedataundefined). If the query fails, users see a perpetual loading spinner.♻️ Suggested improvement
- const { data: teamMembers } = useQuery( + const { data: teamMembers, isPending } = useQuery( convexQuery( api.team_members.queries.listByTeam, open ? { teamId: team.id } : 'skip', ), ); // Convex mutations for team member management const addTeamMember = useMutation(api.team_members.mutations.addMember); const removeTeamMember = useMutation(api.team_members.mutations.removeMember); - const isLoading = teamMembers === undefined; + const isLoading = isPending;Optionally, destructure
isErrorto show an error state instead of infinite loading on failures.services/platform/app/features/automations/components/automation-tester.tsx (1)
95-98:⚠️ Potential issue | 🟡 MinorSetting state during render is an anti-pattern.
This code block executes during the render phase and calls
setDryRunResultandsetIsDryRunning, which can cause React to re-render unexpectedly and may lead to infinite render loops in some scenarios.🔧 Proposed fix: Move state sync to useEffect
+ useEffect(() => { + if (isDryRunning && dryRunQuery && !dryRunResult) { + setDryRunResult(dryRunQuery); + setIsDryRunning(false); + } + }, [isDryRunning, dryRunQuery, dryRunResult]); - if (isDryRunning && dryRunQuery && !dryRunResult) { - setDryRunResult(dryRunQuery); - setIsDryRunning(false); - }Note: You'll need to add
useEffectto the imports from React on line 13.services/platform/app/routes/dashboard/$id/_knowledge/websites.tsx (1)
27-33:⚠️ Potential issue | 🟠 MajorAdd error handling and use
isPendinginstead ofisLoadingfor TanStack Query v5.With TanStack Query v5, a failed query results in
data === undefinedwithisLoading === false, causing the current code to incorrectly show the empty state instead of surfacing errors. Additionally, v5 redefinesisLoadingto mean "first fetch in-flight AND actively fetching"; the docs recommendisPendingfor checking if initial data is available.🛠️ Suggested adjustment
- const { data: hasWebsites, isLoading } = useQuery( + const { data: hasWebsites, isPending, isError, error } = useQuery( convexQuery(api.websites.queries.hasWebsites, { organizationId }), ); - if (isLoading) { + if (isPending) { return <WebsitesTableSkeleton organizationId={organizationId} />; } + if (isError) { + throw error; + } if (!hasWebsites) { return <WebsitesEmptyState organizationId={organizationId} />; }services/platform/app/routes/dashboard/$id/_knowledge/customers.tsx (1)
25-34:⚠️ Potential issue | 🟠 MajorHandle query errors explicitly to avoid false "empty" state.
React Query doesn't throw errors by default; if the request fails,
isLoadingbecomes false whilehasCustomersremainsundefined, making the UI renderCustomersEmptyStateand hide the failure. AddthrowOnError: trueto propagate errors to your error boundary, and usehasCustomers === falseinstead of!hasCustomersto distinguish an explicit false result from an error state.🔧 Suggested fix
const { id: organizationId } = Route.useParams(); - const { data: hasCustomers, isLoading } = useQuery( - convexQuery(api.customers.queries.hasCustomers, { organizationId }), - ); + const query = convexQuery(api.customers.queries.hasCustomers, { organizationId }); + const { data: hasCustomers, isLoading } = useQuery({ + ...query, + throwOnError: true, + }); if (isLoading) { return <CustomersTableSkeleton organizationId={organizationId} />; } - if (!hasCustomers) { + if (hasCustomers === false) { return <CustomersEmptyState organizationId={organizationId} />; }
🤖 Fix all issues with AI agents
In `@services/platform/app/features/automations/executions/executions-client.tsx`:
- Around line 54-58: The execution journal query currently only destructures
data (journal) from useQuery, so query failures silently produce undefined for
JsonViewer; update the call to useQuery(convexQuery(...)) to either destructure
isError and error alongside data (e.g., { data: journal, isError, error }) and
handle/render the error before passing data to JsonViewer, or configure the
query to propagate errors by adding throwOnError: true to the useQuery options;
reference the useQuery call,
convexQuery(api.wf_executions.queries.getExecutionStepJournal, execution._id ? {
executionId: execution._id } : 'skip'), and ensure any early return/render uses
isError/error or lets the parent error boundary catch it so JsonViewer doesn't
receive undefined.
In
`@services/platform/app/features/automations/triggers/components/events-section.tsx`:
- Around line 62-67: Replace the current skip-sentinel pattern with TanStack
Query's enabled option: stop passing 'skip' into convexQuery and instead spread
convexQuery(api.wf_definitions.queries.listAutomationRoots, { organizationId })
into useQuery and add enabled: hasWorkflowFilter; update the const { data:
workflows } = useQuery(...) call so it uses enabled: hasWorkflowFilter while
still calling convexQuery with the actual params (organizationId).
In
`@services/platform/app/features/automations/triggers/components/webhooks-section.tsx`:
- Around line 46-50: The initial query result currently only destructures data
(webhooks) from useQuery, so on first render the component shows DataTable's
empty state; update the useQuery call to also destructure isLoading (from
convexQuery(api.workflows.triggers.queries.getWebhooks, { workflowRootId })) and
then either render a loading indicator while isLoading is true or pass isLoading
into DataTable (or conditionally render a skeleton/spinner before rendering
DataTable) so users don't briefly see the "no webhooks" empty state.
In `@services/platform/app/features/chat/hooks/use-human-input-requests.ts`:
- Around line 26-31: The query is passing the string 'skip' as args to
convexQuery which `@convex-dev/react-query` doesn't accept; update the useQuery
call that wraps
convexQuery(api.approvals.queries.getHumanInputRequestsForThread, threadId ? {
threadId } : 'skip') to instead always pass valid args (e.g., undefined or omit
args) and control execution with TanStack Query's enabled option: compute
enabled = Boolean(threadId) and pass { enabled } as the third parameter to
useQuery (or in the options object for convexQuery wrapper) so the query only
runs when threadId is present; modify the call referencing convexQuery,
useQuery, api.approvals.queries.getHumanInputRequestsForThread, and threadId
accordingly.
In `@services/platform/app/features/documents/components/documents-client.tsx`:
- Around line 107-109: Destructure and expose the query loading state from the
useQuery call so the component can explicitly use it instead of checking
documentsResult === undefined; update the useQuery invocation that currently
returns data as documentsResult (via convexQuery and
api.documents.queries.listDocuments) to also destructure isLoading, and change
the skeleton/initial-loading check (currently using documentsResult ===
undefined at the skeleton render) to use isLoading for clarity and future
refetch handling.
In
`@services/platform/app/features/settings/account/components/account-form-client.tsx`:
- Around line 52-58: The component currently branches on hasCredential before
the TanStack Query is guaranteed to have succeeded; update the query guard to
check the query's isSuccess (in addition to or instead of isCredentialLoading)
so that rendering of the set-password flow only happens when data is defined.
Locate the useQuery call that uses
convexQuery(api.accounts.queries.hasCredentialAccount, {}) and the variables
hasCredential/isCredentialLoading, and change the conditional to require
isSuccess (from useQuery) before using hasCredential to decide whether to show
the set-password form.
In `@services/platform/app/hooks/use-cached-paginated-query.ts`:
- Around line 35-40: The current cache write skips when result.results.length
=== 0 causing empty-list loading flashes; change the logic in
use-cached-paginated-query to write to paginatedQueryCache for the given
cacheKey whenever result.status !== 'LoadingFirstPage' (remove the
results.length > 0 guard) and continue to set wasExhausted: result.status ===
'Exhausted' so empty result sets are cached as well.
- Around line 14-39: paginatedQueryCache is unbounded and can leak memory; add a
small eviction policy (e.g., LRU or size cap) to bound growth: update the
module-level paginatedQueryCache usage (and/or build a wrapper around it) so
that when inserting in useCachedPaginatedQuery you enforce a maxEntries limit
(evict oldest entry(s) when size > maxEntries) and optionally store a timestamp
to support TTL-based purge; reference the paginatedQueryCache Map, the
buildCacheKey function, and the insertion point inside useCachedPaginatedQuery
to implement the eviction logic.
In `@services/platform/app/hooks/use-convex-auth.ts`:
- Around line 8-10: The current useQuery call destructures isLoading but React
Query v5 changed its semantics; decide whether you mean "data not ready" (use
isPending) or "initial fetch spinner only" (keep isLoading). Update the
destructured variable from
useQuery(convexQuery(api.users.queries.getCurrentUser, {})) to use isPending
when the intent is to indicate “user data is not ready,” or retain isLoading if
you specifically want the initial-fetch-only behavior, and ensure the rest of
this hook/component uses the chosen symbol (isPending or isLoading)
consistently.
In `@services/platform/app/routes/dashboard/`$id/_knowledge.tsx:
- Around line 25-31: The component currently only checks isLoading and falls
back to a default 'member' role when userContext is undefined; update the
useQuery handling to also destructure isError and error from the useQuery call
(alongside data: userContext and isLoading) for the
convexQuery(api.members.queries.getCurrentMemberContext, { organizationId }),
and add an early return or distinct error UI when isError is true (render a
simple error message or error layout instead of continuing to render with the
default role); ensure any places that assume userContext exist (role checks, nav
permission logic) only run when userContext is defined.
In `@services/platform/app/routes/dashboard/`$id/_knowledge/tone-of-voice.tsx:
- Around line 64-68: The useQuery call for
convexQuery(api.tone_of_voice.queries.hasExampleMessages, { organizationId })
returns data that is never used — only isLoading is read into isExamplesLoading;
remove this query to avoid the unnecessary network request or, if the
boolean/data is actually required, destructure and use its data (e.g., const {
data: hasExamples, isLoading: isExamplesLoading } = useQuery(...)) so the result
is consumed; update any logic currently relying on isExamplesLoading to use
isToneLoading if you remove the query, and delete the unused isExamplesLoading
variable and its hook invocation to eliminate the redundant call.
In
`@services/platform/app/routes/dashboard/`$id/automations/$amId/configuration.tsx:
- Around line 59-63: The useQuery call for
convexQuery(api.wf_definitions.queries.getWorkflow, { wfDefinitionId:
automationId }) only destructures data: workflow, causing the UI to stay in a
skeleton on query errors; update the hook call to also destructure isError (and
isLoading if helpful) from useQuery and change the rendering logic in the
component to show a user-facing error state/message when isError is true
(instead of treating missing workflow as loading), while continuing to show the
skeleton only when isLoading is true or when workflow is legitimately undefined
during loading.
In `@services/platform/app/routes/dashboard/`$id/settings/account.tsx:
- Around line 43-45: The current conditional returns AccountSkeleton for both
loading and missing memberContext; change the logic so AccountSkeleton is
rendered only when isLoading is true, and when isLoading is false but
memberContext is falsy render an explicit error/empty state (e.g., an error
message, a PermissionDenied/NotFound component, or a redirect) to avoid an
infinite skeleton; update the conditional around isLoading and memberContext in
the component (the block that currently references isLoading, memberContext, and
AccountSkeleton) to implement these two distinct branches.
In `@services/platform/app/routes/dashboard/`$id/settings/integrations.tsx:
- Around line 99-109: The conditional rendering block redundantly checks
!memberContext along with isMemberLoading before returning <IntegrationsSkeleton
/>; simplify by removing the redundant !memberContext check (leave
isMemberLoading and the other loading flags intact) so the condition reads only
on isMemberLoading, isShopifyLoading, isCirculyLoading, isProtelLoading,
isEmailLoading, and isSsoLoading when deciding to return IntegrationsSkeleton;
update the condition that references isMemberLoading and memberContext in the
file to remove memberContext or, if you prefer defensive code, add a brief
comment next to the condition explaining why memberContext is kept.
In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx:
- Around line 86-92: The audit logs query is firing for non-admin users; update
the query that creates auditLogs (look for the useQuery/useAuditLogs call that
returns isLogsLoading/auditLogs) to add an enabled option so it only runs when
member status is known and the user is admin (e.g. enabled: !isMemberLoading &&
!!memberContext && memberContext.isAdmin). This prevents the unnecessary network
request while preserving backend authorization.
In `@services/platform/app/routes/dashboard/`$id/settings/organization.tsx:
- Around line 74-79: The skeleton loading guard currently waits on isOrgLoading
and isMembersLoading before checking membership, causing non-admins to see the
skeleton; update the conditional logic around isMemberLoading, isOrgLoading,
isMembersLoading, memberContext and members so that once memberContext is
resolved you short‑circuit and render AccessDenied for non‑admins immediately
(i.e., check memberContext and the admin flag first and return AccessDenied
before waiting for org/members loading), otherwise keep the existing skeleton
path when membership is still loading.
In `@services/platform/app/routes/dashboard/`$id/settings/teams.tsx:
- Around line 48-56: The current check `if (isLoading || !memberContext)`
conflates loading, a legitimate null response, and errors causing
TeamsSettingsSkeleton to show indefinitely; modify the component using the
existing useQuery / convexQuery call to: 1) only render TeamsSettingsSkeleton
when isLoading is true; 2) explicitly handle memberContext === null (from
api.members.queries.getCurrentMemberContext) as the unauthenticated case and
render the appropriate unauthenticated/empty state or redirect; and 3) handle
query errors via useQuery’s isError or error fields to show an error UI instead
of the skeleton; update references to isLoading, memberContext,
api.members.queries.getCurrentMemberContext, and TeamsSettingsSkeleton
accordingly.
Swap all `useQuery` calls from `convex/react` to `@tanstack/react-query` + `convexQuery()` from `@convex-dev/react-query`. Data now persists in the QueryClient cache across navigations (staleTime: Infinity, gcTime: 5min), eliminating skeleton flashes on return visits. Add `useCachedPaginatedQuery` hook that wraps `usePaginatedQuery` with a module-level cache so paginated lists (automations, products, customers, vendors, websites, conversations) also show cached data instantly on re-navigation while the WebSocket subscription re-establishes.
3d94b5d to
957409e
Compare
Summary
useQuerycalls fromconvex/reactto@tanstack/react-query+convexQuery()from@convex-dev/react-query, eliminating skeleton flashes on navigation by leveraging TanStack Query's built-in cache (staleTime: Infinityset byconvexQuery(),gcTime: 5mindefault)useCachedPaginatedQueryhook — a drop-in replacement forusePaginatedQuerythat caches results in a module-levelMapacross unmount/remount cycles, returning cached data instantly while the WebSocket subscription re-establishesastype casts in factory hooks with@ts-expect-errordirectives per project lint rulesWhat changed
use-convex-auth,use-list-teams, chat approval hooks,use-audit-logsdashboard/$id,dashboard/index,create-organization,log-in,sign-up_knowledge,settingsautomation-navigation,automation-assistant,automation-tester,executions-client,triggers-client, agent configuration routesuse-entity-data-factory,use-offline-entity-data— replacedas any/aswith@ts-expect-erroruseCachedPaginatedQueryTest plan
npm run typecheck --workspace=@tale/platform)npm run lint --workspace=@tale/platform)Summary by CodeRabbit