refactor: always preserve marketplace search state in URL#160
Conversation
Review Summary by QodoSimplify marketplace search state to always use URL parameters
WalkthroughsDescription• Simplify marketplace search state management by always using URL query parameters • Remove conditional logic for preserving search state in atoms • Eliminate hydration component and atom-based state management • Fix search mode logic to correctly check category collections Diagramflowchart LR
A["Atom-based State Management"] -->|Remove| B["URL Query Parameters Only"]
C["HydrateMarketplaceAtoms Component"] -->|Delete| B
D["Conditional preserveSearchStateInQuery Logic"] -->|Simplify| B
E["Search Mode Logic"] -->|Fix negation| B
File Changes1. web/app/components/plugins/marketplace/atoms.ts
|
Code Review by Qodo
1. usePluginPageContext uses any
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| } | ||
|
|
||
| export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) { | ||
| export function usePluginPageContext(selector: any) { |
There was a problem hiding this comment.
1. usepluginpagecontext uses any 📘 Rule violation ✓ Correctness
usePluginPageContext now types selector as any, which defeats TypeScript type safety and can hide incorrect selector usage. This violates the requirement to avoid explicit any in TypeScript production code.
Agent Prompt
## Issue description
`usePluginPageContext` accepts `selector: any`, which bypasses type checking and violates the no-explicit-any rule.
## Issue Context
This hook wraps `useContextSelector` and can be typed generically so callers get proper inference and safety.
## Fix Focus Areas
- web/app/components/plugins/plugin-page/context.tsx[51-53]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
40
to
44
| const searchMode = useAtomValue(searchModeAtom) | ||
| const isSearchMode = !!searchPluginText | ||
| || filterPluginTags.length > 0 | ||
| || (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) | ||
| || (searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) | ||
| return isSearchMode |
There was a problem hiding this comment.
2. Search mode default inverted 🐞 Bug ✓ Correctness
useMarketplaceSearchMode now defaults to search mode when the active category has collections, which forces plugin search queries (often with empty query) and prevents showing collections for categories that do have collections. This contradicts the comment and the plugin-type-switch behavior, causing the marketplace to render the wrong view by default.
Agent Prompt
### Issue description
`useMarketplaceSearchMode` uses the wrong default boolean for categories with/without collections, causing the marketplace to enter search mode by default for `all/tool` (which have collections) and not enter search mode for categories that lack collections.
### Issue Context
The comment and related code (collections prefetching + plugin type switch behavior) indicate that categories *without* collections should force search mode.
### Fix Focus Areas
- web/app/components/plugins/marketplace/atoms.ts[35-45]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
22
to
24
| export function useActivePluginType() { | ||
| const preserveSearchStateInQuery = useAtomValue(preserveSearchStateInQueryAtom) | ||
| const queryState = useQueryState('category', marketplaceSearchParamsParsers.category) | ||
| const atomState = useAtom(activePluginTypeAtom) | ||
| return preserveSearchStateInQuery ? queryState : atomState | ||
| return useQueryState('tab', marketplaceSearchParamsParsers.category) | ||
| } |
There was a problem hiding this comment.
3. Tab param collides globally 🐞 Bug ✓ Correctness
Marketplace category state is now stored in the generic URL parameter tab, which is also used by the globally-mounted account settings modal query state. This causes URL-state interference: opening/switching account settings tabs can overwrite marketplace category, and switching marketplace categories can overwrite the account settings tab value.
Agent Prompt
### Issue description
Marketplace currently stores its active category in `?tab=...`, which conflicts with the global account settings modal’s `tab` query parameter.
### Issue Context
`ModalContextProvider` is mounted in the common layout and uses `useAccountSettingModal`, which reads/writes `tab`.
### Fix Focus Areas
- web/app/components/plugins/marketplace/atoms.ts[22-27]
- web/app/components/plugins/marketplace/search-params.ts[5-9]
- web/app/components/plugins/marketplace/hydration-server.tsx[12-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
22
to
24
| export function useActivePluginType() { | ||
| const preserveSearchStateInQuery = useAtomValue(preserveSearchStateInQueryAtom) | ||
| const queryState = useQueryState('category', marketplaceSearchParamsParsers.category) | ||
| const atomState = useAtom(activePluginTypeAtom) | ||
| return preserveSearchStateInQuery ? queryState : atomState | ||
| return useQueryState('tab', marketplaceSearchParamsParsers.category) | ||
| } |
There was a problem hiding this comment.
4. Server/client param mismatch 🐞 Bug ⛯ Reliability
Server-side hydration parses the marketplace category from the category query param, but the client now reads/writes the category using tab. As a result, server prefetch/hydration will ignore the client’s actual category and can prefetch the wrong data (or none), degrading correctness and performance when searchParams is supplied.
Agent Prompt
### Issue description
Server-side parsing/hydration reads `category`, but client-side marketplace state now uses `tab`, so SSR prefetching and client state are desynchronized.
### Issue Context
`HydrateQueryClient` still uses `createLoader(marketplaceSearchParamsParsers)` and checks `params.category`.
### Fix Focus Areas
- web/app/components/plugins/marketplace/atoms.ts[22-24]
- web/app/components/plugins/marketplace/search-params.ts[5-9]
- web/app/components/plugins/marketplace/hydration-server.tsx[12-29]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Benchmark PR from agentic-review-benchmarks#9