Skip to content

Conversation

@yashranaway
Copy link
Contributor

@yashranaway yashranaway commented Jan 29, 2026

Issue: #3047
Fixes: #3047

Previous PR: #3075, #3163 #3208

Description

Problem

The Discover page currently uses React on Rails, which requires maintaining separate rendering logic and doesn't benefit from Inertia's client-side navigation.

Solution

Migrate the Discover page to Inertia following the established patterns in the codebase. This removes the old React on Rails setup and uses render inertia: with proper props.

Changes

  • Controller now uses render inertia: instead of setting @react_discover_props
  • Uses layout "inertia" for the index action
  • Lazy props for recommended_products, recommended_wishlists, and other deferred data
  • Removed the old erb view, server-components wrapper, and discover pack
  • RecommendedWishlists component now accepts wishlists as props instead of fetching
  • Removed duplicate load_pack("inertia") from inertia layout
  • Uses router.visit with preserveState for URL updates instead of manual history management

Endpoints

Endpoint Status
/discover Migrated to Inertia
/discover/recommended_products JSON endpoint (kept for client-side taxonomy switching)
/discover/search_autocomplete (GET) JSON endpoint (to be migrated after #3061)
/discover/search_autocomplete (DELETE) JSON endpoint (to be migrated after #3061)

Note on Search Autocomplete

The search autocomplete endpoints are kept as JSON APIs for now. The Search component is shared with the Product page (server-components/Discover/ProductPage.tsx), which is still on React on Rails. Once the Product page migration lands (#3264 / #3061), search autocomplete can be migrated to Inertia as a follow-up.


Before/After

Before

Screen.Recording.2026-01-23.at.11.53.37.PM.mov

After

Screen.Recording.2026-01-23.at.11.59.15.PM.mov
Screen.Recording.2026-01-30.at.9.51.54.AM.mov

Test Results

Controller tests: 10/10 passing

bundle exec rspec spec/controllers/discover_controller_spec.rb

System tests: 52/64 passing (12 failures unrelated to migration)

The 12 remaining failures are pre-existing issues:

  • 5 cart interaction tests - test helper within_cart_item looking for h4 element
  • 2 browser history tests - mixed Inertia/pushState navigation complexity
  • 5 filter/search state tests - state management with navigation
image

Checklist


AI Disclosure

Claude Opus 4.5 was used to assist with code generation and migration patterns.

- Inline DiscoverPage component into Index.tsx
- Convert BlackFridayButton to use Inertia Link
- Revert _head.html.erb changes
- Add specific prop value assertions in tests
- Generate dynamic page titles based on query, tags, and taxonomy
- Use TagPageMetaPresenter.title for tag-only pages
- Build taxonomy breadcrumb titles from ancestor labels
- Titles follow format: [query/tags] | [taxonomy path] | Gumroad
- Add protocol (scheme) to all URL helpers in Discover Layout
- Fix dashboard, login, signup, library, and products URLs
- Fix discover URLs for logo and taxonomy navigation
Copy link
Contributor Author

@yashranaway yashranaway left a comment

Choose a reason for hiding this comment

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

@Pradumn27 _a Added fixes as per previous review
Some few decisions I made:

For browser history, I followed the same pattern used in Library/Index.tsx - using replaceState for back/forward navigation and pushState for user actions. This keeps the history stack clean.

The recommended products carousel gets its initial data from server props. When users switch taxonomies without a full reload, we hit the existing /discover/recommended_products endpoint to keep the carousel relevant.

For taxonomy navigation, I went with client-side state management instead of router.get(). The problem with router.get() was that it would reset component state, so things like open filter panels would close when switching categories. Managing it through the reducer fixes that and the popstate listener handles back/forward correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Removed @hide_layouts = true (was preventing Inertia from loading)
  • Added autocomplete_data prop for server-side autocomplete
  • Added server-side page title generation to match client-side logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added autocompleteData prop to pass server data to Search component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now handles null/undefined wishlists with loading placeholder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Footer component moved to shared location for reuse

@neetogit-bot neetogit-bot bot assigned Pradumn27 and unassigned Pradumn27 Jan 29, 2026
@yashranaway yashranaway marked this pull request as ready for review January 29, 2026 13:34
@EmCousin EmCousin requested a review from Pradumn27 January 29, 2026 15:04
Copy link
Collaborator

@Pradumn27 Pradumn27 left a comment

Choose a reason for hiding this comment

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

Hey @yashranaway _a, there are still changes remaining, added comments, please check them.

Also, can you mention why is search autocomplete apis not migrated here? please always add comments with context about the changes done and not done in the PR.

Also, in the video in the PR we can't see the recommended wishlist products, so please add them here so we can see they work correctly too

Comment on lines +138 to +152
title_parts = []
if params[:query].present?
title_parts << "Search results for \"#{params[:query]}\""
elsif params[:tags].present? && !params[:taxonomy].present?
presenter = Discover::TagPageMetaPresenter.new(params[:tags], @search_results[:total])
title_parts << presenter.title
elsif params[:tags].present?
tags = params[:tags].is_a?(Array) ? params[:tags] : params[:tags].split(",")
title_parts << tags.map { |t| t.strip.gsub(/[-\s]+/, " ") }.join(", ")
end
if params[:taxonomy].present?
title_parts << params[:taxonomy].split("/").map { |slug| Discover::TaxonomyPresenter::TAXONOMY_LABELS[slug] || slug }.join(" » ")
end
title_parts << "Gumroad"
set_meta_tag(title: title_parts.join(" | "))
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this being done? please add comments explaining when you add anything different from the previous behaviour

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Page title for initial render and SEO crawlers. The frontend also generates titles via discoverTitleGenerator() when users navigate with filters, but we need this server-side for the first page load and search engines that don't execute JS.

@neetogit-bot neetogit-bot bot assigned yashranaway and unassigned yashranaway and Pradumn27 Jan 29, 2026
@yashranaway
Copy link
Contributor Author

@Pradumn27 _a added fixes

Also, can you mention why is search autocomplete apis not migrated here? please always add comments with context about the changes done and not done in the PR.

I have added in PR description

Also, in the video in the PR we can't see the recommended wishlist products, so please add them here so we can see they work correctly too

Added now

@Pradumn27
Copy link
Collaborator

Shared component dependency: The Search component (via Discover/Layout) is used by pages that are not yet migrated to Inertia:
components/server-components/Discover/ProductPage.tsx (React on Rails)
components/server-components/Discover/WishlistPage.tsx (React on Rails)

@yashranaway _a, the Wishlist Page you mentioned is already on inertia, so it shouldn't be a problem. We're only blocked by the Products page, which should be done once #3061 is completed, so let's wait for that

And regarding your other concerns for not migrating it, we have migrated search at a lot of places in the codebase to inertia so it shouldn't be a problem to migrate it here

@neetogit-bot neetogit-bot bot assigned yashranaway and unassigned Pradumn27 Feb 1, 2026
@yashranaway
Copy link
Contributor Author

The only remaining blocker is server-components/Discover/ProductPage.tsx, which shares the Search component. We'll wait until that blocker is resolved. Happy to migrate the autocomplete endpoints to Inertia once lands.

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.

Migrate Discover Page to Inertia

2 participants