perf(platform): migrate to TanStack Query#463
Conversation
yannickmonney
force-pushed
the
feat/tanstack-query-migration
branch
from
541ef1c to
9f8d9b3
Compare
📝 WalkthroughWalkthroughThis PR systematically refactors mutation and action hooks across the codebase to change their return type signatures. Previously, hooks returned objects with Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
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/features/tone-of-voice/components/tone-of-voice-form-client.tsx (1)
238-246:⚠️ Potential issue | 🟡 MinorGenerate button inside
<Form>lackstype="button"— will trigger form submission.This is pre-existing, but since you're modifying this area: the generate button at Line 238 is inside the
<Form>(Line 215). Withouttype="button", the HTML default istype="submit", so clicking it will triggerhandleSubmit(onSubmit)in addition tohandleGenerateTone.Proposed fix
<Button variant="outline" + type="button" onClick={handleGenerateTone} disabled={isGenerating} >services/platform/app/routes/dashboard/$id/automations/$amId/configuration.tsx (1)
56-61: 🧹 Nitpick | 🔵 TrivialCode on line 61 is correct as written;
await updateWorkflow({...})works becauseuseMutationfrom convex/react returns a callableReactMutationobject.However, the manual
isSavingstate is unnecessary boilerplate. SinceupdateWorkflowis aReactMutationobject with its ownisPendingproperty, consider removing the local state and using the mutation object's tracking directly:const updateWorkflow = useUpdateAutomation(); // In JSX: disabled={updateWorkflow.isPending} // Remove: setIsSaving(true/false) from handleSaveThis simplifies the component by eliminating manual state management that duplicates the mutation object's built-in tracking.
services/platform/app/features/settings/teams/hooks/mutations.ts (1)
4-10: 🛠️ Refactor suggestion | 🟠 MajorRemove the duplicate
useCreateTeamMemberhook—consolidate touseAddTeamMemberor create an alias.Both hooks call
useConvexMutation(api.team_members.mutations.addMember)identically. SinceuseAddTeamMemberhas broader test coverage and is used in team-members-dialog.tsx, keep that name and either removeuseCreateTeamMemberor export it as a deprecated alias for backward compatibility.♻️ Suggested consolidation
-export function useCreateTeamMember() { - return useConvexMutation(api.team_members.mutations.addMember); -} - export function useAddTeamMember() { return useConvexMutation(api.team_members.mutations.addMember); } + +/** `@deprecated` Use useAddTeamMember instead */ +export const useCreateTeamMember = useAddTeamMember;services/platform/app/features/custom-agents/components/custom-agent-row-actions.tsx (1)
89-109:⚠️ Potential issue | 🟡 Minor
handleDeactivateConfirmlacks the early-return guard present in the other handlers.
handleDuplicate,handlePublish, andhandleActivateall guard against double invocation withif (isX) return;, buthandleDeactivateConfirmdoes not. While theConfirmDialogbutton is disabled viaisLoading={isDeactivating}, adding the guard would be consistent and would protect against programmatic re-invocation.Proposed fix
const handleDeactivateConfirm = useCallback(async () => { + if (isDeactivating) return; setIsDeactivating(true);services/platform/app/features/custom-agents/components/custom-agent-active-toggle.tsx (1)
50-88: 🧹 Nitpick | 🔵 TrivialMinor: both flags are unconditionally reset in
finally, but only one is ever set.Lines 78–79 reset both
isPublishingandisActivatingregardless of which branch was taken. This works correctly since resetting an already-false flag is a no-op, but it's slightly misleading to read.No functional issue — just a readability note.
🤖 Fix all issues with AI agents
In
`@services/platform/app/features/automations/components/automation-navigation.tsx`:
- Around line 63-68: The component currently manages three local loading states
(isPublishing, isCreatingDraft, isUnpublishing) for the mutations returned by
usePublishAutomationDraft, useCreateDraftFromActive, and useUnpublishAutomation;
replace these manual flags with the mutation objects' built-in pending state
(e.g., usePublishAutomationDraft().isPending) so the UI derives loading from the
hooks instead of setIsPublishing/setIsCreatingDraft/setIsUnpublishing and remove
the try/finally toggles; this prevents setState-on-unmounted warnings and lets
callers use isPending/isError/isSuccess from the mutation objects for consistent
state management.
In
`@services/platform/app/features/automations/components/automation-row-actions.tsx`:
- Around line 40-45: Add explicit loading guards for publish and duplicate like
the others: introduce state flags (e.g., isPublishing and isDuplicating), guard
early in handlePublish and handleDuplicate to return if already true, set the
respective flag to true before calling republishAutomation.mutate/mutateAsync
and duplicateAutomation.mutate/mutateAsync and clear it in a finally block;
update any UI props (disable/aria-busy) for the corresponding menu items so the
menu reflects the loading state and prevents double submissions. Ensure you
reference handlePublish, handleDuplicate, republishAutomation, and
duplicateAutomation when applying these changes.
In
`@services/platform/app/features/custom-agents/components/custom-agent-row-actions.tsx`:
- Around line 36-43: Replace the manual loading flags (isDuplicating,
isPublishing, isDeactivating, isActivating) and their setState usage with the
mutation object's built-in pending state: destructure isPending from the
mutation hooks (useDuplicateCustomAgent, usePublishCustomAgent,
useUnpublishCustomAgent, useActivateCustomAgentVersion) and use those values for
UI/loading state; update the action handlers to call the mutation (mutate or
mutateAsync) directly without setting local flags or relying on finally blocks
so the UI reflects the mutation's isPending automatically.
In
`@services/platform/app/features/settings/account/components/account-form-client.tsx`:
- Line 47: The hook useUpdatePassword returns a UseMutationResult object but the
code currently treats updatePassword as a function; updatePassword calls will
throw. Fix by either (A) destructuring mutateAsync from useUpdatePassword (e.g.,
const { mutateAsync: updatePassword } = useUpdatePassword()) so existing call
sites (await updatePassword(...)) keep working, or (B) keep the full mutation
object (const updatePassword = useUpdatePassword()) and change all call sites to
await updatePassword.mutateAsync(...) and use updatePassword.isPending/isError
as needed; update references around the updatePassword usage and prop shapes
(places referencing updatePassword, mutateAsync, isPending) accordingly.
In
`@services/platform/app/features/settings/api-keys/components/api-key-revoke-dialog.tsx`:
- Around line 29-52: The component is manually managing a loading flag and
re-entry guard around the revoke flow; refactor to consume a TanStack Query
mutation from useRevokeApiKey instead: change useRevokeApiKey(organizationId) to
return a useMutation result and in the component destructure mutateAsync (e.g.,
revokeKey) and isPending (e.g., isRevoking), remove the local useState, the
re-entry guard, and the try/finally boilerplate around handleConfirm, and call
revokeKey(apiKey.id) inside handleConfirm while preserving the existing toast,
onOpenChange(false), and onSuccess calls and error handling. Ensure
useRevokeApiKey's signature and returned types match the component usage
(mutateAsync and isPending).
In
`@services/platform/app/features/settings/organization/hooks/__tests__/mutation-hooks.test.ts`:
- Around line 3-6: The shared mock ignores the mutation name so tests can't
assert each hook wires to the correct API; replace the current mock
(mockMutationFn) with a factory mock that captures the argument passed to
useConvexMutation (e.g. create a mockUseConvexMutation = vi.fn((name) =>
mockMutationFn)) and update the vi.mock to return useConvexMutation:
mockUseConvexMutation, then in each hook test call the hook (e.g.
useRemoveMember()) and assert mockUseConvexMutation was called with the expected
mutation name (e.g. 'removeMember') so each hook is validated against the
correct mutation.
In
`@services/platform/app/features/vendors/hooks/__tests__/mutation-hooks.test.ts`:
- Around line 29-33: Add a beforeEach that calls jest.clearAllMocks() inside the
describe block that tests useBulkCreateVendors so mock state is reset between
tests; locate the describe('useBulkCreateVendors', ...) block around the
useBulkCreateVendors() call and add a beforeEach(() => jest.clearAllMocks())
(consistent with the existing beforeEach usages in the useDeleteVendor and
useUpdateVendor tests) to prevent leaked mock state affecting
mockMutationFn-based assertions.
In `@services/platform/app/hooks/use-convex-mutation.ts`:
- Line 1: The current export re-exports Convex's useMutation (a bare callable)
as useConvexMutation; instead wrap the Convex mutation callable inside
`@tanstack/react-query`'s useMutation so callers get the full UseMutationResult
with isLoading/isError/isSuccess, mutate and mutateAsync and built-in cache
invalidation hooks; specifically, change the implementation of useConvexMutation
to call convex's useMutation to obtain the remote callable, then pass that
callable as the mutationFn into `@tanstack/react-query`'s useMutation (and
useQueryClient for invalidation patterns) and return the resulting mutation
object rather than the raw Convex function.
In `@services/platform/convex/README.md`:
- Around line 92-125: The example action retryRagIndexing references
ragAction.execute but ragAction is neither imported nor declared; update the
example to make it self-contained by either importing/declaring the actual
ragAction used in your codebase (e.g., add an import for ragAction or
instantiate it) or replace the call with a clear placeholder comment or a call
to a provided helper (for example, call a locally declared function like
executeRagOperation(ctx, { operation: 'upload_document', recordId:
args.documentId }) and show its signature) so readers can run the snippet
without an undefined symbol.
| const publishAutomation = usePublishAutomationDraft(); | ||
| const [isPublishing, setIsPublishing] = useState(false); | ||
| const createDraftFromActive = useCreateDraftFromActive(); | ||
| const [isCreatingDraft, setIsCreatingDraft] = useState(false); | ||
| const unpublishAutomation = useUnpublishAutomation(); | ||
| const [isUnpublishing, setIsUnpublishing] = useState(false); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Three parallel local loading states add verbosity — consider the hook's built-in pending state.
The migration introduces three separate useState flags (isPublishing, isCreatingDraft, isUnpublishing) to replace what was previously provided by the hooks' isPending. Each flag is manually toggled in try/finally blocks. This is functionally correct but:
- Manually managed loading states can drift out of sync if the component unmounts mid-flight (no cleanup of the
finallyclause on unmounted components → React state-update-on-unmounted warning). - If
useConvexMutationwraps TanStack Query'suseMutationin the future,isPendingfrom the hook would be the idiomatic and safer approach.
For now the pattern is consistent with the rest of the PR, but worth noting.
Based on learnings: "In services/platform/app/features mutation hooks, the preferred pattern is to return the full mutation object from useConvexMutation... This allows callers to access isPending, isError, isSuccess, and other TanStack Query mutation properties for UI state management."
🤖 Prompt for AI Agents
In
`@services/platform/app/features/automations/components/automation-navigation.tsx`
around lines 63 - 68, The component currently manages three local loading states
(isPublishing, isCreatingDraft, isUnpublishing) for the mutations returned by
usePublishAutomationDraft, useCreateDraftFromActive, and useUnpublishAutomation;
replace these manual flags with the mutation objects' built-in pending state
(e.g., usePublishAutomationDraft().isPending) so the UI derives loading from the
hooks instead of setIsPublishing/setIsCreatingDraft/setIsUnpublishing and remove
the try/finally toggles; this prevents setState-on-unmounted warnings and lets
callers use isPending/isError/isSuccess from the mutation objects for consistent
state management.
| const duplicateAutomation = useDuplicateAutomation(); | ||
| const deleteAutomation = useDeleteAutomation(); | ||
| const republishAutomation = useRepublishAutomation(); | ||
| const unpublishAutomation = useUnpublishAutomation(); | ||
| const [isUnpublishing, setIsUnpublishing] = useState(false); | ||
| const updateAutomation = useUpdateAutomation(); |
There was a problem hiding this comment.
Inconsistent loading-state coverage across mutation handlers.
handleUnpublishConfirm and handleDeleteConfirm track loading state (isUnpublishing / isDeleting) to prevent double-submission and drive UI feedback. However, handlePublish (line 47) and handleDuplicate (line 67) have no such guard — rapid clicks could trigger duplicate mutations.
This is partially mitigated by the dropdown menu closing on click, but consider adding loading guards for publish and duplicate as well for consistency and resilience.
🤖 Prompt for AI Agents
In
`@services/platform/app/features/automations/components/automation-row-actions.tsx`
around lines 40 - 45, Add explicit loading guards for publish and duplicate like
the others: introduce state flags (e.g., isPublishing and isDuplicating), guard
early in handlePublish and handleDuplicate to return if already true, set the
respective flag to true before calling republishAutomation.mutate/mutateAsync
and duplicateAutomation.mutate/mutateAsync and clear it in a finally block;
update any UI props (disable/aria-busy) for the corresponding menu items so the
menu reflects the loading state and prevents double submissions. Ensure you
reference handlePublish, handleDuplicate, republishAutomation, and
duplicateAutomation when applying these changes.
| const [isDuplicating, setIsDuplicating] = useState(false); | ||
| const [isPublishing, setIsPublishing] = useState(false); | ||
| const [isDeactivating, setIsDeactivating] = useState(false); | ||
| const [isActivating, setIsActivating] = useState(false); | ||
| const duplicateAgent = useDuplicateCustomAgent(); | ||
| const publishAgent = usePublishCustomAgent(); | ||
| const unpublishAgent = useUnpublishCustomAgent(); | ||
| const activateVersion = useActivateCustomAgentVersion(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
fd "use-convex-mutation" --extension ts --extension tsx -x cat {}Repository: tale-project/tale
Length of output: 125
🏁 Script executed:
rg -n "export.*useConvexMutation|function useConvexMutation" --type ts --type tsx services/platform/app/hooks/Repository: tale-project/tale
Length of output: 88
🏁 Script executed:
rg -n "useDuplicateCustomAgent|usePublishCustomAgent|useUnpublishCustomAgent|useActivateCustomAgentVersion" --type ts --type tsx -A 3 services/platform/app/features/custom-agents/Repository: tale-project/tale
Length of output: 88
🏁 Script executed:
rg "useDuplicateCustomAgent|usePublishCustomAgent|useUnpublishCustomAgent|useActivateCustomAgentVersion" --type ts -A 3 services/platform/app/features/custom-agents/Repository: tale-project/tale
Length of output: 8226
🏁 Script executed:
fd -e ts -e tsx "use.*custom-agent" services/platform/app/features/custom-agents/hooks/Repository: tale-project/tale
Length of output: 148
🏁 Script executed:
cat services/platform/app/features/custom-agents/hooks/mutations.tsRepository: tale-project/tale
Length of output: 1825
🏁 Script executed:
cat services/platform/app/features/custom-agents/components/custom-agent-row-actions.tsxRepository: tale-project/tale
Length of output: 6035
Consider using mutation object's isPending instead of manual state flags.
Four useState flags duplicate what TanStack Query's mutation object provides via isPending. The hooks (useDuplicateCustomAgent, usePublishCustomAgent, etc.) follow the preferred pattern by returning the full mutation object from useConvexMutation. If the mutation object is accessible in the handlers, destructuring isPending from each hook would eliminate the manual setState(true)/setState(false) boilerplate and guarantee correct state cleanup even if finally blocks are skipped.
🤖 Prompt for AI Agents
In
`@services/platform/app/features/custom-agents/components/custom-agent-row-actions.tsx`
around lines 36 - 43, Replace the manual loading flags (isDuplicating,
isPublishing, isDeactivating, isActivating) and their setState usage with the
mutation object's built-in pending state: destructure isPending from the
mutation hooks (useDuplicateCustomAgent, usePublishCustomAgent,
useUnpublishCustomAgent, useActivateCustomAgentVersion) and use those values for
UI/loading state; update the action handlers to call the mutation (mutate or
mutateAsync) directly without setting local flags or relying on finally blocks
so the UI reflects the mutation's isPending automatically.
| const { t: tCommon } = useT('common'); | ||
| const { t: tToast } = useT('toast'); | ||
| const { mutateAsync: updatePassword } = useUpdatePassword(); | ||
| const updatePassword = useUpdatePassword(); |
There was a problem hiding this comment.
updatePassword is now a mutation object, not a function — calls at Lines 128 and 231 will throw at runtime.
useUpdatePassword() returns the full UseMutationResult object (per the hook at mutations.ts), but Lines 128 and 231 still call await updatePassword({...}) as if it were a function. This will produce a TypeError at runtime.
Either destructure mutateAsync from the hook (reverting to the old pattern), or update all call sites to use updatePassword.mutateAsync(...).
Option A: use mutateAsync at call sites (aligns with returning the full object)
--- a/ChangePasswordForm onSubmit
- await updatePassword({
+ await updatePassword.mutateAsync({
currentPassword: data.currentPassword,
newPassword: data.newPassword,
});
--- b/SetPasswordForm onSubmit
- await updatePassword({
+ await updatePassword.mutateAsync({
newPassword: data.newPassword,
});Option B: destructure mutateAsync in the parent (simpler call sites)
- const updatePassword = useUpdatePassword();
+ const { mutateAsync: updatePassword } = useUpdatePassword();And revert the prop types on Lines 104 and 208 back to:
- updatePassword: ReturnType<typeof useUpdatePassword>;
+ updatePassword: ReturnType<typeof useUpdatePassword>['mutateAsync'];Note: Option A also unlocks updatePassword.isPending in the child components, which could replace the isSubmitting from react-hook-form for a more consistent loading state with the mutation lifecycle. Based on learnings, the preferred pattern is to return the full mutation object so callers can access isPending, isError, isSuccess, etc.
Also applies to: 104-104, 128-131, 208-208, 231-233
🤖 Prompt for AI Agents
In
`@services/platform/app/features/settings/account/components/account-form-client.tsx`
at line 47, The hook useUpdatePassword returns a UseMutationResult object but
the code currently treats updatePassword as a function; updatePassword calls
will throw. Fix by either (A) destructuring mutateAsync from useUpdatePassword
(e.g., const { mutateAsync: updatePassword } = useUpdatePassword()) so existing
call sites (await updatePassword(...)) keep working, or (B) keep the full
mutation object (const updatePassword = useUpdatePassword()) and change all call
sites to await updatePassword.mutateAsync(...) and use
updatePassword.isPending/isError as needed; update references around the
updatePassword usage and prop shapes (places referencing updatePassword,
mutateAsync, isPending) accordingly.
| const [isRevoking, setIsRevoking] = useState(false); | ||
| const revokeKey = useRevokeApiKey(organizationId); | ||
|
|
||
| const handleConfirm = () => { | ||
| const handleConfirm = async () => { | ||
| if (isRevoking) return; | ||
|
|
||
| revokeKey(apiKey.id, { | ||
| onSuccess: () => { | ||
| toast({ | ||
| title: tSettings('apiKeys.keyRevoked'), | ||
| variant: 'success', | ||
| }); | ||
| onOpenChange(false); | ||
| onSuccess?.(); | ||
| }, | ||
| onError: (error) => { | ||
| console.error(error); | ||
| toast({ | ||
| title: tSettings('apiKeys.keyRevokeFailed'), | ||
| variant: 'destructive', | ||
| }); | ||
| }, | ||
| }); | ||
| setIsRevoking(true); | ||
| try { | ||
| await revokeKey(apiKey.id); | ||
| toast({ | ||
| title: tSettings('apiKeys.keyRevoked'), | ||
| variant: 'success', | ||
| }); | ||
| onOpenChange(false); | ||
| onSuccess?.(); | ||
| } catch (error) { | ||
| console.error(error); | ||
| toast({ | ||
| title: tSettings('apiKeys.keyRevokeFailed'), | ||
| variant: 'destructive', | ||
| }); | ||
| } finally { | ||
| setIsRevoking(false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Manual loading state is a symptom of the hook not using useMutation.
This useState-based isRevoking flag, the re-entry guard, and the try/catch/finally boilerplate would all collapse if useRevokeApiKey returned a TanStack Query mutation object. For example:
const { mutateAsync: revokeKey, isPending: isRevoking } = useRevokeApiKey(organizationId);
const handleConfirm = async () => {
try {
await revokeKey(apiKey.id);
toast({ title: tSettings('apiKeys.keyRevoked'), variant: 'success' });
onOpenChange(false);
onSuccess?.();
} catch (error) {
console.error(error);
toast({ title: tSettings('apiKeys.keyRevokeFailed'), variant: 'destructive' });
}
};isPending is managed automatically, and useMutation already prevents concurrent invocations. This ties back to the hook-level suggestion — once the hook is refactored, this component simplifies.
🤖 Prompt for AI Agents
In
`@services/platform/app/features/settings/api-keys/components/api-key-revoke-dialog.tsx`
around lines 29 - 52, The component is manually managing a loading flag and
re-entry guard around the revoke flow; refactor to consume a TanStack Query
mutation from useRevokeApiKey instead: change useRevokeApiKey(organizationId) to
return a useMutation result and in the component destructure mutateAsync (e.g.,
revokeKey) and isPending (e.g., isRevoking), remove the local useState, the
re-entry guard, and the try/finally boilerplate around handleConfirm, and call
revokeKey(apiKey.id) inside handleConfirm while preserving the existing toast,
onOpenChange(false), and onSuccess calls and error handling. Ensure
useRevokeApiKey's signature and returned types match the component usage
(mutateAsync and isPending).
| const mockMutationFn = vi.fn(); | ||
|
|
||
| vi.mock('@/app/hooks/use-convex-mutation', () => ({ | ||
| useConvexMutation: () => ({ | ||
| mutateAsync: mockMutateAsync, | ||
| isPending: false, | ||
| }), | ||
| useConvexMutation: () => mockMutationFn, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Shared mock doesn't verify which API mutation each hook wires to.
Since all hooks share a single mockMutationFn and the useConvexMutation mock ignores its argument, these tests confirm the hooks return the mock and forward calls, but they can't catch if e.g. useRemoveMember accidentally wires to updateMemberRole. Consider capturing and asserting on the argument passed to the useConvexMutation factory:
♻️ Suggested improvement
-const mockMutationFn = vi.fn();
+const mockMutationFn = vi.fn();
+const mockUseConvexMutation = vi.fn(() => mockMutationFn);
vi.mock('@/app/hooks/use-convex-mutation', () => ({
- useConvexMutation: () => mockMutationFn,
+ useConvexMutation: (...args: unknown[]) => mockUseConvexMutation(...args),
}));Then in each test:
it('wires to the correct mutation', () => {
useRemoveMember();
expect(mockUseConvexMutation).toHaveBeenCalledWith('removeMember');
});🤖 Prompt for AI Agents
In
`@services/platform/app/features/settings/organization/hooks/__tests__/mutation-hooks.test.ts`
around lines 3 - 6, The shared mock ignores the mutation name so tests can't
assert each hook wires to the correct API; replace the current mock
(mockMutationFn) with a factory mock that captures the argument passed to
useConvexMutation (e.g. create a mockUseConvexMutation = vi.fn((name) =>
mockMutationFn)) and update the vi.mock to return useConvexMutation:
mockUseConvexMutation, then in each hook test call the hook (e.g.
useRemoveMember()) and assert mockUseConvexMutation was called with the expected
mutation name (e.g. 'removeMember') so each hook is validated against the
correct mutation.
| describe('useBulkCreateVendors', () => { | ||
| it('returns the full mutation result from useConvexMutation', () => { | ||
| it('returns the mutation function from useConvexMutation', () => { | ||
| const result = useBulkCreateVendors(); | ||
| expect(result).toHaveProperty('mutateAsync', mockMutateAsync); | ||
| expect(result).toHaveProperty('isPending', false); | ||
| expect(result).toBe(mockMutationFn); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing beforeEach with clearAllMocks in the useBulkCreateVendors describe block.
The useDeleteVendor and useUpdateVendor blocks both clear mocks before each test, but useBulkCreateVendors does not. If test ordering changes or more tests are added to this block, leaked mock state could cause flaky failures.
🔧 Add beforeEach for consistency
describe('useBulkCreateVendors', () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ });
+
it('returns the mutation function from useConvexMutation', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('useBulkCreateVendors', () => { | |
| it('returns the full mutation result from useConvexMutation', () => { | |
| it('returns the mutation function from useConvexMutation', () => { | |
| const result = useBulkCreateVendors(); | |
| expect(result).toHaveProperty('mutateAsync', mockMutateAsync); | |
| expect(result).toHaveProperty('isPending', false); | |
| expect(result).toBe(mockMutationFn); | |
| }); | |
| describe('useBulkCreateVendors', () => { | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| it('returns the mutation function from useConvexMutation', () => { | |
| const result = useBulkCreateVendors(); | |
| expect(result).toBe(mockMutationFn); | |
| }); |
🤖 Prompt for AI Agents
In
`@services/platform/app/features/vendors/hooks/__tests__/mutation-hooks.test.ts`
around lines 29 - 33, Add a beforeEach that calls jest.clearAllMocks() inside
the describe block that tests useBulkCreateVendors so mock state is reset
between tests; locate the describe('useBulkCreateVendors', ...) block around the
useBulkCreateVendors() call and add a beforeEach(() => jest.clearAllMocks())
(consistent with the existing beforeEach usages in the useDeleteVendor and
useUpdateVendor tests) to prevent leaked mock state affecting
mockMutationFn-based assertions.
| mutationFn: (args: FunctionArgs<Func>) => convexClient.mutation(func, args), | ||
| }); | ||
| } | ||
| export { useMutation as useConvexMutation } from 'convex/react'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
convex react useMutation return type ReactMutation
💡 Result:
In Convex React, useMutation is typed to return a ReactMutation<Mutation>:
useMutation<Mutation>(mutation): ReactMutation<Mutation>whereMutation extends FunctionReference<"mutation">. [1]ReactMutation<Mutation>is a callable interface: calling it returnsPromise<FunctionReturnType<Mutation>>, and it also has helpers like.withOptimisticUpdate(...). [2]
TypeScript examples
import { useMutation, type ReactMutation } from "convex/react";
import { api } from "../convex/_generated/api";
const m = useMutation(api.tasks.create);
// Option 1: infer the exact return type
type M1 = typeof m;
// Option 2: name it explicitly
type M2 = ReactMutation<typeof api.tasks.create>;Sources: Convex React API docs for useMutation [1] and ReactMutation [2].
🌐 Web query:
@tanstack/react-query useMutation return type isPending isError isSuccess
💡 Result:
In @tanstack/react-query, useMutation returns a UseMutationResult<TData, TError, TVariables, TContext> object (aka the mutation “result”) that includes status booleans like isPending, isError, and isSuccess. These booleans are derived from status. [1]
- v5 naming: mutation
status: 'pending' | 'error' | 'success' | 'idle', and the oldisLoadingflag was renamed toisPendingfor mutations. [2] - Usage (typical):
mutation.isPending,mutation.isError,mutation.isSuccessare the standard fields you read for UI state. [3]
TypeScript example
import { useMutation, type UseMutationResult } from '@tanstack/react-query'
type TData = unknown
type TError = Error
type TVariables = { id: string }
const mutation: UseMutationResult<TData, TError, TVariables> = useMutation({
mutationFn: async (vars) => {
// ...
return {} as TData
},
})
// mutation.isPending / mutation.isError / mutation.isSuccessIf you’re on v4: the equivalent flag is isLoading (and status: 'loading'), not isPending. [2]
Sources: [1] [2] [3]
🌐 Web query:
Convex useMutation vs TanStack Query useMutation differences
💡 Result:
What they are
- Convex
useMutation(convex/react): a hook that gives you a typed, stable “mutation handle” for calling a Convex backend mutation over Convex’s client/runtime (WebSocket + reactive data model). It can also be configured with Convex-style optimistic updates. [1][2] - TanStack Query
useMutation(@tanstack/react-query): a hook that manages the lifecycle of an arbitrary async function (mutationFn) (typically an HTTP request), with status flags and callbacks, and optional integration with the Query Cache via invalidation/refetch patterns. [4][5]
Key differences (practical)
-
Data consistency model
- Convex: queries are reactive (kept in sync by Convex), so after a mutation completes, subscribed queries update without you doing “invalidate/refetch” work in the common case. [3]
- TanStack Query: mutations do not automatically update queries; you typically invalidate or manually update cached query data via
queryClientinonSuccess/onMutate. [5]
-
API shape
- Convex:
useMutation(api.foo.bar)returns aReactMutationobject you can call, and you can chain configuration like.withOptimisticUpdate(...). The returned value is stable across renders. [1][2] - TanStack Query:
useMutation({ mutationFn, onSuccess, onError, ... })returns{ mutate, mutateAsync, status, data, error, ... }. [4]
- Convex:
-
Optimistic updates
- Convex: first-class optimistic updates via
.withOptimisticUpdate(...)that temporarily modify local query results and automatically roll back when the mutation finishes. [2] - TanStack Query: optimistic updates are usually implemented using
onMutate+queryClient.setQueryData+ rollback via context inonError. (Supported pattern, but not a single built-in “optimistic update object” tied to a reactive query system.) [5]
- Convex: first-class optimistic updates via
-
Retries / networking behavior
- Convex: when using Convex with TanStack Query, some options are effectively unnecessary/ignored because Convex has its own retry/sync mechanism and its query results are “never stale” in that integration. [3]
- TanStack Query: retry, network mode, mutation cache time, etc., are standard controls for request behavior. [4]
-
Using Convex inside TanStack Query
- If you want TanStack Query’s mutation state machine but execute Convex mutations, Convex provides
useConvexMutation(...)as a wrapper to use as amutationFn. [3]
- If you want TanStack Query’s mutation state machine but execute Convex mutations, Convex provides
Sources: Convex React useMutation API and stability/usage [1], Convex optimistic updates via withOptimisticUpdate [2], Convex guidance for TanStack Query integration and what gets ignored/changed [3], TanStack Query useMutation reference [4], TanStack Query mutation concepts and patterns [5].
useMutation from convex/react returns a bare callable, not a TanStack Query mutation object.
The PR title says "migrate to TanStack Query," but this re-export aliases Convex's own useMutation (returning ReactMutation<T> — a callable), not @tanstack/react-query's useMutation (returning UseMutationResult with isPending, isError, isSuccess, mutate, mutateAsync, etc.). This means all downstream hooks return plain functions, and callers lose built-in loading/error state properties.
Components compensate with local useState flags, but this scatters state management that TanStack Query provides natively. If the intent is to migrate to TanStack Query, wrap the Convex mutation in @tanstack/react-query's useMutation so callers access isPending, isError, isSuccess, and cache invalidation patterns directly.
Per established patterns: hooks should return the full mutation object to expose these state properties for UI management, not require callers to manage state manually.
🤖 Prompt for AI Agents
In `@services/platform/app/hooks/use-convex-mutation.ts` at line 1, The current
export re-exports Convex's useMutation (a bare callable) as useConvexMutation;
instead wrap the Convex mutation callable inside `@tanstack/react-query`'s
useMutation so callers get the full UseMutationResult with
isLoading/isError/isSuccess, mutate and mutateAsync and built-in cache
invalidation hooks; specifically, change the implementation of useConvexMutation
to call convex's useMutation to obtain the remote callable, then pass that
callable as the mutationFn into `@tanstack/react-query`'s useMutation (and
useQueryClient for invalidation patterns) and return the resulting mutation
object rather than the raw Convex function.
| // convex/documents/actions.ts | ||
| import { v } from 'convex/values'; | ||
|
|
||
| export const bulkCreateCustomers = action({ | ||
| import { internal } from '../_generated/api'; | ||
| import { action } from '../_generated/server'; | ||
|
|
||
| export const retryRagIndexing = action({ | ||
| args: { | ||
| organizationId: v.string(), | ||
| customers: v.array( | ||
| v.object({ | ||
| /* fields */ | ||
| }), | ||
| ), | ||
| documentId: v.id('documents'), | ||
| }, | ||
| returns: v.object({ | ||
| success: v.boolean(), | ||
| jobId: v.optional(v.string()), | ||
| error: v.optional(v.string()), | ||
| }), | ||
| handler: async (ctx, args) => { | ||
| // Actions can call mutations and interact with external systems | ||
| for (const customer of args.customers) { | ||
| await ctx.runMutation(api.customers.mutations.createCustomer, { | ||
| organizationId: args.organizationId, | ||
| ...customer, | ||
| }); | ||
| // Actions can call queries/mutations and interact with external systems | ||
| const document = await ctx.runQuery( | ||
| internal.documents.internal_queries.getDocumentByIdRaw, | ||
| { documentId: args.documentId }, | ||
| ); | ||
|
|
||
| if (!document) { | ||
| return { success: false, error: 'Document not found' }; | ||
| } | ||
|
|
||
| const result = await ragAction.execute(ctx, { | ||
| operation: 'upload_document', | ||
| recordId: args.documentId, | ||
| }); | ||
|
|
||
| return { success: result.success, jobId: result.jobId }; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Undefined ragAction in the example will confuse readers.
Line 118 uses ragAction.execute(ctx, {...}) but ragAction is never imported or declared in the snippet. Either add the import/declaration or replace with a generic comment/pseudo-code so the example is self-contained.
📝 Suggested fix
+import { internal } from '../_generated/api';
+import { action } from '../_generated/server';
+
export const retryRagIndexing = action({
args: {
documentId: v.id('documents'),
@@ -118,7 +121,8 @@
- const result = await ragAction.execute(ctx, {
- operation: 'upload_document',
- recordId: args.documentId,
- });
+ // Call external system or internal action for RAG indexing
+ const result = await ctx.runAction(
+ internal.documents.internal_actions.indexDocument,
+ { documentId: args.documentId },
+ );
return { success: result.success, jobId: result.jobId };🤖 Prompt for AI Agents
In `@services/platform/convex/README.md` around lines 92 - 125, The example action
retryRagIndexing references ragAction.execute but ragAction is neither imported
nor declared; update the example to make it self-contained by either
importing/declaring the actual ragAction used in your codebase (e.g., add an
import for ragAction or instantiate it) or replace the call with a clear
placeholder comment or a call to a provided helper (for example, call a locally
declared function like executeRagOperation(ctx, { operation: 'upload_document',
recordId: args.documentId }) and show its signature) so readers can run the
snippet without an undefined symbol.
1 participant
Summary by CodeRabbit