⚡ Optimize fetchData: use Promise.all and safe pagination #149
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughPagination and data fetching logic for GitHub issues and pull requests were refactored. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeComponent
participant useGitHubData
participant GitHubAPI
User->>HomeComponent: Submit username / change page / change tab
HomeComponent->>useGitHubData: fetchData(username, page, perPage)
useGitHubData->>GitHubAPI: Fetch issues (paginated)
useGitHubData->>GitHubAPI: Fetch PRs (paginated)
GitHubAPI-->>useGitHubData: Return items + total counts
useGitHubData-->>HomeComponent: Update issues, PRs, total counts
HomeComponent-->>User: Render updated table and pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/hooks/useGitHubData.ts (3)
9-29: LGTM with minor suggestions for improvement.The refactored pagination logic is cleaner and more robust. The safe array check and explicit
per_page: 100parameter are good improvements.However, the comment on line 9 is misleading since this function is actually used with the GitHub Search API (
GET /search/issues), not REST endpoints.Consider updating the comment for accuracy:
- // Helper to fetch multiple pages from REST endpoint (not search API) + // Helper to fetch multiple pages from GitHub Search API with paginationAlso consider adding error handling within the pagination loop to make it more resilient to individual page failures.
38-39: Consider production-ready logging approach.The console timing is useful for measuring the optimization impact during development.
For production code, consider using a proper logging library or removing console statements:
- console.time('fetchData'); + // TODO: Replace with proper logging in production + console.time('fetchData');
53-56: LGTM!State updates are handled correctly, and the console timing provides useful performance metrics.
Same suggestion as earlier regarding production-ready logging for the
console.timeEndcall.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useGitHubData.ts(3 hunks)
🔇 Additional comments (2)
src/hooks/useGitHubData.ts (2)
40-51: Excellent optimization! This addresses the core PR objective.The Promise.all implementation enables concurrent fetching of issues and PRs, which should significantly reduce loading times as intended. The query parameters are correctly structured for both searches.
58-58: Good improvement to error handling.The fallback error message ensures users always receive meaningful feedback, even when
err.messageis falsy.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/Home/Home.tsx (1)
183-228: Consider pagination count consistency with filtering.The table implementation is solid, but there's a potential UX inconsistency: the pagination uses
totalCountfrom the API (total unfiltered results) while the table showscurrentFilteredData(filtered results). This means if a user applies filters, the pagination count won't match the visible data count.Consider whether pagination should reflect filtered data count or API total count based on your UX requirements.
Otherwise, the implementation is excellent:
- Proper loading state handling
- Clean table structure with appropriate data display
- Correct pagination configuration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/hooks/useGitHubAuth.ts(1 hunks)src/hooks/useGitHubData.ts(1 hunks)src/hooks/usePagination.ts(0 hunks)src/pages/Home/Home.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- src/hooks/usePagination.ts
✅ Files skipped from review due to trivial changes (1)
- src/hooks/useGitHubAuth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useGitHubData.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/pages/Home/Home.tsx (2)
src/hooks/useGitHubAuth.ts (1)
useGitHubAuth(4-23)src/hooks/useGitHubData.ts (1)
useGitHubData(3-65)
🔇 Additional comments (7)
src/pages/Home/Home.tsx (7)
1-1: LGTM: Added necessary import for new useEffect usage.The addition of
useEffectimport is required for the new pagination effect implementation.
41-43: LGTM: Hook integration updated correctly.The destructuring of both hooks is updated to include the new state variables (
totalIssues,totalPrs) from the optimizeduseGitHubDatahook.
46-53: LGTM: Filter state management looks good.The addition of pagination and filter state variables provides good separation of concerns and follows React best practices.
62-70: LGTM: Event handlers updated correctly.Good implementation:
- Resetting page to 0 on new search prevents pagination issues
- Proper conversion between 0-based UI pagination and 1-based API pagination
- Clean and straightforward page change handler
72-115: LGTM: Excellent consolidation of filtering logic.The new
filterDatafunction provides several improvements:
- Centralizes all filtering logic for better maintainability
- Handles all filter types correctly (state, title, repository, date range)
- Properly handles the "merged" state for pull requests
- Uses immutable filtering patterns with spread operator
- Clear data flow with
currentRawDataandcurrentFilteredData
149-175: LGTM: Great UI/UX improvements.Excellent reorganization of the interface:
- Filters are logically grouped with responsive flex layout
- Tabs display total counts for better user awareness
- Side-by-side tabs and state filter with responsive wrapping
- Page reset on tab change prevents pagination errors
- Clean, modern layout with appropriate spacing
40-231: Excellent implementation aligning with optimization objectives.This refactor successfully achieves the PR goals:
✅ Performance: Leverages the optimized
useGitHubDatahook withPromise.allfor concurrent API calls
✅ Pagination: Implements efficient API-level pagination instead of fetching all data
✅ State Management: Clean internal state management replacing the deletedusePaginationhook
✅ UX: Improved layout, total counts in tabs, and responsive design
✅ Maintainability: Consolidated filtering logic and clear data flowThe component is well-structured and should deliver the promised 1-2 second loading time improvement.
|
🎉🎉 Thank you for your contribution! Your PR #149 has been merged! 🎉🎉 |
2 participants
Related Issue
Description
Optimized the data fetching logic in
useGitHubData.tsto reduce loading time and improve responsiveness. Previously, fetching was slower due to sequential or redundant API calls. This update ensures PRs and Issues are fetched more efficiently, reducing the wait time to just 1–2 seconds.How Has This Been Tested?
Screenshots (if applicable)
Screen.Recording.2025-07-30.111944.mp4
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Style