Skip to content

useupdatemutation usedeletemutation should include an id #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KevinBonnoron merged 3 commits into from
Oct 23, 2025
Merged

useupdatemutation usedeletemutation should include an id #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b3afb5
feat(mutation): pass id to use mutation function and allow sync & asy…
KevinBonnoron Oct 23, 2025
dae2330
chore: fix lint issues
KevinBonnoron Oct 23, 2025
133ab95
chore(mutation): add isError field to all mutations
KevinBonnoron Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions examples/useCreateMutation-example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,38 @@ function App() {
}

function CreateMutationExample() {
const { mutate: createPost, isPending, isSuccess, error } = useCreateMutation('posts');
const { data: posts } = useCollection('posts', { perPage: 10 });

const { mutateAsync: createPost, isPending, isSuccess, isError, error } = useCreateMutation('posts');
const [title, setTitle] = useState('');
const [content, setContent] = useState('');
const [status, setStatus] = useState<'draft' | 'published'>('draft');

const handleCreate = async (e: FormEvent) => {
e.preventDefault();
if (!title.trim() || !content.trim()) return;

try {
const newPost = await createPost({
title,
content,
title: title.trim(),
content: content.trim(),
status,
});
console.log('Post created:', newPost);
setTitle('');
setContent('');
setStatus('draft');
} catch (err) {
console.error('Failed to create post:', err);
}
};

if (error) return <div>Error: {error}</div>;

return (
<div>
<h2>Create Post</h2>
<h2>Create New Post</h2>

<form onSubmit={handleCreate} style={{ border: '1px solid #ccc', padding: '20px', marginBottom: '20px' }}>
<h3>Create New Post</h3>
<h3>Create Post</h3>
<div>
<input type="text" value={title} onChange={(e) => setTitle(e.target.value)} placeholder="Post title" required />
</div>
Expand All @@ -60,6 +61,7 @@ function CreateMutationExample() {
{isPending ? 'Creating...' : 'Create Post'}
</button>
{isSuccess && <p style={{ color: 'green' }}>Post created successfully!</p>}
{isError && error && <p style={{ color: 'red' }}>Error: {error}</p>}
</form>

<div>
Expand Down
49 changes: 27 additions & 22 deletions examples/useDeleteMutation-example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,34 @@ function App() {
}

function DeleteMutationExample() {
const { mutate: deletePost, isPending, isSuccess, error } = useDeleteMutation('posts');
const { data: posts } = useCollection('posts', { perPage: 10 });

const [deletingId, setDeletingId] = useState<string | null>(null);
const { mutateAsync: deletePost, isPending, isSuccess, isError, error } = useDeleteMutation('posts', deletingId);

const handleDelete = async (id: string) => {
if (window.confirm('Are you sure you want to delete this post?')) {
setDeletingId(id);
try {
const success = await deletePost(id);
if (success) {
console.log('Post deleted successfully');
}
} catch (err) {
console.error('Failed to delete post:', err);
} finally {
setDeletingId(null);
}
const handleDelete = async (postId: string) => {
if (isPending) {
return;
}

setDeletingId(postId);
try {
await deletePost();
console.log('Post deleted successfully');
} catch (err) {
console.error('Failed to delete post:', err);
} finally {
setDeletingId(null);
}
};
Comment on lines 18 to +35

@coderabbitai coderabbitai Bot Oct 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Closure bug causes "ID is required" error on every delete.

The delete flow is broken due to closure capture timing:

  1. Initial render: deletingId=null → hook initialized with id=null → deletePost will throw
  2. User clicks delete → handleDelete('A') captures current deletePost (with id=null)
  3. Line 26: setDeletingId('A') queues state update but doesn't take effect yet
  4. Line 28: await deletePost() calls the captured function → throws "ID is required" because it uses id=null

The state update happens after the function reference is captured, so deletePost() always uses id=null.

Solution: Move the ID into the function call rather than hook initialization:

-  const [deletingId, setDeletingId] = useState<string | null>(null);
-  const { mutateAsync: deletePost, isPending, isSuccess, isError, error } = useDeleteMutation('posts', deletingId);
+  const { mutateAsync: deletePost, isPending, isSuccess, isError, error } = useDeleteMutation('posts', null);

   const handleDelete = async (postId: string) => {
     if (isPending) {
       return;
     }

-    setDeletingId(postId);
     try {
-      await deletePost();
+      await deletePost(postId);
       console.log('Post deleted successfully');
     } catch (err) {
       console.error('Failed to delete post:', err);
-    } finally {
-      setDeletingId(null);
     }
   };

However, this requires updating the hook API to accept the ID as an argument to mutateAsync instead of at initialization. Alternatively, redesign the hook to not require pre-initialization with an ID.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In examples/useDeleteMutation-example.tsx around lines 18-35, the delete handler
captures a stale deletePost closure because the hook was initialized with
deletingId=null; change the pattern so the ID is passed when calling mutateAsync
rather than at hook initialization: update the example to call the mutation with
the id (e.g., await deletePost(postId)) and keep deletingId state only for
UI/loading purposes; then update the useDeleteMutation hook signature and types
to remove the required id parameter on init and accept the id argument in
mutateAsync (adjust internal logic to read the id from the mutate call), and
update all other call sites/tests to the new API accordingly.


const confirmDelete = (post: RecordModel) => {
if (window.confirm(`Are you sure you want to delete "${post.title}"?`)) {
handleDelete(post.id);
}
};

if (error) return <div>Error: {error}</div>;
if (isError) return <div>Error: {error}</div>;

return (
<div>
Expand All @@ -53,18 +59,17 @@ function DeleteMutationExample() {
<div style={{ marginTop: '10px' }}>
<button
type="button"
onClick={() => handleDelete(post.id)}
disabled={isPending && deletingId === post.id}
onClick={() => confirmDelete(post)}
disabled={isPending}
style={{
backgroundColor: deletingId === post.id ? '#ff6b6b' : '#dc3545',
backgroundColor: '#dc3545',
color: 'white',
border: 'none',
padding: '5px 10px',
borderRadius: '4px',
cursor: deletingId === post.id ? 'not-allowed' : 'pointer',
padding: '8px 16px',
cursor: isPending ? 'not-allowed' : 'pointer',
}}
>
{isPending && deletingId === post.id ? 'Deleting...' : 'Delete'}
{isPending ? 'Deleting...' : 'Delete'}
</button>
</div>
</div>
Expand Down
7 changes: 3 additions & 4 deletions examples/useUpdateMutation-example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ function App() {
}

function UpdateMutationExample() {
const { mutate: updatePost, isPending, isSuccess, error } = useUpdateMutation('posts');
const { data: posts } = useCollection('posts', { perPage: 10 });

const [editingId, setEditingId] = useState<string | null>(null);
const { mutateAsync: updatePost, isPending, isSuccess, isError, error } = useUpdateMutation('posts', editingId);
const [editTitle, setEditTitle] = useState('');
const [editContent, setEditContent] = useState('');
const [editStatus, setEditStatus] = useState<'draft' | 'published'>('draft');
Expand All @@ -26,7 +26,7 @@ function UpdateMutationExample() {
if (!editingId) return;

try {
const updatedPost = await updatePost(editingId, {
const updatedPost = await updatePost({
title: editTitle,
content: editContent,
status: editStatus,
Expand All @@ -53,8 +53,6 @@ function UpdateMutationExample() {
setEditContent('');
};

if (error) return <div>Error: {error}</div>;

return (
<div>
<h2>Update Posts</h2>
Expand All @@ -81,6 +79,7 @@ function UpdateMutationExample() {
Cancel
</button>
{isSuccess && <p style={{ color: 'green' }}>Post updated successfully!</p>}
{isError && error && <p style={{ color: 'red' }}>Error: {error}</p>}
</form>
)}

Expand Down
32 changes: 23 additions & 9 deletions src/hooks/useCreateMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import { usePocketBase } from './usePocketBase';
*
* @example
* ```tsx
* const { mutate, isPending, isSuccess, error } = useCreateMutation<Post>('posts');
* const { mutateAsync, isPending, isSuccess, isError, error } = useCreateMutation<Post>('posts');
*
* const handleCreate = async () => {
* const newPost = await mutate({ title: 'Hello', content: 'World' });
* if (newPost) {
* try {
* const newPost = await mutateAsync({ title: 'Hello', content: 'World' });
* console.log('Created:', newPost);
* } catch (err) {
* console.error('Failed to create post:', err);
* }
* };
* ```
Expand All @@ -28,30 +30,42 @@ export function useCreateMutation<Record extends RecordModel>(collectionName: st
const [isPending, setIsPending] = useState(false);
const [error, setError] = useState<string | null>(null);

const mutate = useCallback(
async (bodyParams: Partial<Record>, options?: RecordOptions): Promise<Record | null> => {
const mutateAsync = useCallback(
async (bodyParams: Partial<Record>, options?: RecordOptions): Promise<Record> => {
try {
setIsPending(true);
setError(null);
const record = await recordService.create(bodyParams, options);
const record = options ? await recordService.create(bodyParams, options) : await recordService.create(bodyParams);
return record as Record;
} catch (err) {
setError(err instanceof Error ? err.message : 'Error creating record');
return null;
const errorMessage = err instanceof Error ? err.message : 'Error creating record';
setError(errorMessage);
throw new Error(errorMessage);
} finally {
setIsPending(false);
}
},
[recordService],
);

const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {
// Error is already handled in mutateAsync
});
},
[mutateAsync],
);
Comment on lines +51 to +58

@coderabbitai coderabbitai Bot Oct 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment per coding guidelines.

The inline comment violates the "no code comments" guideline. Keep the silent catch but remove the comment.

   const mutate = useCallback(
     (bodyParams: Partial<Record>, options?: RecordOptions): void => {
-      mutateAsync(bodyParams, options).catch(() => {
-        // Error is already handled in mutateAsync
-      });
+      mutateAsync(bodyParams, options).catch(() => {});
     },
     [mutateAsync],
   );

As per coding guidelines.

📝 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.

Suggested change
const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {
// Error is already handled in mutateAsync
});
},
[mutateAsync],
);
const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {});
},
[mutateAsync],
);
🤖 Prompt for AI Agents 
In src/hooks/useCreateMutation.ts around lines 51 to 58, remove the inline
comment inside the catch block that reads "// Error is already handled in
mutateAsync" while preserving the silent catch behavior; simply leave the catch
callback body empty (or use an empty statement) so the promise rejection is
swallowed without any inline comments, adhering to the "no code comments"
guideline.


return useMemo(
(): UseCreateMutationResult<Record> => ({
mutate,
mutateAsync,
isPending,
isError: !!error,
error,
isSuccess: !isPending && !error,
}),
[mutate, isPending, error],
[mutate, mutateAsync, isPending, error],
);
}
34 changes: 25 additions & 9 deletions src/hooks/useDeleteMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { CommonOptions } from 'pocketbase';
import type { CommonOptions, RecordModel } from 'pocketbase';
import { useCallback, useMemo, useState } from 'react';
import type { UseDeleteMutationResult } from '../types';
import { usePocketBase } from './usePocketBase';
Expand All @@ -7,6 +7,7 @@ import { usePocketBase } from './usePocketBase';
* Hook for deleting records from a PocketBase collection.
*
* @param collectionName - The name of the PocketBase collection
* @param id - The ID of the record to delete
* @returns An object containing the mutate function and mutation state
*
* @example
Expand All @@ -21,36 +22,51 @@ import { usePocketBase } from './usePocketBase';
* };
* ```
*/
export function useDeleteMutation(collectionName: string): UseDeleteMutationResult {
export function useDeleteMutation<Record extends RecordModel = RecordModel>(collectionName: string, id: Record['id'] | null): UseDeleteMutationResult {
const pb = usePocketBase();
const recordService = useMemo(() => pb.collection(collectionName), [pb, collectionName]);
const [isPending, setIsPending] = useState(false);
const [error, setError] = useState<string | null>(null);

const mutate = useCallback(
async (id: string, options?: CommonOptions): Promise<boolean> => {
const mutateAsync = useCallback(
async (options?: CommonOptions): Promise<void> => {
if (!id) {
throw new Error('ID is required');
}

try {
setIsPending(true);
setError(null);
await recordService.delete(id, options);
return true;
} catch (err) {
setError(err instanceof Error ? err.message : 'Error deleting record');
return false;
const errorMessage = err instanceof Error ? err.message : 'Error deleting record';
setError(errorMessage);
throw new Error(errorMessage);
} finally {
setIsPending(false);
}
},
[recordService],
[recordService, id],
);

const mutate = useCallback(
(options?: CommonOptions): void => {
mutateAsync(options).catch(() => {
// Error is already handled in mutateAsync
});
},
[mutateAsync],
);
Comment thread
KevinBonnoron marked this conversation as resolved.

return useMemo(
(): UseDeleteMutationResult => ({
mutate,
mutateAsync,
isPending,
isError: !!error,
error,
isSuccess: !isPending && !error,
}),
[mutate, isPending, error],
[mutate, mutateAsync, isPending, error],
);
}
31 changes: 24 additions & 7 deletions src/hooks/useUpdateMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { usePocketBase } from './usePocketBase';
*
* @template Record - The record type extending RecordModel
* @param collectionName - The name of the PocketBase collection
* @param id - The ID of the record to update
* @returns An object containing the mutate function and mutation state
*
* @example
Expand All @@ -22,36 +23,52 @@ import { usePocketBase } from './usePocketBase';
* };
* ```
*/
export function useUpdateMutation<Record extends RecordModel>(collectionName: string): UseUpdateMutationResult<Record> {
export function useUpdateMutation<Record extends RecordModel>(collectionName: string, id: Record['id'] | null): UseUpdateMutationResult<Record> {
const pb = usePocketBase();
const recordService = useMemo(() => pb.collection(collectionName), [pb, collectionName]);
const [isPending, setIsPending] = useState(false);
const [error, setError] = useState<string | null>(null);

const mutate = useCallback(
async (id: string, bodyParams: Partial<Record>, options?: RecordOptions): Promise<Record | null> => {
const mutateAsync = useCallback(
async (bodyParams: Partial<Record>, options?: RecordOptions): Promise<Record> => {
if (!id) {
throw new Error('ID is required');
}

try {
setIsPending(true);
setError(null);
const record = options ? await recordService.update(id, bodyParams, options) : await recordService.update(id, bodyParams);
return record as Record;
} catch (err) {
setError(err instanceof Error ? err.message : 'Error updating record');
return null;
const errorMessage = err instanceof Error ? err.message : 'Error updating record';
setError(errorMessage);
throw new Error(errorMessage);
} finally {
setIsPending(false);
}
},
[recordService],
[recordService, id],
);

const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {
// Error is already handled in mutateAsync
});
},
[mutateAsync],
);
Comment on lines +54 to 61

@coderabbitai coderabbitai Bot Oct 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment per coding guidelines.

The inline comment violates the "no code comments" guideline. Keep the silent catch but remove the comment.

   const mutate = useCallback(
     (bodyParams: Partial<Record>, options?: RecordOptions): void => {
-      mutateAsync(bodyParams, options).catch(() => {
-        // Error is already handled in mutateAsync
-      });
+      mutateAsync(bodyParams, options).catch(() => {});
     },
     [mutateAsync],
   );

As per coding guidelines.

📝 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.

Suggested change
const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {
// Error is already handled in mutateAsync
});
},
[mutateAsync],
);
const mutate = useCallback(
(bodyParams: Partial<Record>, options?: RecordOptions): void => {
mutateAsync(bodyParams, options).catch(() => {});
},
[mutateAsync],
);
🤖 Prompt for AI Agents 
In src/hooks/useUpdateMutation.ts around lines 54 to 61 there is an inline
comment inside the catch block; remove that comment and leave the silent catch
body intact (i.e., keep .catch(() => { }) with an empty block), then reformat to
satisfy linting/formatting rules.


return useMemo(
(): UseUpdateMutationResult<Record> => ({
mutate,
mutateAsync,
isPending,
isError: !!error,
error,
isSuccess: !isPending && !error,
}),
[mutate, isPending, error],
[mutate, mutateAsync, isPending, error],
);
}
2 changes: 1 addition & 1 deletion src/types/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ export * from './query-result.type';
export * from './record-transformer.type';
export * from './useAuth.type';
export * from './useCollection.type';
export * from './useCommon.type';
export * from './useCreateMutation.type';
export * from './useDeleteMutation.type';
export * from './useQueryCommon.type';

@coderabbitai coderabbitai Bot Oct 23, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the missing export for UseMutationCommonOptions.

The rename from useCommon.type to useQueryCommon.type is consistent with the interface rename. However, the new UseMutationCommonOptions interface introduced in src/types/useMutationCommon.type.ts is not exported from this index file.

According to the codebase learnings, all exported types must be explicitly defined in src/types/index.ts. Verify whether other mutation type files import UseMutationCommonOptions directly or expect it to be re-exported from here.

Based on learnings.

Run the following script to check imports of UseMutationCommonOptions:

🏁 Script executed:

#!/bin/bash
# Check if UseMutationCommonOptions is imported anywhere and whether the export is needed

echo "=== Checking imports of UseMutationCommonOptions ==="
rg -n "import.*UseMutationCommonOptions" --type=ts --type=tsx

echo -e "\n=== Checking if useMutationCommon.type is referenced ==="
rg -n "from.*useMutationCommon" --type=ts --type=tsx

Length of output: 166

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Check if UseMutationCommonOptions is imported anywhere and whether the export is needed

echo "=== Checking imports of UseMutationCommonOptions ==="
rg -n "import.*UseMutationCommonOptions" --type=ts

echo -e "\n=== Checking if useMutationCommon.type is referenced ==="
rg -n "from.*useMutationCommon" --type=ts

echo -e "\n=== Checking current exports in src/types/index.ts ==="
cat src/types/index.ts

echo -e "\n=== Checking if useMutationCommon.type file exists ==="
fd "useMutationCommon" src/types/

Length of output: 1317

Add missing export for useMutationCommon.type in src/types/index.ts.

The verification confirms the issue: UseMutationCommonOptions is imported directly by three mutation type files (useCreateMutation.type.ts, useDeleteMutation.type.ts, useUpdateMutation.type.ts), and all three of these files are exported from the index. However, src/types/index.ts does not export useMutationCommon.type itself, breaking the consistent export pattern where all type files are re-exported from the index.

Add export * from './useMutationCommon.type'; to src/types/index.ts to make UseMutationCommonOptions accessible to external consumers through the index file.

🤖 Prompt for AI Agents 
In src/types/index.ts around line 8, the file currently exports
useQueryCommon.type but is missing an export for useMutationCommon.type; add the
missing re-export by adding export * from './useMutationCommon.type'; so
UseMutationCommonOptions and related types are available through the index
alongside the other type files.

export * from './useRecord.type';
export * from './useUpdateMutation.type';
Loading