-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Adding support for pagination in SelectComponent #17142
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.
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
Adding support for pagination in SelectComponent #17142
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
WalkthroughThe updates introduce improved caching and pagination to component and app selection in the React package. New hooks and enhanced logic enable client-side search with debounce, infinite scrolling for component lists, and more stable query keys for better cache management. No public API signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SelectComponent
participant useComponentsWithPagination
participant Server
User->>SelectComponent: Type in search box
SelectComponent->>SelectComponent: Debounce input (300ms)
SelectComponent->>useComponentsWithPagination: Fetch components (with search, pagination)
useComponentsWithPagination->>Server: Request components (limit, after cursor)
Server-->>useComponentsWithPagination: Return components, pageInfo
useComponentsWithPagination-->>SelectComponent: Provide filtered components, hasMore, loadMore
User->>SelectComponent: Scrolls to end, clicks "Load more"
SelectComponent->>useComponentsWithPagination: loadMore()
useComponentsWithPagination->>Server: Request next page
Server-->>useComponentsWithPagination: Return next page
useComponentsWithPagination-->>SelectComponent: Append new components
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/connect-react/src/components/SelectApp.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs packages/connect-react/src/components/SelectComponent.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs packages/connect-react/src/hooks/use-component.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs
✨ Finishing Touches
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. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/connect-react/src/hooks/use-component.tsx (1)
24-26:gcTimecompatibilityRepeat of earlier comment – verify library version before merging.
🧹 Nitpick comments (4)
packages/connect-react/src/hooks/use-components.tsx (1)
41-47: Consider forwarding the search term to the backend
queryParamsomitsq, so every page fetch returns an unfiltered slice that you then filter client-side. Passingqwould reduce payload size and the number of pages to walk through.packages/connect-react/src/components/SelectApp.tsx (1)
116-116: Minor typing nit
onChange={(o) => onChange?.(o ? (o as AppResponse) : undefined)}
react-selectalready gives you a typed option if you add a generic parameter toSelect<AppResponse>. That would eliminate the cast.packages/connect-react/src/components/SelectComponent.tsx (2)
53-64: Client-side filtering can miss results not yet paged inBecause you fetch unfiltered pages of 50, a component that matches the search term but lives beyond the pages already loaded will be invisible until the user keeps clicking “Load more”. Forwarding
searchQueryto the backend (or resetting pagination on query change) will give a better UX.
78-97: Load-more UXTying the button display to
!isLoadinghides the button while the first page loads, but it still shows during a “load more” request. Consider also hiding/disabled state whileisLoadingMoreis true to avoid duplicate clicks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/connect-react/src/components/SelectApp.tsx(2 hunks)packages/connect-react/src/components/SelectComponent.tsx(2 hunks)packages/connect-react/src/hooks/use-apps.tsx(1 hunks)packages/connect-react/src/hooks/use-component.tsx(1 hunks)packages/connect-react/src/hooks/use-components.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/connect-react/src/components/SelectApp.tsx (1)
packages/sdk/src/shared/index.ts (1)
AppResponse(127-127)
🔇 Additional comments (4)
packages/connect-react/src/hooks/use-components.tsx (1)
21-23: ConfirmgcTimesupport in your React-Query version
gcTimeexists only in TanStack Query v5 (currently RC). If the project is still on v4, the option will be ignored (or cause a TS error) and you should keep usingcacheTime.packages/connect-react/src/hooks/use-apps.tsx (1)
16-18: ValidategcTimevscacheTimeSee previous note in
use-components.tsx; ensure@tanstack/react-queryv5 is actually in use.packages/connect-react/src/components/SelectApp.tsx (1)
27-41: Nice cleanup on the debounce logicThe cancellation guard prevents set-state-after-unmount issues. 👏
packages/connect-react/src/components/SelectComponent.tsx (1)
25-40: Debounce effect looks goodGuard + cleanup prevent stale updates. All good.
| // Update pagination state | ||
| if (!cancelled) { | ||
| const pageInfo = data.page_info; | ||
| setHasMore(pageInfo ? (pageInfo.count >= 50) : false); | ||
| setIsLoadingMore(false); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
hasMore heuristic may misfire
page_info.count >= 50 assumes the API always returns exactly limit items until exhaustion. Prefer a flag from the API (has_next, next_cursor) or compare data.length < limit to decide.
🤖 Prompt for AI Agents
In packages/connect-react/src/hooks/use-components.tsx around lines 82 to 87,
the current logic sets hasMore based on page_info.count >= 50, which assumes the
API always returns the full limit of items until no more are available. To fix
this, update the logic to use a more reliable indicator from the API such as a
has_next flag or next_cursor if available, or alternatively compare the length
of the returned data array to the requested limit to determine if more items
exist. Adjust setHasMore accordingly to reflect this improved heuristic.
WHY
Summary by CodeRabbit