Skip to content

refactor: always preserve marketplace search state in URL#9

Open
tomerqodo wants to merge 2 commits intoaugment_full_base_refactor_always_preserve_marketplace_search_state_in_url_pr9from
augment_full_head_refactor_always_preserve_marketplace_search_state_in_url_pr9
Open

refactor: always preserve marketplace search state in URL#9
tomerqodo wants to merge 2 commits intoaugment_full_base_refactor_always_preserve_marketplace_search_state_in_url_pr9from
augment_full_head_refactor_always_preserve_marketplace_search_state_in_url_pr9

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#9

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Jan 26, 2026

🤖 Augment PR Summary

Summary: Refactors the plugins marketplace UI so search/filter state is always preserved in the URL query string.

Changes:

  • Removed the optional “preserve search state” Jotai atom and related client hydration wrapper.
  • Switched marketplace search text/category/tags hooks to always use useQueryState (query-driven state).
  • Simplified the Marketplace component by dropping HydrateMarketplaceAtoms while keeping server-side query prefetch.
  • Relaxed typing for usePluginPageContext’s selector parameter.

Technical Notes: SSR prefetch still parses marketplace search params via marketplaceSearchParamsParsers and depends on consistent query param keys.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 3 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

const queryState = useQueryState('category', marketplaceSearchParamsParsers.category)
const atomState = useAtom(activePluginTypeAtom)
return preserveSearchStateInQuery ? queryState : atomState
return useQueryState('tab', marketplaceSearchParamsParsers.category)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useActivePluginType() now reads/writes tab, but server parsing/prefetch (createLoader(marketplaceSearchParamsParsers) and params.category) still depends on the category param; this can make the selected category default to all and skip intended SSR prefetching.

Other Locations
  • web/app/components/plugins/marketplace/hydration-server.tsx:16
  • web/app/components/plugins/marketplace/hydration-server.tsx:19
  • web/app/components/plugins/marketplace/search-params.ts:6

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

const isSearchMode = !!searchPluginText
|| filterPluginTags.length > 0
|| (searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)))
|| (searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition appears inverted vs the preceding comment (“force the search mode for those categories” that don’t have collections); using PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType) will force search mode for categories with collections instead.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

}

export function usePluginPageContext(selector: (value: PluginPageContextValue) => any) {
export function usePluginPageContext(selector: any) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usePluginPageContext now accepts selector: any; since useContextSelector expects a selector function, this removes the type guard that would prevent non-function values from causing runtime errors.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants