-
Notifications
You must be signed in to change notification settings - Fork 19.5k
refactor: marketplace state management #30702
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
Conversation
Summary of ChangesHello @hyoban, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant architectural refactor of the marketplace's state and data management. It transitions the handling of search and filter parameters to a URL-driven approach using Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request significantly refactors the plugin marketplace's state management and data fetching logic. The core change involves migrating from internal React state and refs for search parameters (query, category, tags) to using URL query parameters managed by the nuqs library. This change removes the showSearchParams prop, as URL synchronization is now the default behavior.
Key changes include:
- Context Refactoring: The
MarketplaceContextis simplified, removing many internal state variables and handlers. It now primarily exposes TanStack Query data and a localsortstate, with URL-driven filters being managed externally. - Data Fetching Hooks:
useMarketplaceCollectionsAndPluginsanduseMarketplacePluginsare refactored to directly accept parameters and return standard TanStack Query results, removing their internal state management. Query keys are standardized using a newmarketplaceKeysfactory. - Server-Side Rendering (SSR) Integration: The main
Marketplacecomponent is converted to an async server component that prefetches initial collection data usinggetQueryClientanddehydratefrom TanStack Query, passing the dehydrated state to a newMarketplaceClientcomponent for client-side hydration. - URL State Management: New individual
nuqshooks (useMarketplaceSearchQuery,useMarketplaceCategory,useMarketplaceTags) are introduced for granular control over URL query parameters, replacing the previous combineduseMarketplaceFiltershook for setting individual parameters. TheMarketplaceContextProvidernow consumes thesenuqshooks directly. - Test Updates: Corresponding tests are updated to reflect these changes, mocking
nuqshooks and verifying direct URL parameter manipulation instead of internal context state.
The review comment specifically addresses a redundant API call issue in handleMoreClick within context.tsx. The original implementation triggered both a direct queryPlugins call and a setUrlFilters update, which then caused a second, debounced fetch via a useEffect hook. The suggested fix is to remove the direct queryPlugins call from handleMoreClick, allowing the useEffect hook (which reacts to URL and sort state changes) to be the single source of truth for triggering data fetches, thus preventing duplicate requests.
- Delete context.tsx, use nuqs for URL state (q, category, tags) - Use jotai for local state (sort) via useMarketplaceSort hook - Use TanStack Query for reactive data fetching - Add useMarketplaceMoreClick hook to avoid prop drilling - Add query-keys.ts for centralized query key management - Add marketplace-client.tsx for client-side hydration - Simplify component props by using hooks directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
412015e to
58550a5
Compare
- Remove tests that used the old MarketplaceContext (now using nuqs/atoms) - Update mocks from ../context to ../state and ../atoms - Fix ListWithCollection tests to not pass onMoreClick prop - Fix ListWrapper tests to use mockMarketplaceData instead of props - Delete useMarketplaceFilters tests (hook no longer exists) - Fix ActivePluginType useState type in tool-picker.tsx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
Pull request overview
This PR refactors the marketplace state management from a React Context-based approach to a modern architecture using Jotai atoms and TanStack Query. The refactoring improves code organization by separating client and server concerns, adds server-side data prefetching support, and provides better type safety and maintainability.
Key Changes:
- Replaced React Context (
MarketplaceContext) with Jotai atoms for state management - Introduced TanStack Query for data fetching with proper query key management
- Added server-side hydration support using Next.js App Router patterns
- Consolidated search parameter handling using
nuqslibrary
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
web/hooks/use-query-params.ts |
Removed useMarketplaceFilters hook (moved to marketplace-specific atoms) |
web/context/query-client.tsx |
Refactored to use singleton pattern for browser query client |
web/context/query-client-server.ts |
New server-side query client factory with caching |
web/app/components/plugins/marketplace/atoms.ts |
New Jotai atoms for marketplace state with URL sync support |
web/app/components/plugins/marketplace/query.ts |
TanStack Query hooks for collections and plugins data fetching |
web/app/components/plugins/marketplace/state.ts |
Unified state management combining atoms and queries |
web/app/components/plugins/marketplace/hydration-server.tsx |
Server-side data prefetching and hydration |
web/app/components/plugins/marketplace/hydration-client.tsx |
Client-side atom hydration wrapper |
web/app/components/plugins/marketplace/search-params.ts |
Centralized search parameter parsers |
web/app/components/plugins/marketplace/utils.ts |
Added helper functions for plugins fetching and formatting |
web/app/components/plugins/marketplace/constants.ts |
Moved constants from plugin-type-switch and added new ones |
web/app/components/plugins/marketplace/index.tsx |
Simplified to use new hydration components |
web/app/components/plugins/marketplace/context.tsx |
Removed entire context-based implementation |
web/app/components/plugins/marketplace/plugin-type-switch.tsx |
Updated to use atoms instead of context |
web/app/components/plugins/marketplace/sort-dropdown/index.tsx |
Updated to use atoms for sort state |
web/app/components/plugins/marketplace/search-box/search-box-wrapper.tsx |
Updated to use atoms for search state |
web/app/components/plugins/marketplace/list/* |
Updated list components to use new state management |
web/app/(commonLayout)/plugins/page.tsx |
Simplified by removing async and unused props |
| Test files | Updated to reflect new state management architecture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Important
Fixes #<issue number>.Summary
Problems with the old implementation
Duplicate state storage: Each state has both
useState+useRef, requiring manual synchronization with lots of boilerplate codeImperative queries triggered in useEffect: Manually calling
queryPlugins/queryPluginsWithDebounced/cancelQueryPluginsWithDebounced, triggered inside useEffect with complex dependencies prone to bugs, no standardized query keysComplex URL synchronization: Three layers of nested functions
applyUrlFilters→debouncedUpdateSearchParams→handleUpdateSearchParamsSSR data not in cache: Server-side data passed via props, not entering TanStack Query cache, causing duplicate requests after client hydration
No URL parameter prefetching: Server only fetches default data, prefetching is ineffective when users visit with URL parameters
Props drilling:
showSearchParamsdrilled through 4 component layers to reachPluginTypeSwitchNew approach
Jotai atomic state + nuqs URL binding + TanStack Query HydrationBoundary - declarative, single data source, no props drilling. URL parameter prefetching currently only supports category (collections data), search parameters are not prefetched yet.
Fixes #30699
Screenshots
Checklist
make lintandmake type-check(backend) andcd web && npx lint-staged(frontend) to appease the lint gods