docs(mutations): document return type semantics for mutation hooks #16
Conversation
Add comprehensive JSDoc documentation explaining mutation return value behavior: - MutationResult interface: Detailed docs with code examples showing all three patterns (check return value, try/catch, reactive status flags) - MutationOptions.throwOnError: Clear explanation of both modes - useCreateVariable: Added return value section and improved example - useUpdateVariable: Added return value section and improved example - useDeleteVariable: Added return value section and improved example - useBulkUpdateVariables: Added return value section and improved example Key documentation points: - mutate() returns Promise<TData | undefined> - On success: returns TData - On error (throwOnError: false): returns undefined, error in state - On error (throwOnError: true): throws error Also fixes: - src/utils/retry.ts: Changed /* istanbul ignore next */ to /* c8 ignore next */ for proper Codecov/Vitest V8 coverage exclusion Refs: Codex Audit Item #3, #9
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
==========================================
+ Coverage 91.28% 91.37% +0.09%
==========================================
Files 36 36
Lines 987 986 -1
Branches 284 283 -1
==========================================
Hits 901 901
+ Misses 85 84 -1
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive JSDoc documentation to explain mutation return value behavior and error handling semantics for mutation hooks. The documentation clarifies how the mutate() function returns Promise<TData | undefined>, with different behavior based on the throwOnError option.
Key changes:
- Enhanced MutationResult and MutationOptions interfaces with detailed documentation and code examples
- Added "Return Value" sections to all mutation hooks explaining success/error handling patterns
- Updated code examples across all hooks to demonstrate async/await usage and result checking
- Fixed coverage comment syntax from istanbul to c8 for Vitest/V8 compatibility
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/retry.ts | Updated coverage exclusion comment from istanbul to c8 for Vitest/V8 coverage tools |
| src/types/mutations.ts | Enhanced MutationOptions.throwOnError and MutationResult with comprehensive documentation including three recommended usage patterns |
| src/hooks/useCreateVariable.ts | Added return value documentation section and improved example with async/await pattern |
| src/hooks/useUpdateVariable.ts | Added return value documentation section and improved example with async/await pattern |
| src/hooks/useDeleteVariable.ts | Added return value documentation section and improved example with async/await pattern |
| src/hooks/useBulkUpdateVariables.ts | Added return value documentation section and improved example with async/await pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * // Pattern 3: Use status flags (reactive) | ||
| * if (isSuccess) { | ||
| * console.log('Created:', data); | ||
| * } | ||
| * if (isError) { | ||
| * console.error('Failed:', error); |
There was a problem hiding this comment.
In Pattern 3, the variables isSuccess, isError, data, and error are referenced without context. These are properties of the MutationResult object and should be shown as being destructured from the hook or accessed via dot notation to provide accurate guidance to developers.
| * // Pattern 3: Use status flags (reactive) | |
| * if (isSuccess) { | |
| * console.log('Created:', data); | |
| * } | |
| * if (isError) { | |
| * console.error('Failed:', error); | |
| * // Pattern 3: Use status flags (reactive) from the mutation result | |
| * const mutation = useCreateVariable(); | |
| * | |
| * if (mutation.isSuccess) { | |
| * console.log('Created:', mutation.data); | |
| * } | |
| * if (mutation.isError) { | |
| * console.error('Failed:', mutation.error); |
| * ```ts | ||
| * // Pattern 1: Check return value (when throwOnError is false) | ||
| * const result = await mutate(payload); | ||
| * if (result === undefined) { | ||
| * // Check error state | ||
| * console.error('Mutation failed:', error); | ||
| * } else { | ||
| * // Use result | ||
| * console.log('Created:', result); | ||
| * } |
There was a problem hiding this comment.
In Pattern 1, the variable error is referenced without context. The code example shows console.error('Mutation failed:', error); but error is not in scope within this standalone example - it's a property of the MutationResult object. This should be shown as accessing the error from the mutation result object (e.g., destructured from the hook or accessed via dot notation) to provide accurate guidance to developers.
1 participant
Add comprehensive JSDoc documentation explaining mutation return value behavior:
Key documentation points:
Also fixes:
Refs: Codex Audit Item #3, #9