-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: always preserve marketplace search state in URL #147
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
base: qodo_action_req_1_base_refactor_always_preserve_marketplace_search_state_in_url_pr9
Are you sure you want to change the base?
Conversation
Code Review by Qodo
1. usePluginPageContext uses any
|
| } | ||
|
|
||
| export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) { | ||
| export function usePluginPageContext(selector: any) { |
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.
1. usepluginpagecontext uses any 📘 Rule violation ✓ Correctness
• The selector parameter was changed to an explicit any, which disables TypeScript’s type checking for this API surface and can hide real type errors. • This violates the requirement to avoid explicit any types and can lead to unsafe usage across all call sites of usePluginPageContext.
Agent prompt
## Issue description
`usePluginPageContext` currently accepts `selector: any`, which disables type safety and violates the no-explicit-any requirement.
## Issue Context
This hook is part of a shared context API; leaving `any` here propagates unsafe typing to all consumers.
## Fix Focus Areas
- web/app/components/plugins/plugin-page/context.tsx[51-52]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const isSearchMode = !!searchPluginText | ||
| || filterPluginTags.length > 0 | ||
| || (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) | ||
| || (searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType))) |
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.
2. Collections view never shows 🐞 Bug ✓ Correctness
• useMarketplaceSearchMode now defaults to search mode when the active category is in PLUGIN_CATEGORY_WITH_COLLECTIONS (including the default all). • This makes useMarketplaceData always build queryParams and run the plugins search query on initial load, so the collections UI path is skipped. • Users can see an empty state or unintended search results instead of curated collections, and the page does extra network work.
Agent prompt
## Issue description
Marketplace defaults into search mode for categories that have collections (including `all`), preventing the collections view from rendering and triggering search queries with empty criteria.
## Issue Context
`PLUGIN_CATEGORY_WITH_COLLECTIONS` includes `all` and `tool`. The List UI renders collections only when `plugins` is `undefined`, but `useMarketplaceData` will always produce `queryParams` (and thus `plugins`) when search mode is true.
## Fix Focus Areas
- web/app/components/plugins/marketplace/atoms.ts[35-45]
- web/app/components/plugins/marketplace/constants.ts[25-30]
- web/app/components/plugins/marketplace/state.ts[20-36]
- web/app/components/plugins/marketplace/list/index.tsx[29-38]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Ssr/tab param mismatch 🐞 Bug ⛯ Reliability
• useActivePluginType now reads/writes query param tab, but the shared parsers and server hydration still parse category. • SSR prefetch (HydrateQueryClient) will ignore tab and default to category=all, causing wrong prefetched data and client/server mismatch (layout shift + wasted requests) on direct links like ?tab=tool. • Existing deep links or surrounding code that sets category (e.g., PluginPageContext) no longer drive the marketplace category selection.
Agent prompt
## Issue description
Marketplace category is stored in URL param `tab` on the client, but server-side parsing/hydration still reads `category`. This breaks SSR prefetch and any deep links using the old param.
## Issue Context
`createLoader(marketplaceSearchParamsParsers)` only loads keys present on the parser object (`category`, `q`, `tags`). `useActivePluginType` now writes `tab`, so SSR cannot see it.
## Fix Focus Areas
- web/app/components/plugins/marketplace/atoms.ts[19-27]
- web/app/components/plugins/marketplace/search-params.ts[5-9]
- web/app/components/plugins/marketplace/hydration-server.tsx[16-28]
- web/app/components/plugins/plugin-page/context.tsx[66-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Benchmark PR from agentic-review-benchmarks#9