Add three-version search page with loadable SSR and RSC streaming#13
Add three-version search page with loadable SSR and RSC streaming#13AbanoubGhadban merged 6 commits intomainfrom
Conversation
- Add image_url column to restaurants table with migration - Backfill existing records with picsum.photos placeholder URLs - Update seeder to include image_url for new seeds - Fix RestaurantCardFooter: add missing RatingBadge import, add getDistance - Fix TrendingItems: add missing MenuItem type import - Add Haversine distance utility (utils/distance.ts) Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Spinner: animated circle with optional label, used as Suspense fallback - CardWidgetsSkeleton: pulse-animated placeholder matching card widget dimensions to minimize CLS when replaced with real content Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
- search_ssr: fetches all data server-side sequentially, returns complete page - search_client: sends basic restaurant info, client fetches async data - search_rsc: sends basic info, uses RSC streaming for async data - Routes: /search/ssr, /search/client, /search/rsc - Default search: Italian restaurants in New York Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ttern - Task 3: Now covers V1 (Full Server SSR) and V2 (Client Components) - Task 4: Fix getReactOnRailsAsyncProp as prop (not import), use stream_react_component_with_async_props with emit block, remove incorrect AsyncProps Ruby module - Multi-restaurant search grid (4 cards) instead of single restaurant - Correct CLS expectations for RSC streaming (~0.02-0.08, not 0.00) - Add three-version performance comparison table and demo flow Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ebpack configs - Add @loadable/component with @swc/plugin-loadable-components for SSR-compatible code splitting in V2 (SearchPageClient) using .client.tsx/.server.tsx convention - Add LoadablePlugin to client webpack config for loadable-stats.json generation - Fix RSC WebpackLoader by adding enforce:'post' rule (swc-loader uses function format that extractLoader() can't find) - Fix ReactOnRails global mismatch by using react-on-rails-pro/client in client-bundle.js to match generated pack imports - Fix V3 RSC async props by using stream_react_component_with_async_props with emit block for restaurant widget data - Add assets_to_copy config for loadable-stats.json in node renderer - Add yield :head to layout for loadable link/script/style tag injection - Use pnpm.overrides to resolve react-on-rails yalc version for pro dependency - Add .swc/, .yalc/, yalc.lock to .gitignore Co-Authored-By: Claude Opus 4.6 <[email protected]>
WalkthroughThis PR implements a three-version architecture for a restaurant search feature: V1 (full server-side SSR), V2 (server structure with client-side async widgets), and V3 (React Server Components with streaming). It includes new React components, Rails controller actions, views, Loadable integration, dependency updates, database migrations, and utility functions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser/Client
participant Rails as Rails Server
participant React as React Components
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over Client,DB: V1 SSR - Full Server Rendering
Client->>Rails: GET /search/ssr
Rails->>DB: fetch_restaurants (by_cuisine, city, limit)
DB-->>Rails: Restaurant array
Rails->>Rails: serialize_promotion & serialize_menu_item
Rails->>Rails: build `@restaurant_data` with status, wait_time, specials, trending
Rails->>React: render SearchPageSSR with `@restaurant_data`
React-->>Client: Complete HTML with all data
Client->>Client: Display fully rendered page
end
rect rgba(150, 200, 100, 0.5)
Note over Client,DB: V2 Client - Hybrid Rendering + Async Fetch
Client->>Rails: GET /search/client
Rails->>DB: fetch_restaurants
DB-->>Rails: Restaurant array
Rails->>React: render SearchPageClient (hydrate)
React-->>Client: HTML with headers only
Client->>Client: Hydrate SearchPageClient
Client->>Rails: fetch AsyncRestaurantWidgets data per restaurant
Rails-->>Client: status, wait_time, specials, trending (JSON)
Client->>React: Update state with async widget data
React-->>Client: Render complete page with widgets
end
rect rgba(200, 150, 100, 0.5)
Note over Client,DB: V3 RSC - Streaming with Async Props
Client->>Rails: GET /search/rsc
Rails->>DB: fetch_restaurants
DB-->>Rails: Restaurant array
Rails->>Rails: initiate stream_react_component_with_async_props
Rails-->>Client: Send initial SearchPageRSC shell + Suspense boundaries
Rails->>Rails: emit restaurant_{id}_widgets for each restaurant
Rails-->>Client: Stream widget data as promises resolve
Client->>React: Render AsyncRestaurantWidgetsRSC with streamed data
React-->>Client: Progressive rendering with skeleton fallbacks
Client->>Client: Display complete page as stream completes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@app/javascript/components/restaurant/AsyncRestaurantWidgets.tsx`:
- Around line 24-39: The fetches in AsyncRestaurantWidgets.tsx (the calls using
restaurantId, opts and the state setters setStatus, setWaitTime, setSpecials,
setTrending) parse JSON without checking response.ok, which lets 4xx/5xx bodies
or thrown JSON errors become widget state; update each fetch to first check r.ok
and throw or handle non-ok responses before calling r.json(), then only call the
corresponding setter with the parsed data on success, and ensure the catch
handlers are not empty (at minimum log the error or set an error state) so
failures don’t silently inject error payloads into
StatusBadge/WaitTime/Specials/Trending components.
In `@app/javascript/utils/distance.ts`:
- Around line 1-3: Update the hardcoded default user coordinates in distance.ts
so they match the app's default search location: change the constants USER_LAT
and USER_LNG from Portland values to New York City coordinates (e.g., USER_LAT
-> 40.7128 and USER_LNG -> -74.0060) to ensure distance calculations align with
restaurants_controller.rb's default NYC search.
In `@app/views/restaurants/search_rsc.html.erb`:
- Line 9: The template uses p.ends_at.iso8601 which will raise NoMethodError
when ends_at is nil; update the ERB to safely call iso8601 only when present by
changing the reference to use the safe navigation operator (p.ends_at&.iso8601)
so promotions without an ends_at don’t crash the view rendering.
- Around line 1-17: The view iterates `@restaurants` and calls methods
(restaurant.current_status, current_wait_time, active_promotions,
trending_items) that trigger N+1 DB queries; fix by eager-loading the needed
associations in the controller before rendering SearchPageRSC so those methods
don't hit the DB per restaurant—update the fetch that sets `@restaurants` to
include/preload associations used by those methods (e.g. special_hours, hours,
orders, promotions, menu_items and any join targets like order_lines or orders
for trending_items) or load computed data into a presenter/hash passed into
stream_react_component_with_async_props so
emit.call("restaurant_#{r.id}_widgets", ...) uses preloaded data instead of
firing queries.
In `@Gemfile`:
- Around line 19-21: Remove the hardcoded absolute local paths for the gems
'react_on_rails' and 'react_on_rails_pro' and replace them with a fallback that
uses either a local override or the published gems: update the Gemfile to check
for a local override (e.g., eval_gemfile 'Gemfile.local' if it exists or use
ENV['REACT_ON_RAILS_LOCAL_PATH'] to point to a local path) and otherwise declare
gem 'react_on_rails' and gem 'react_on_rails_pro' without a path so Bundler
pulls the published versions; ensure the changes reference the exact gem names
'react_on_rails' and 'react_on_rails_pro' so CI and other developers don’t rely
on machine-specific paths.
In `@IMPLEMENTATION_TASKS.md`:
- Around line 153-172: The doc snippet is inconsistent with the real
SearchPageClient implementation: replace the React.lazy + <Suspense> example
with the `@loadable/component` pattern used in SearchPageClient by importing
AsyncRestaurantWidgets via loadable (matching the real AsyncRestaurantWidgets
identifier) and using its built-in { fallback } option (e.g.,
CardWidgetsSkeleton) instead of wrapping with Suspense; update the snippet so
the SearchPageClient component shows loadable(() =>
import('../restaurant/AsyncRestaurantWidgets'), { fallback: <CardWidgetsSkeleton
/> }) and remove the Suspense wrapper while keeping RestaurantCardHeader,
RestaurantCardFooter, restaurants.map and the same restaurant.id key usage.
In `@package.json`:
- Around line 17-19: The yalc:publish script contains a hardcoded absolute path
(/mnt/ssd/react_on_rails_v16.3) which will break on other machines; update the
yalc:publish script to derive the repo root dynamically (e.g., use an
environment variable like REPO_ROOT, or a git-based command such as git
rev-parse --show-toplevel, or make the script run from the package root via
relative paths) and ensure yalc:link and yalc:setup continue to work with that
change; modify the npm script names yalc:publish, yalc:link, and yalc:setup so
they no longer rely on machine-specific absolute paths and document or default
the required env var if you choose that approach.
In `@tasks/Task_4_RSC_Version.md`:
- Around line 220-223: The doc incorrectly references fetch_restaurants_basic
while the controller defines fetch_restaurants; update the documentation in
Task_4_RSC_Version.md (the search_rsc example) to call `@restaurants` =
fetch_restaurants to match the RestaurantsController method name, or
alternatively rename the controller method to fetch_restaurants_basic if you
intend that name—ensure the method name in the search_rsc snippet and the
RestaurantsController (fetch_restaurants) are identical so readers aren't
confused.
- Around line 56-112: The doc snippet defines a local Restaurant interface
instead of importing the shared type used in the real component; update the
example by removing the local "interface Restaurant { ... }" and add the import
"import { Restaurant } from '../../types';" at the top, and then update the
Props type to reference that imported Restaurant type in the SearchPageRSC
component so the example matches the actual implementation (refer to
SearchPageRSC and Props/Restaurant symbols).
🧹 Nitpick comments (13)
tasks/Task_3_Traditional_Version.md (3)
333-342: Add language specifiers to fenced code blocks.The timeline diagrams at lines 333 and 348 lack language specifiers, which triggers markdownlint MD040 warnings. Use
```textfor these blocks.Also applies to: 348-357
151-157: Doc referencesReact.lazybut implementation uses@loadable/component.The V2 code sample in this task doc shows
lazy(() => import(...))with<Suspense>, but the actual implementation in the PR uses@loadable/componentfor SSR-compatible code splitting. Consider updating the doc to match the implementation to avoid confusion for future contributors.
230-234: Silent error swallowing in fetch calls.The example code uses
.catch(() => {})on all five fetch calls, silently discarding errors. In the actual component implementation, this means network failures or server errors will leave widgets permanently in their initialnullstate with no user feedback. Consider at minimum logging errors or showing an error state.config/webpack/clientWebpackConfig.js (1)
5-5:isHMRis truthy for any non-empty string, including"false".
process.env.HMRreturns a string, soHMR=falsewould still be truthy. If this env var is only ever set/unset (not"false"), this is fine. Otherwise, consider an explicit check likeprocess.env.HMR === 'true'.app/javascript/components/restaurant/AsyncRestaurantWidgets.tsx (1)
27-39: Silent error swallowing hides API failures from the user.All four
.catch(() => {})blocks discard errors with no feedback. Consider at minimum logging toconsole.error, or adding an error state so the UI can show a fallback message instead of an empty section.app/javascript/components/search/SearchPageRSC.tsx (1)
15-15:asynckeyword onSearchPageRSCis unnecessary — noawaitin the function body.The component doesn't perform any async operations itself; the async work is in
AsyncRestaurantWidgetsRSC. Removingasyncavoids wrapping the return value in a needless Promise.Proposed fix
-export default async function SearchPageRSC({ restaurants, getReactOnRailsAsyncProp }: Props) { +export default function SearchPageRSC({ restaurants, getReactOnRailsAsyncProp }: Props) {app/javascript/components/restaurant/AsyncRestaurantWidgetsRSC.tsx (1)
11-11: Consider adding a typed return shape instead ofPromise<any>.This would provide compile-time safety for
data.status,data.wait_time, etc. A shared interface (e.g.,RestaurantWidgetsData) could be reused by both the RSC and client widget components.app/javascript/startup/SearchPageClient.server.tsx (1)
1-1:'use client'on a server-side rendering entry point is semantically confusing but likely necessary.This file runs on the server (Node.js) via
renderToString, yet carries a'use client'directive. In an RSC-aware bundler context this is needed to mark the component tree boundary. A brief comment explaining why would help future readers.Suggested clarifying comment
-'use client'; +'use client'; +// This directive tells the RSC bundler to treat SearchPageClient's tree as client components. +// The file itself runs on the server (Node.js) for SSR via renderToString.app/javascript/components/search/SearchPageClient.tsx (1)
19-37: Minor: consider adding an empty-state guard forrestaurants.If
restaurantsis an empty array or unexpectedlyundefined/null(e.g., serialization issue from Rails), the component silently renders an empty grid with no feedback to the user.app/controllers/restaurants_controller.rb (2)
47-52: Default filter values are hardcoded — consider documenting this is intentional for the demo.
fetch_restaurantsdefaults to'Italian'cuisine in'New York'with a limit of 4. This is fine for a demo, but if the seeded data doesn't include exactly these values, the page will render empty. A comment or a fallback would help future developers.
10-31: Consider adding explicit serialization tosearch_clientfor consistency.The
search_clientview passes the raw@restaurantsrelation withprerender: true, relying on Rails' default.as_jsonbehavior. While this should produce the expected shape (matching the TypeScript interface with database columns likeimage_url,latitude,longitude, etc.), making the serialization explicit in the controller—as done insearch_ssr—would clarify the intent and prevent unintended field inclusion if the model changes. No N+1 queries should occur from the default serialization since it only includes database columns, not association-loading methods likecurrent_statusorcurrent_wait_time(which the component fetches client-side anyway).IMPLEMENTATION_TASKS.md (1)
368-374: Add a language identifier to the fenced code block.This code block lacks a language specifier, which was flagged by markdownlint (MD040). Use a
textorplaintextidentifier since it's a comparison table.Proposed fix
-``` +```text Metric V1 (Full SSR) V2 (Client) V3 (RSC)tasks/Task_4_RSC_Version.md (1)
243-249: Add language identifiers to unlabeled fenced code blocks.Three fenced code blocks (lines 243, 253, 309) lack language specifiers per markdownlint MD040. Use
textorplaintextfor these ASCII diagrams/timelines.Also applies to: 253-266, 309-320
| fetch(`/api/restaurants/${restaurantId}/status`, opts) | ||
| .then((r) => r.json()) | ||
| .then(setStatus) | ||
| .catch(() => {}); | ||
| fetch(`/api/restaurants/${restaurantId}/wait_time`, opts) | ||
| .then((r) => r.json()) | ||
| .then(setWaitTime) | ||
| .catch(() => {}); | ||
| fetch(`/api/restaurants/${restaurantId}/specials`, opts) | ||
| .then((r) => r.json()) | ||
| .then(setSpecials) | ||
| .catch(() => {}); | ||
| fetch(`/api/restaurants/${restaurantId}/trending`, opts) | ||
| .then((r) => r.json()) | ||
| .then(setTrending) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
No response.ok check before parsing JSON — failed responses will silently set error payloads as widget state.
If the server returns a 4xx/5xx, r.json() will either throw (swallowed by the empty catch) or parse an error body object into state, causing child components to render with unexpected data shapes (e.g., undefined fields on StatusBadge).
Proposed fix
- fetch(`/api/restaurants/${restaurantId}/status`, opts)
- .then((r) => r.json())
- .then(setStatus)
- .catch(() => {});
+ fetch(`/api/restaurants/${restaurantId}/status`, opts)
+ .then((r) => { if (!r.ok) throw new Error(r.statusText); return r.json(); })
+ .then(setStatus)
+ .catch(() => {});Apply the same pattern to all four fetches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fetch(`/api/restaurants/${restaurantId}/status`, opts) | |
| .then((r) => r.json()) | |
| .then(setStatus) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/wait_time`, opts) | |
| .then((r) => r.json()) | |
| .then(setWaitTime) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/specials`, opts) | |
| .then((r) => r.json()) | |
| .then(setSpecials) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/trending`, opts) | |
| .then((r) => r.json()) | |
| .then(setTrending) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/status`, opts) | |
| .then((r) => { if (!r.ok) throw new Error(r.statusText); return r.json(); }) | |
| .then(setStatus) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/wait_time`, opts) | |
| .then((r) => { if (!r.ok) throw new Error(r.statusText); return r.json(); }) | |
| .then(setWaitTime) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/specials`, opts) | |
| .then((r) => { if (!r.ok) throw new Error(r.statusText); return r.json(); }) | |
| .then(setSpecials) | |
| .catch(() => {}); | |
| fetch(`/api/restaurants/${restaurantId}/trending`, opts) | |
| .then((r) => { if (!r.ok) throw new Error(r.statusText); return r.json(); }) | |
| .then(setTrending) | |
| .catch(() => {}); |
🤖 Prompt for AI Agents
In `@app/javascript/components/restaurant/AsyncRestaurantWidgets.tsx` around lines
24 - 39, The fetches in AsyncRestaurantWidgets.tsx (the calls using
restaurantId, opts and the state setters setStatus, setWaitTime, setSpecials,
setTrending) parse JSON without checking response.ok, which lets 4xx/5xx bodies
or thrown JSON errors become widget state; update each fetch to first check r.ok
and throw or handle non-ok responses before calling r.json(), then only call the
corresponding setter with the parsed data on success, and ensure the catch
handlers are not empty (at minimum log the error or set an error state) so
failures don’t silently inject error payloads into
StatusBadge/WaitTime/Specials/Trending components.
| // Default user location: Portland, OR city center | ||
| const USER_LAT = 45.5155; | ||
| const USER_LNG = -122.6789; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what city the controller defaults to for restaurant search
rg -n 'default\|city\|New York\|Portland' --type ruby -g '*restaurants_controller*'Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
🏁 Script executed:
#!/bin/bash
# Also search for any restaurant location constants in the codebase
rg -n 'New York\|restaurant.*lat\|restaurant.*lng\|RESTAURANT' -i --type ruby --type typescript --type javascriptRepository: shakacode/react-server-components-marketplace-demo
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check for any files that might define restaurant locations or defaults
fd -e ts -e js -e rb | xargs grep -l 'restaurant' -i | head -20Repository: shakacode/react-server-components-marketplace-demo
Length of output: 805
🏁 Script executed:
#!/bin/bash
# Read the restaurants controller to see default location
cat -n app/controllers/restaurants_controller.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 2625
🏁 Script executed:
#!/bin/bash
# Also check the API restaurants controller
cat -n app/controllers/api/restaurants_controller.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 2020
🏁 Script executed:
#!/bin/bash
# Search for New York or Portland in restaurant-related files
rg -n 'New York|Portland|latitude|longitude' -i app/models/restaurant.rb app/controllers/restaurants_controller.rb app/controllers/api/restaurants_controller.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 334
🏁 Script executed:
#!/bin/bash
# Check if there's a seeds file with restaurant data
find . -name 'seeds.rb' -o -name '*seed*' | head -10Repository: shakacode/react-server-components-marketplace-demo
Length of output: 225
🏁 Script executed:
#!/bin/bash
# Read the distance.ts file to confirm the coordinates
cat -n app/javascript/utils/distance.tsRepository: shakacode/react-server-components-marketplace-demo
Length of output: 1141
🏁 Script executed:
#!/bin/bash
# Check seed data to see where restaurants are located
head -50 db/seeds.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 472
🏁 Script executed:
#!/bin/bash
# Check the small seeder to see restaurant locations
grep -n 'New York\|Portland\|latitude\|longitude\|city:' db/seed_scripts/small_seeder.rb | head -20Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
🏁 Script executed:
#!/bin/bash
# Read the small seeder file
cat app/javascript/utils/distance.tsdb/seed_scripts/small_seeder.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 189
🏁 Script executed:
#!/bin/bash
# Try reading the seeder files with a simpler approach
wc -l db/seed_scripts/*.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 217
🏁 Script executed:
#!/bin/bash
# Get details on restaurant creation in seeders
rg -n 'city.*New York|city.*Portland|Restaurant\.create' db/seed_scripts/Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
🏁 Script executed:
#!/bin/bash
# Read the small seeder file
cat -n db/seed_scripts/small_seeder.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 1020
🏁 Script executed:
#!/bin/bash
# Read base seeder to understand restaurant structure
head -100 db/seed_scripts/base_seeder.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 3538
🏁 Script executed:
#!/bin/bash
# Search for restaurant creation with city information
grep -A5 -B5 'city:' db/seed_scripts/*.rb | head -50Repository: shakacode/react-server-components-marketplace-demo
Length of output: 988
Hardcoded user location (Portland, OR) doesn't align with default search location (New York).
The controller defaults to searching for restaurants in New York (line 50 of restaurants_controller.rb), but the user location is hardcoded to Portland, OR. This mismatch causes unnecessarily large distances—around 2,400 miles for New York restaurants, and 630–2,000+ miles for other major cities in the seed data. Align the default user location with New York to provide more realistic distance calculations.
Proposed fix
-// Default user location: Portland, OR city center
-const USER_LAT = 45.5155;
-const USER_LNG = -122.6789;
+// Default user location: New York, NY city center
+const USER_LAT = 40.7128;
+const USER_LNG = -74.0060;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Default user location: Portland, OR city center | |
| const USER_LAT = 45.5155; | |
| const USER_LNG = -122.6789; | |
| // Default user location: New York, NY city center | |
| const USER_LAT = 40.7128; | |
| const USER_LNG = -74.0060; |
🤖 Prompt for AI Agents
In `@app/javascript/utils/distance.ts` around lines 1 - 3, Update the hardcoded
default user coordinates in distance.ts so they match the app's default search
location: change the constants USER_LAT and USER_LNG from Portland values to New
York City coordinates (e.g., USER_LAT -> 40.7128 and USER_LNG -> -74.0060) to
ensure distance calculations align with restaurants_controller.rb's default NYC
search.
| <%= stream_react_component_with_async_props("SearchPageRSC", props: { restaurants: @restaurants }, prerender: true) do |emit| | ||
| @restaurants.each do |r| | ||
| emit.call("restaurant_#{r.id}_widgets", { | ||
| status: r.current_status, | ||
| wait_time: r.current_wait_time, | ||
| specials: r.active_promotions.map { |p| | ||
| { id: p.id, title: p.title, description: p.description, | ||
| discount_type: p.discount_type, discount_value: p.discount_value.to_f, | ||
| code: p.code, ends_at: p.ends_at.iso8601 } | ||
| }, | ||
| trending: r.trending_items.map { |item| | ||
| { id: item.id, name: item.name, category: item.category, | ||
| price: item.price.to_f, description: item.description } | ||
| } | ||
| }) | ||
| end | ||
| end %> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check controller action for eager loading
rg -n "search_rsc" -A 20 -g '*.rb' -g '!node_modules/**'Repository: shakacode/react-server-components-marketplace-demo
Length of output: 4272
🏁 Script executed:
#!/bin/bash
# Find Restaurant model
find . -name "restaurant.rb" -path "*/models/*" | head -5Repository: shakacode/react-server-components-marketplace-demo
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Check Restaurant model for associations and methods
cat -n ./app/models/restaurant.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 4156
N+1 query risk: @restaurants.each calls multiple database queries per restaurant.
Each iteration triggers separate DB queries for:
current_status(queriesspecial_hoursandhoursassociations)current_wait_time(queriesordersmultiple times)active_promotions(queriespromotionsassociation)trending_items(complex query joiningmenu_items,order_lines, andorders)
With 4 restaurants, this could result in 24+ queries instead of 1-5. Eager-load these associations in the controller:
`@restaurants` = fetch_restaurants.includes(:special_hours, :hours, :orders, :promotions, :menu_items)🤖 Prompt for AI Agents
In `@app/views/restaurants/search_rsc.html.erb` around lines 1 - 17, The view
iterates `@restaurants` and calls methods (restaurant.current_status,
current_wait_time, active_promotions, trending_items) that trigger N+1 DB
queries; fix by eager-loading the needed associations in the controller before
rendering SearchPageRSC so those methods don't hit the DB per restaurant—update
the fetch that sets `@restaurants` to include/preload associations used by those
methods (e.g. special_hours, hours, orders, promotions, menu_items and any join
targets like order_lines or orders for trending_items) or load computed data
into a presenter/hash passed into stream_react_component_with_async_props so
emit.call("restaurant_#{r.id}_widgets", ...) uses preloaded data instead of
firing queries.
| specials: r.active_promotions.map { |p| | ||
| { id: p.id, title: p.title, description: p.description, | ||
| discount_type: p.discount_type, discount_value: p.discount_value.to_f, | ||
| code: p.code, ends_at: p.ends_at.iso8601 } |
There was a problem hiding this comment.
p.ends_at.iso8601 will raise NoMethodError if ends_at is nil.
If any promotion doesn't have an ends_at date, this will crash mid-stream. Guard with p.ends_at&.iso8601.
Proposed fix
- code: p.code, ends_at: p.ends_at.iso8601 }
+ code: p.code, ends_at: p.ends_at&.iso8601 }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| code: p.code, ends_at: p.ends_at.iso8601 } | |
| code: p.code, ends_at: p.ends_at&.iso8601 } |
🤖 Prompt for AI Agents
In `@app/views/restaurants/search_rsc.html.erb` at line 9, The template uses
p.ends_at.iso8601 which will raise NoMethodError when ends_at is nil; update the
ERB to safely call iso8601 only when present by changing the reference to use
the safe navigation operator (p.ends_at&.iso8601) so promotions without an
ends_at don’t crash the view rendering.
| # React on Rails - local development from v16.3 branch | ||
| gem 'react_on_rails', path: '/mnt/ssd/react_on_rails_v16.3/react_on_rails' | ||
| gem 'react_on_rails_pro', path: '/mnt/ssd/react_on_rails_v16.3/react_on_rails_pro' |
There was a problem hiding this comment.
Hardcoded absolute local paths will break for other developers and CI.
These paths (/mnt/ssd/react_on_rails_v16.3/...) are machine-specific and will cause bundle install failures for anyone without this exact directory structure. This should not be merged to main.
Consider using environment-based overrides (e.g., a Gemfile.local pattern via eval_gemfile) or reverting to published gem versions for the shared branch.
🤖 Prompt for AI Agents
In `@Gemfile` around lines 19 - 21, Remove the hardcoded absolute local paths for
the gems 'react_on_rails' and 'react_on_rails_pro' and replace them with a
fallback that uses either a local override or the published gems: update the
Gemfile to check for a local override (e.g., eval_gemfile 'Gemfile.local' if it
exists or use ENV['REACT_ON_RAILS_LOCAL_PATH'] to point to a local path) and
otherwise declare gem 'react_on_rails' and gem 'react_on_rails_pro' without a
path so Bundler pulls the published versions; ensure the changes reference the
exact gem names 'react_on_rails' and 'react_on_rails_pro' so CI and other
developers don’t rely on machine-specific paths.
| ```typescript | ||
| // SearchPage.tsx - SSRed by react_component | ||
| export default function SearchPage({ restaurant_id }: Props) { | ||
| return <SearchPageContent restaurantId={restaurant_id} />; | ||
| } | ||
| 'use client'; // Makes ALL components into client components | ||
|
|
||
| // SearchPageContent.tsx - Forces client-side via "use client" | ||
| "use client"; | ||
| export function SearchPageContent({ restaurantId }: Props) { | ||
| const [status, setStatus] = useState(null); | ||
| useEffect(() => { | ||
| fetch(`/api/restaurants/${restaurantId}/status`) | ||
| .then(r => r.json()) | ||
| .then(setStatus); | ||
| }, [restaurantId]); | ||
| return status ? <StatusBadge status={status} /> : null; | ||
| } | ||
| ``` | ||
| const AsyncRestaurantWidgets = lazy(() => import('../restaurant/AsyncRestaurantWidgets')); | ||
|
|
||
| **View Template**: | ||
| ```erb | ||
| <!-- app/views/restaurants/search.html.erb --> | ||
| <RestaurantCardHeader restaurant={<%= restaurant.to_json %>} /> | ||
| <Suspense fallback={<Spinner />}> | ||
| <AsyncStatus restaurantId={<%= restaurant.id %>} /> | ||
| </Suspense> | ||
| <!-- etc --> | ||
| export default function SearchPageClient({ restaurants }: Props) { | ||
| return ( | ||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | ||
| {restaurants.map((restaurant) => ( | ||
| <div key={restaurant.id}> | ||
| <RestaurantCardHeader restaurant={restaurant} /> | ||
| <Suspense fallback={<CardWidgetsSkeleton />}> | ||
| <AsyncRestaurantWidgets restaurantId={restaurant.id} /> | ||
| </Suspense> | ||
| <RestaurantCardFooter restaurant={restaurant} /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Doc shows lazy() + <Suspense> but actual implementation uses @loadable/component.
The V2 code example here uses React.lazy() with <Suspense>, but the actual SearchPageClient.tsx uses loadable() from @loadable/component with its built-in { fallback } option. Update the doc snippet to match the implementation to avoid confusing future contributors.
🤖 Prompt for AI Agents
In `@IMPLEMENTATION_TASKS.md` around lines 153 - 172, The doc snippet is
inconsistent with the real SearchPageClient implementation: replace the
React.lazy + <Suspense> example with the `@loadable/component` pattern used in
SearchPageClient by importing AsyncRestaurantWidgets via loadable (matching the
real AsyncRestaurantWidgets identifier) and using its built-in { fallback }
option (e.g., CardWidgetsSkeleton) instead of wrapping with Suspense; update the
snippet so the SearchPageClient component shows loadable(() =>
import('../restaurant/AsyncRestaurantWidgets'), { fallback: <CardWidgetsSkeleton
/> }) and remove the Suspense wrapper while keeping RestaurantCardHeader,
RestaurantCardFooter, restaurants.map and the same restaurant.id key usage.
| "yalc:publish": "cd /mnt/ssd/react_on_rails_v16.3 && pnpm build && cd packages/react-on-rails && yalc publish --force && cd ../react-on-rails-pro && yalc publish --force && cd ../react-on-rails-pro-node-renderer && yalc publish --force", | ||
| "yalc:link": "yalc add react-on-rails-pro react-on-rails-pro-node-renderer", | ||
| "yalc:setup": "pnpm yalc:publish && pnpm yalc:link && pnpm install" |
There was a problem hiding this comment.
Hardcoded absolute path in yalc:publish will break for other developers.
The path /mnt/ssd/react_on_rails_v16.3 is machine-specific. This script will fail for any other contributor. Consider using an environment variable or documenting the expected setup.
Suggested approach
- "yalc:publish": "cd /mnt/ssd/react_on_rails_v16.3 && pnpm build && cd packages/react-on-rails && yalc publish --force && cd ../react-on-rails-pro && yalc publish --force && cd ../react-on-rails-pro-node-renderer && yalc publish --force",
+ "yalc:publish": "cd ${ROR_SOURCE_DIR:?Set ROR_SOURCE_DIR to your react_on_rails checkout} && pnpm build && cd packages/react-on-rails && yalc publish --force && cd ../react-on-rails-pro && yalc publish --force && cd ../react-on-rails-pro-node-renderer && yalc publish --force",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "yalc:publish": "cd /mnt/ssd/react_on_rails_v16.3 && pnpm build && cd packages/react-on-rails && yalc publish --force && cd ../react-on-rails-pro && yalc publish --force && cd ../react-on-rails-pro-node-renderer && yalc publish --force", | |
| "yalc:link": "yalc add react-on-rails-pro react-on-rails-pro-node-renderer", | |
| "yalc:setup": "pnpm yalc:publish && pnpm yalc:link && pnpm install" | |
| "yalc:publish": "cd ${ROR_SOURCE_DIR:?Set ROR_SOURCE_DIR to your react_on_rails checkout} && pnpm build && cd packages/react-on-rails && yalc publish --force && cd ../react-on-rails-pro && yalc publish --force && cd ../react-on-rails-pro-node-renderer && yalc publish --force", | |
| "yalc:link": "yalc add react-on-rails-pro react-on-rails-pro-node-renderer", | |
| "yalc:setup": "pnpm yalc:publish && pnpm yalc:link && pnpm install" |
🤖 Prompt for AI Agents
In `@package.json` around lines 17 - 19, The yalc:publish script contains a
hardcoded absolute path (/mnt/ssd/react_on_rails_v16.3) which will break on
other machines; update the yalc:publish script to derive the repo root
dynamically (e.g., use an environment variable like REPO_ROOT, or a git-based
command such as git rev-parse --show-toplevel, or make the script run from the
package root via relative paths) and ensure yalc:link and yalc:setup continue to
work with that change; modify the npm script names yalc:publish, yalc:link, and
yalc:setup so they no longer rely on machine-specific absolute paths and
document or default the required env var if you choose that approach.
|
|
||
| ```typescript | ||
| // No "use client" directive! | ||
| // This means: put in RSC bundle (server components), not server bundle (traditional SSR) | ||
| // This means: put in RSC bundle (server components), not client bundle | ||
|
|
||
| import React, { Suspense } from 'react'; | ||
| import AsyncStatus from '../async/rsc/AsyncStatus'; | ||
| import AsyncWaitTime from '../async/rsc/AsyncWaitTime'; | ||
| import AsyncSpecials from '../async/rsc/AsyncSpecials'; | ||
| import AsyncTrending from '../async/rsc/AsyncTrending'; | ||
| import AsyncRating from '../async/rsc/AsyncRating'; | ||
| import Spinner from '../shared/Spinner'; | ||
| import { RestaurantCardHeader } from '../restaurant/RestaurantCardHeader'; | ||
| import { RestaurantCardFooter } from '../restaurant/RestaurantCardFooter'; | ||
| import { CardWidgetsSkeleton } from '../shared/CardWidgetsSkeleton'; | ||
| import AsyncRestaurantWidgetsRSC from '../restaurant/AsyncRestaurantWidgetsRSC'; | ||
|
|
||
| interface Restaurant { | ||
| id: number; | ||
| name: string; | ||
| cuisine_type: string; | ||
| city: string; | ||
| state: string; | ||
| image_url: string; | ||
| latitude: number; | ||
| longitude: number; | ||
| average_rating: number; | ||
| review_count: number; | ||
| } | ||
|
|
||
| interface Props { | ||
| restaurant_id: number; | ||
| restaurants: Restaurant[]; | ||
| getReactOnRailsAsyncProp: (propName: string) => Promise<any>; | ||
| } | ||
|
|
||
| // This is an async server component (RSC) | ||
| // No "use client" = goes in RSC bundle | ||
| async function SearchPageRSC({ restaurant_id }: Props) { | ||
| // Async server component (RSC) — no hooks allowed | ||
| export default async function SearchPageRSC({ restaurants, getReactOnRailsAsyncProp }: Props) { | ||
| return ( | ||
| <div className="container mx-auto p-4"> | ||
| <h1 className="text-3xl font-bold mb-6">Restaurant Details</h1> | ||
|
|
||
| {/* Static content */} | ||
| <RestaurantCardHeader restaurantId={restaurant_id} /> | ||
|
|
||
| {/* Dynamic content - all data fetched SERVER-SIDE */} | ||
| <div className="space-y-6"> | ||
| <div className="grid grid-cols-2 gap-4"> | ||
| <Suspense fallback={<Spinner label="Checking status..." />}> | ||
| <AsyncStatus restaurantId={restaurant_id} /> | ||
| </Suspense> | ||
|
|
||
| <Suspense fallback={<Spinner label="Getting wait time..." />}> | ||
| <AsyncWaitTime restaurantId={restaurant_id} /> | ||
| </Suspense> | ||
|
|
||
| <Suspense fallback={<Spinner label="Loading specials..." />}> | ||
| <AsyncSpecials restaurantId={restaurant_id} /> | ||
| </Suspense> | ||
|
|
||
| <Suspense fallback={<Spinner label="Finding trending items..." />}> | ||
| <AsyncTrending restaurantId={restaurant_id} /> | ||
| </Suspense> | ||
|
|
||
| <Suspense fallback={<Spinner label="Fetching ratings..." />}> | ||
| <AsyncRating restaurantId={restaurant_id} /> | ||
| </Suspense> | ||
| </div> | ||
| <h1 className="text-3xl font-bold mb-6">Restaurant Search — RSC Streaming</h1> | ||
| <p className="text-gray-500 mb-4">Data streamed from server as each piece resolves.</p> | ||
|
|
||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> | ||
| {restaurants.map((restaurant) => ( | ||
| <div key={restaurant.id} className="bg-white rounded-lg shadow-md p-4"> | ||
| {/* Static content — rendered immediately */} | ||
| <RestaurantCardHeader restaurant={restaurant} /> | ||
|
|
||
| {/* Async widgets — streamed as data resolves on server */} | ||
| <Suspense fallback={<CardWidgetsSkeleton />}> | ||
| <AsyncRestaurantWidgetsRSC | ||
| restaurantId={restaurant.id} | ||
| getReactOnRailsAsyncProp={getReactOnRailsAsyncProp} | ||
| /> | ||
| </Suspense> | ||
|
|
||
| <RestaurantCardFooter restaurant={restaurant} /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'SearchPageRSC.tsx' --exec cat {}Repository: shakacode/react-server-components-marketplace-demo
Length of output: 1681
Update the doc example to show the import statement instead of a local Restaurant interface definition.
The actual SearchPageRSC.tsx correctly imports Restaurant from ../../types (line 4), but the documentation snippet (lines 67-78) shows a local interface definition instead. The doc example should reflect the actual implementation to avoid confusion about the correct pattern.
Actual implementation
import { Restaurant } from '../../types';🤖 Prompt for AI Agents
In `@tasks/Task_4_RSC_Version.md` around lines 56 - 112, The doc snippet defines a
local Restaurant interface instead of importing the shared type used in the real
component; update the example by removing the local "interface Restaurant { ...
}" and add the import "import { Restaurant } from '../../types';" at the top,
and then update the Props type to reference that imported Restaurant type in the
SearchPageRSC component so the example matches the actual implementation (refer
to SearchPageRSC and Props/Restaurant symbols).
| def search_rsc | ||
| @restaurants = fetch_restaurants_basic | ||
| stream_view_containing_react_components(template: "restaurants/search_rsc") | ||
| end |
There was a problem hiding this comment.
Doc references fetch_restaurants_basic but controller defines fetch_restaurants.
Line 221 shows @restaurants = fetch_restaurants_basic but the actual RestaurantsController (Line 41 of restaurants_controller.rb) uses fetch_restaurants. This mismatch will confuse anyone using this doc as a reference.
Proposed fix
- `@restaurants` = fetch_restaurants_basic
+ `@restaurants` = fetch_restaurants📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def search_rsc | |
| @restaurants = fetch_restaurants_basic | |
| stream_view_containing_react_components(template: "restaurants/search_rsc") | |
| end | |
| def search_rsc | |
| `@restaurants` = fetch_restaurants | |
| stream_view_containing_react_components(template: "restaurants/search_rsc") | |
| end |
🤖 Prompt for AI Agents
In `@tasks/Task_4_RSC_Version.md` around lines 220 - 223, The doc incorrectly
references fetch_restaurants_basic while the controller defines
fetch_restaurants; update the documentation in Task_4_RSC_Version.md (the
search_rsc example) to call `@restaurants` = fetch_restaurants to match the
RestaurantsController method name, or alternatively rename the controller method
to fetch_restaurants_basic if you intend that name—ensure the method name in the
search_rsc snippet and the RestaurantsController (fetch_restaurants) are
identical so readers aren't confused.
2 participants
Three search page implementations demonstrating RSC performance improvements:
Co-Authored-By: Claude Opus 4.6 [email protected]
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes