refactor: extract pagination utility and add hook documentation#123
Conversation
|
@od-hunter is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR refactors pagination handling in bounty hooks by extracting formatting logic into a dedicated utility module. It adds localStorage-backed recent search persistence to the search hook and creates new pagination utility functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
hooks/use-bounty-search.ts (2)
105-115: Consider trimming the search term before storing.The function checks
!term.trim()but stores the originaltermwhich may have leading/trailing whitespace, potentially causing duplicate entries like"foo"and" foo".Proposed fix
const addRecentSearch = (term: string) => { - if (!term.trim()) return; + const trimmed = term.trim(); + if (!trimmed) return; - const newRecent = [term, ...recentSearches.filter((t) => t !== term)].slice( + const newRecent = [trimmed, ...recentSearches.filter((t) => t !== trimmed)].slice( 0, MAX_RECENT_SEARCHES, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-search.ts` around lines 105 - 115, The addRecentSearch function currently checks term.trim() but stores the original term, allowing entries with surrounding whitespace; modify addRecentSearch to compute const trimmed = term.trim(), return early if !trimmed, then build newRecent from trimmed (deduping against recentSearches), call setRecentSearches(newRecent) and persist JSON.stringify(newRecent) to RECENT_SEARCHES_KEY; reference addRecentSearch, recentSearches, MAX_RECENT_SEARCHES, setRecentSearches, and RECENT_SEARCHES_KEY when making the change.
74-85: Consider SSR safety and corrupted data cleanup for localStorage access.If this code runs during server-side rendering,
localStoragewill be undefined and cause an error. Additionally, when JSON parsing fails, the corrupted data remains in localStorage, potentially causing repeated errors on subsequent mounts.Proposed fix
// Load recent searches on mount useEffect(() => { + if (typeof window === "undefined") return; const saved = localStorage.getItem(RECENT_SEARCHES_KEY); if (saved) { try { // eslint-disable-next-line react-hooks/set-state-in-effect - setRecentSearches(JSON.parse(saved)); + const parsed = JSON.parse(saved); + if (Array.isArray(parsed) && parsed.every((item) => typeof item === "string")) { + setRecentSearches(parsed); + } else { + localStorage.removeItem(RECENT_SEARCHES_KEY); + } } catch (e) { console.error("Failed to parse recent searches", e); + localStorage.removeItem(RECENT_SEARCHES_KEY); } } }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-bounty-search.ts` around lines 74 - 85, The effect that loads recent searches should guard against SSR and clean corrupted data: inside the useEffect that reads RECENT_SEARCHES_KEY, first check that window and window.localStorage exist (or typeof window !== "undefined") before calling localStorage.getItem; when JSON.parse fails in the catch block, remove the bad entry from localStorage (localStorage.removeItem(RECENT_SEARCHES_KEY)) and optionally reset state via setRecentSearches([]) to avoid repeated failures; keep the existing setRecentSearches call when parsing succeeds and preserve the try/catch around JSON.parse in the useEffect.lib/utils/pagination.ts (1)
38-53: Consider generalizing the formatter for reuse with other entity types.The function is currently specific to
BountyFieldsFragment. If other paginated entities need similar transformation, a generic version would reduce duplication.Generic alternative
export function formatPaginatedResponse<T>( items: T[], total: number, limit: number, page: number, ): PaginatedResponse<T> { return { data: items, pagination: { page, limit, total, totalPages: calculateTotalPages(total, limit), }, }; } // Keep type-safe alias for bounties if desired export const formatPaginatedBounties = formatPaginatedResponse<BountyFieldsFragment>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils/pagination.ts` around lines 38 - 53, Replace the concrete formatter formatPaginatedBounties with a generic formatPaginatedResponse<T> that accepts items: T[], total, limit, page and returns PaginatedResponse<T> (reusing calculateTotalPages for totalPages); then export a type-safe alias const formatPaginatedBounties = formatPaginatedResponse<BountyFieldsFragment> so existing callers keep working while other entities can reuse the generic formatter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hooks/use-bounty-search.ts`:
- Around line 105-115: The addRecentSearch function currently checks term.trim()
but stores the original term, allowing entries with surrounding whitespace;
modify addRecentSearch to compute const trimmed = term.trim(), return early if
!trimmed, then build newRecent from trimmed (deduping against recentSearches),
call setRecentSearches(newRecent) and persist JSON.stringify(newRecent) to
RECENT_SEARCHES_KEY; reference addRecentSearch, recentSearches,
MAX_RECENT_SEARCHES, setRecentSearches, and RECENT_SEARCHES_KEY when making the
change.
- Around line 74-85: The effect that loads recent searches should guard against
SSR and clean corrupted data: inside the useEffect that reads
RECENT_SEARCHES_KEY, first check that window and window.localStorage exist (or
typeof window !== "undefined") before calling localStorage.getItem; when
JSON.parse fails in the catch block, remove the bad entry from localStorage
(localStorage.removeItem(RECENT_SEARCHES_KEY)) and optionally reset state via
setRecentSearches([]) to avoid repeated failures; keep the existing
setRecentSearches call when parsing succeeds and preserve the try/catch around
JSON.parse in the useEffect.
In `@lib/utils/pagination.ts`:
- Around line 38-53: Replace the concrete formatter formatPaginatedBounties with
a generic formatPaginatedResponse<T> that accepts items: T[], total, limit, page
and returns PaginatedResponse<T> (reusing calculateTotalPages for totalPages);
then export a type-safe alias const formatPaginatedBounties =
formatPaginatedResponse<BountyFieldsFragment> so existing callers keep working
while other entities can reuse the generic formatter.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
hooks/use-bounties.tshooks/use-bounty-search.tshooks/use-infinite-bounties.tslib/utils/pagination.ts
|
Hi @Benjtalkshow & @0xdevcollins please review |
Closes #101
the usebounties already use graphql
Refactor GraphQL bounty hooks with pagination utility and comprehensive documentation
Summary
This PR improves code maintainability and developer experience for the GraphQL-based bounty list hooks by extracting shared pagination logic into a reusable utility and adding comprehensive JSDoc documentation to all related hooks.
Note: The GraphQL migration for bounty list hooks was already completed in a previous implementation. This PR focuses on optimization and documentation.
Changes Made
1. New Pagination Utility (
lib/utils/pagination.ts)calculateTotalPages()helper function for consistent page count calculationformatPaginatedBounties()function to transform GraphQLPaginatedBountiesresponses to the internalPaginatedResponse<BountyFieldsFragment>formatuseBounties()anduseInfiniteBounties()hooks2. Enhanced Hook Documentation
hooks/use-bounties.tsformatPaginatedBounties()utilityhooks/use-infinite-bounties.tsflatMappattern for accessing all bountiesformatPaginatedBounties()utilityhooks/use-bounty-search.tsBenefits
✅ Reduced Code Duplication - Pagination transformation logic now centralized
✅ Improved Maintainability - Changes to response mapping only need to be made in one place
✅ Better Documentation - JSDoc with examples helps developers use hooks correctly
✅ Type Safety - Utility function ensures consistent pagination object shape
✅ Easier Testing - Pagination logic can now be unit tested independently
GraphQL API Integration
All bounty list operations use the GraphQL API:
useBounties()- Uses generateduseBountiesQueryhookuseInfiniteBounties()- UsesBountiesDocumentwith GraphQL fetcheruseBountySearch()- UsesBountiesDocumentwith search variablesThe GraphQL
PaginatedBountiesresponse shape is properly mapped to internal formats:Testing
search-command.tsxcontinues to work with enhanceduseBountySearch()Breaking Changes
None - All changes are backward compatible. The hooks' public interfaces remain identical.
Files Modified
lib/utils/pagination.tshooks/use-bounties.tshooks/use-infinite-bounties.tshooks/use-bounty-search.tsHow to Use This PR
lib/utils/pagination.tsSummary by CodeRabbit
New Features
Refactor