Add e-commerce product page with SSR, Client, and RSC versions#22
Merged
AbanoubGhadban merged 8 commits intomainfrom Feb 28, 2026
Merged
Add e-commerce product page with SSR, Client, and RSC versions#22AbanoubGhadban merged 8 commits intomainfrom
AbanoubGhadban merged 8 commits intomainfrom
Conversation
…omparison Implements a realistic e-commerce product page demo (#17) with three rendering strategies. Features product gallery, reviews (50K per product), rating distribution charts, markdown descriptions with syntax highlighting, related products, and add-to-cart interactions. RSC version uses Suspense streaming to progressively load heavy sections while keeping ~400KB of libraries (marked, highlight.js, date-fns) server-side only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add fetchpriority="high" and decoding="async" to main product image - Add loading="lazy" to thumbnail images to reduce bandwidth contention - Move below-the-fold content (description, features, specs) behind Suspense boundary to shrink initial RSC stream and prioritize LCP - Add product page entries to benchmark scripts (constants, collectors, runner) with per-page selector overrides Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RSC LCP improved from 2772ms to 2044ms on Slow 3G — now fastest across all 3 page versions (SSR 2346ms, Client 2318ms). Key optimizations: - Add <link rel="preload" as="image"> for hero product image in all views - Replace 85+ inline SVG star icons with Unicode ★ characters, reducing RSC payload by ~20KB uncompressed - Enable Rack::Deflater gzip compression in production (137KB → 14.6KB) - Reduce RSC review count from 10 to 5 to trim async payload - Add loading="lazy" to related product images - Reuse shared StarRating component across ReviewCard and ReviewDistributionChart instead of duplicating inline SVGs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add database indexes for dashboard analytics queries: - idx_orders_placed_at on orders(placed_at) - idx_orders_placed_at_status on orders(placed_at, status) - idx_order_lines_order_menu on order_lines(order_id, menu_item_id, quantity, price_per_unit) Add .gitignore entries for public/assets/ (sprocket compiled output) and ad-hoc test-*.js scripts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR implements a comprehensive e-commerce product page demonstration across three rendering strategies (SSR, Client-side, RSC with streaming). It adds Product and ProductReview models with database schemas, three controller endpoints (ProductsController and Api::ProductsController) for serving product data, multiple React components for different rendering approaches, corresponding view templates, and seed data for testing. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ServerSSR as Server (SSR)
participant API as API Endpoints
participant DB as Database
rect rgba(100, 150, 200, 0.5)
Note over Browser,DB: Strategy 1: Full Server-Side Rendering (SSR)
Browser->>ServerSSR: GET /product/ssr
ServerSSR->>DB: fetch product, reviews, stats, related
DB-->>ServerSSR: product data + reviews
ServerSSR->>ServerSSR: serialize to React props
ServerSSR-->>Browser: full HTML (React + data)
Browser->>Browser: hydrate and render
end
rect rgba(150, 100, 200, 0.5)
Note over Browser,API: Strategy 2: Client-Side Data Loading
Browser->>ServerSSR: GET /product/client
ServerSSR->>DB: fetch product only
ServerSSR-->>Browser: minimal HTML + product
Browser->>Browser: hydrate and render shell
Browser->>API: fetch review_stats, reviews, related_products
API->>DB: query reviews, stats, related
DB-->>API: data
API-->>Browser: JSON responses
Browser->>Browser: update state and re-render
end
rect rgba(200, 150, 100, 0.5)
Note over Browser,API: Strategy 3: React Server Components (RSC) with Streaming
Browser->>ServerSSR: GET /product/rsc
ServerSSR->>DB: fetch product (no description/specs)
ServerSSR-->>Browser: stream: hero section HTML
Browser->>Browser: render hero (LCP)
ServerSSR->>DB: fetch product_details (async)
ServerSSR-->>Browser: stream: description, specs
ServerSSR->>DB: fetch review_stats
ServerSSR-->>Browser: stream: review distribution
ServerSSR->>DB: fetch reviews (top 5)
ServerSSR-->>Browser: stream: reviews list
ServerSSR->>DB: fetch related_products
ServerSSR-->>Browser: stream: related products grid
Browser->>Browser: progressively render sections as they arrive
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (14)
.gitignore (1)
117-118: Narrow thetest-*.jsignore scope to avoid masking real testsAt Line 118,
test-*.jsapplies across the repo, so it may hide legitimate files unintentionally. If this is only for root-level ad-hoc scripts, scope it explicitly.Proposed .gitignore tweak
-# Ad-hoc test/debug scripts -test-*.js +# Ad-hoc test/debug scripts (root only) +/test-*.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 117 - 118, The global .gitignore pattern "test-*.js" is too broad and may hide real tests; narrow its scope by replacing it with a root-only or directory-scoped pattern (e.g., change "test-*.js" to "/test-*.js" for only repo-root ad-hoc scripts or to "scripts/test-*.js" if these live in a specific folder) so legitimate test files elsewhere are not ignored.config/database.yml (1)
19-19: Simplify optional host lookup
ENV.fetch('DATABASE_HOST', nil)is equivalent toENV['DATABASE_HOST']here. Prefer the simpler form for readability.Suggested cleanup
- host: <%= ENV.fetch('DATABASE_HOST', nil) %> + host: <%= ENV['DATABASE_HOST'] %>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/database.yml` at line 19, Replace the verbose ENV.fetch call for the database host with the simpler ENV access: update the host entry (the line setting host / referencing DATABASE_HOST) to use ENV['DATABASE_HOST'] instead of ENV.fetch('DATABASE_HOST', nil) to improve readability while preserving the same behavior.scripts/lib/constants.mjs (1)
12-14: Consider more specific selectors for benchmarking accuracy.The
likeButton: 'button'selector is very generic and will match any<button>element on the page. If the benchmarking script relies on this to measure interactivity, it may produce inconsistent results. Consider using a more specific selector (e.g.,'[data-testid="add-to-cart"]'or a specific class).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lib/constants.mjs` around lines 12 - 14, The likeButton selector for the product pages ('product-ssr', 'product-client', 'product-rsc') is too generic ('button') and can match unrelated buttons; update the selectors.likeButton value for those entries to a more specific DOM target (for example a data-testid or component-specific class) so the benchmarking script consistently targets the intended interactive element—locate the objects named product-ssr/product-client/product-rsc in scripts/lib/constants.mjs and replace selectors.likeButton with a precise selector such as a data-testid or unique class used by the product page.db/seed_scripts/product_seeder.rb (1)
4-75: Style: RuboCop flagged missing trailing commas in multiline arrays.Static analysis flagged multiple
Style/TrailingCommaInArrayLiteralviolations. Adding trailing commas to multiline arrays improves diff readability and is a common Ruby convention.♻️ Example fix for one array
REVIEW_TITLES_5 = [ 'Absolutely amazing!', 'Best purchase ever!', 'Exceeded expectations', 'Outstanding quality', 'Highly recommend!', 'Worth every penny', 'Perfect in every way', 'Game changer!', 'Love everything about it', - 'Five stars is not enough' + 'Five stars is not enough', ].freeze🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/seed_scripts/product_seeder.rb` around lines 4 - 75, RuboCop is complaining about missing trailing commas in multiline array literals; update each multiline array (REVIEW_TITLES_5, REVIEW_TITLES_4, REVIEW_TITLES_3, REVIEW_TITLES_2, REVIEW_TITLES_1) and every array value inside the REVIEW_COMMENTS hash (keys 5,4,3,2,1) to include a trailing comma after the last element so they end with a comma before the closing ] while preserving the existing .freeze and formatting.app/javascript/components/product/AddToCartSection.tsx (1)
82-87: Buy Now button has noonClickhandler.The "Buy Now" button is rendered but doesn't have any click handler. Is this intentional as a placeholder, or should it trigger some action (e.g., direct checkout)?
Would you like me to help implement the Buy Now functionality or add a TODO comment to track this?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/AddToCartSection.tsx` around lines 82 - 87, The Buy Now button in AddToCartSection has no onClick handler; add a handler (e.g., handleBuyNow) inside the AddToCartSection component and wire it to the button's onClick to perform the intended fast-checkout flow: call the existing addToCart (or create addToCartAndCheckout/proceedToCheckout) to add the current SKU/quantity, then redirect to the checkout page (or open the hosted checkout), handling errors and preventing double-submits via a local isBuying/loading state; if this is intentionally a placeholder instead of implementing checkout, add a clear TODO comment on the button and/or wire it to a no-op stub that logs the intent. Ensure the handler respects the inStock prop and updates UI state (disabled/loading) accordingly.db/seeds.rb (1)
19-21: Consider reducing review count for 'small' mode.50,000 reviews per product seems excessive for a "small" seed mode intended for quick local development. This could result in slow seeding and database bloat during development.
💡 Suggested adjustment for small mode
# Always seed products (idempotent) -reviews_count = mode == 'full' ? 100_000 : 50_000 +reviews_count = mode == 'full' ? 100_000 : 1_000 ProductSeeder.seed!(reviews_per_product: reviews_count)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/seeds.rb` around lines 19 - 21, The current seeding sets reviews_count via reviews_count = mode == 'full' ? 100_000 : 50_000, which makes the non-full (small) mode still seed 50,000 reviews per product; change the conditional so 'small' uses a much lower number (e.g., 5_000) and keep 'full' at 100_000 — update the reviews_count assignment (or replace it with a case/if on mode) and then call ProductSeeder.seed!(reviews_per_product: reviews_count) unchanged; specifically modify the reviews_count logic that references mode and the variable reviews_count to return lower counts for 'small' (and optionally a medium value for 'medium') to speed local development.scripts/lib/collectors.mjs (1)
20-23: The escaping concern applies only if selectors become user-controlled—current usage is safe.While the pattern of directly interpolating values into a template literal is theoretically vulnerable to injection (e.g., if a selector or text contains
'or\), the current code only passes hardcoded selectors from thePAGESconstant inconstants.mjs. All values—'button','Customers Also Viewed','h2','Related Posts'—contain no special characters that would break the injected JavaScript.If selectors ever become configurable from external input (environment variables, API responses, user uploads, etc.), escaping with
JSON.stringifywould be necessary. For now, no action is required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lib/collectors.mjs` around lines 20 - 23, The getCollectorScript function currently interpolates selector/text values (likeSelector, headingSelector, headingText derived from SELECTORS) directly into a template literal; confirm these values remain hardcoded in constants.mjs/PAGES so no escaping is needed now, and if any of these (likeButton, relatedHeadingText) become externally configurable later, update getCollectorScript to serialize/escape them (e.g., via JSON.stringify) before interpolation or validate/whitelist inputs; ensure references are to getCollectorScript, likeSelector, headingSelector, headingText, SELECTORS and the constants file when making this change.app/javascript/components/product/ProductPageClient.tsx (1)
15-31: Consider extracting the skeleton to a reusable component.The inline skeleton fallback works, but could be extracted to align with the
ProductSkeletons.tsxfile mentioned in the AI summary. This would improve consistency and reusability.♻️ Example refactor
+import { ProductContentSkeleton } from './ProductSkeletons'; + const AsyncProductContent = loadable( () => import('./AsyncProductContent'), { - fallback: ( - <div className="animate-pulse space-y-8 mt-8"> - <div className="border-t border-gray-200 pt-8"> - <div className="h-7 w-48 bg-gray-200 rounded mb-4" /> - <div className="space-y-3"> - <div className="h-4 bg-gray-200 rounded w-full" /> - <div className="h-4 bg-gray-200 rounded w-5/6" /> - <div className="h-4 bg-gray-200 rounded w-4/5" /> - </div> - </div> - </div> - ) + fallback: <ProductContentSkeleton /> } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ProductPageClient.tsx` around lines 15 - 31, Extract the inline skeleton JSX used as the fallback for loadable AsyncProductContent into a reusable component (e.g., ProductSkeleton) inside ProductSkeletons.tsx and replace the inline fallback with that component; specifically, create/export ProductSkeleton in ProductSkeletons.tsx containing the current animate-pulse markup, then update the AsyncProductContent loadable call (the fallback option in the loadable(...) invocation) to render <ProductSkeleton /> so the skeleton is centralized and reusable across the product components.app/javascript/components/product/RelatedProducts.tsx (2)
17-50: Product cards are styled as interactive but not clickable.The cards have hover effects (
hover:shadow-lg,hover:border-gray-300, image scale on hover) suggesting they should be clickable, but they're rendered as non-interactive<div>elements with no link or navigation. If users should be able to navigate to these products, consider wrapping the card content in an<a>or using a clickable component.If this is intentional for the demo, this can be ignored.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/RelatedProducts.tsx` around lines 17 - 50, The product cards in RelatedProducts.tsx are rendered as non-interactive divs (the element keyed by product.id) while using hover styles that imply clickability; wrap the card content (the outer div currently applying classes like "group bg-white rounded-xl ...") in a semantic clickable element such as a Next.js <Link> or an <a> that navigates to the product detail route (e.g., `/products/${product.handle}` or product.id), move the key from the wrapper if necessary to the Link, and ensure accessible attributes are added (aria-label or meaningful link text) and any onClick handlers propagate; this preserves the hover styles and makes StarRating and image interactions work as real navigation controls.
23-28: External placeholder service dependency.The fallback
https://via.placeholder.com/400relies on an external service. For a demo this is fine, but for production consider using a local placeholder or a more reliable CDN asset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/RelatedProducts.tsx` around lines 23 - 28, The component RelatedProducts.tsx currently falls back to an external placeholder URL in the img src (product.images[0]?.url || 'https://via.placeholder.com/400'); replace that external dependency with a local or CDN-hosted asset by referencing a local constant (e.g., PLACEHOLDER_IMAGE) or a public/static path (e.g., '/images/placeholder-400.png'), update the src expression to use that constant (product.images[0]?.url || PLACEHOLDER_IMAGE), and ensure the placeholder file is added to your repo's public/static assets or imported where appropriate so the component no longer depends on the external service.app/controllers/products_controller.rb (1)
41-43: Inconsistent image data structure handling.The
hero_image_urlmethod checks both string ("url") and symbol (:url) keys, suggesting inconsistency in howimagesdata is serialized. This may indicate theproduct.imagesJSON column returns different key types depending on context.Consider normalizing the images data structure at the model level or using
with_indifferent_accessconsistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/products_controller.rb` around lines 41 - 43, The hero_image_url method is compensating for inconsistent key types by checking both "url" and :url; normalize product images at the model so controllers can assume one access pattern. In the Product (or relevant) model convert the images JSON to a HashWithIndifferentAccess (e.g., normalize in an images accessor or a before_save/after_find hook) so `@product_data` (or product.images) always supports indifferent access; then simplify hero_image_url to a single dig call (referencing hero_image_url, `@product_data`, and product.images) knowing keys are normalized.app/controllers/api/products_controller.rb (1)
41-68: Consider extracting shared serializers to reduce duplication.
serialize_reviewandserialize_product_cardare duplicated in bothApi::ProductsControllerandProductsController. Consider extracting to a concern or service object.♻️ Example extraction
# app/serializers/product_serializers.rb or app/controllers/concerns/product_serialization.rb module ProductSerialization extend ActiveSupport::Concern private def serialize_review(review) { id: review.id, rating: review.rating, # ... rest of fields } end def serialize_product_card(product) # ... end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/controllers/api/products_controller.rb` around lines 41 - 68, Extract the duplicated serialization methods into a shared module (e.g., ProductSerialization) and include it in both Api::ProductsController and ProductsController; move serialize_review and serialize_product_card into that module as private methods, keep their signatures and return shapes identical, and update both controllers to remove the inline methods and use the included concern to avoid duplication.app/javascript/components/product/ReviewCard.tsx (1)
14-16: Consider defensive parsing for potentially invalid dates.
parseISOwill returnInvalid Dateifreview.created_atis malformed, causingformatDistanceToNowandformatto produce unexpected output (e.g., "Invalid Date"). If the data source is untrusted or could have edge cases, a try-catch or validation would improve robustness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/javascript/components/product/ReviewCard.tsx` around lines 14 - 16, The code currently calls parseISO(review.created_at) and then uses formatDistanceToNow and format on the result; add defensive validation around this flow by checking that review.created_at is present and that parseISO(review.created_at) yields a valid Date (e.g., !isNaN(reviewDate.getTime())) before computing relativeTime and absoluteDate; if invalid, set relativeTime and absoluteDate to safe fallbacks (empty string or "Unknown date") and avoid calling formatDistanceToNow/format to prevent "Invalid Date" output—update the logic around reviewDate, relativeTime, absoluteDate in ReviewCard (where parseISO, formatDistanceToNow, and format are used).db/schema.rb (1)
69-70: Verify whetheridx_orders_placed_atis redundant.With a B-tree index on
(placed_at, status), manyplaced_at-only lookups can already use the leftmost prefix. Keeping both may add write cost with limited read benefit—worth validating against real query patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@db/schema.rb` around lines 69 - 70, The duplicate B-tree indexes likely make idx_orders_placed_at redundant because idx_orders_placed_at_status already provides a leftmost-prefix on placed_at; verify by examining query patterns and explain/remove the redundant index: inspect queries using placed_at (e.g., find-by or range filters) and confirm they can use idx_orders_placed_at_status, run EXPLAIN/EXPLAIN ANALYZE against representative queries to see index usage, then either delete the separate index named idx_orders_placed_at from the schema/migration or add a comment in schema.rb documenting why both indexes are required if you find a case where idx_orders_placed_at is needed (reference the indexes idx_orders_placed_at_status and idx_orders_placed_at and the columns placed_at and status).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/controllers/api/products_controller.rb`:
- Around line 29-37: The related_products method on Product is causing an N+1
because serialize_product_card reads product.images for each related product;
update the Product#related_products implementation to eager-load images (use
includes(:images)) so the query preloads images before limit is applied, keeping
the same filtering and ordering logic and the method name related_products to
locate the change.
In `@app/javascript/components/product/AddToCartSection.tsx`:
- Around line 23-26: The handleAddToCart callback sets a 2s timeout via
setTimeout but never clears it, risking setAddedToCart(false) running after
unmount; modify AddToCartSection to store the timeout ID (via a ref like
timeoutRef) when calling setTimeout in handleAddToCart and add a useEffect
cleanup that clears the timeout (clearTimeout(timeoutRef.current)) on unmount,
or alternatively track mounted state in a ref and check it before calling
setAddedToCart(false); ensure references to handleAddToCart and setAddedToCart
remain consistent and the timeout ID is cleared to prevent the memory leak.
In `@app/javascript/components/product/AsyncProductContent.tsx`:
- Around line 30-43: The three fetch calls (for review_stats, reviews,
related_products) currently parse JSON without checking response.ok and swallow
errors via empty .catch, causing perpetual skeletons; update each fetch to first
check response.ok (e.g., fetch(...).then(r => { if (!r.ok) throw new
Error(`${r.status} ${r.statusText}`); return r.json(); }) ), then in the .then
handlers call setReviewStats / setReviews / setRelatedProducts as before, and in
the .catch handlers do meaningful error handling: console.error the error and
set sensible fallbacks (e.g., setReviewStats(null) or empty objects/arrays)
and/or flip any loading flags (use setXLoading if present) so the UI stops
showing skeletons; keep opts usage unchanged and apply the same pattern to all
three fetches.
In `@app/javascript/components/product/AsyncProductDetailsRSC.tsx`:
- Around line 14-20: The async payload from
getReactOnRailsAsyncProp('product_details') may be null/partial; normalize it
before rendering by ensuring the local details object always has safe defaults
(e.g., description: '', features: [], specs: {}) so ProductDescription,
ProductFeatures and ProductSpecs never receive undefined; update the code around
the details assignment (where getReactOnRailsAsyncProp is called) to
coerce/merge missing fields into those defaults (or use a helper
normalizeProductDetails) and then pass the normalized details to the child
components.
In `@app/javascript/components/product/ProductDescription.tsx`:
- Around line 11-20: The ProductDescription component currently inserts
unsanitized HTML from renderMarkdown into the DOM via dangerouslySetInnerHTML,
which exposes XSS risk; update renderMarkdown (or wrap its output before
assignment to html) to run the generated HTML through a sanitizer (e.g.,
sanitize-html or isomorphic-dompurify) and ensure sanitization happens
server-side or at least on the client before setting dangerouslySetInnerHTML;
reference the ProductDescription component, the renderMarkdown function, and the
html variable and replace the direct assignment with the sanitized HTML string
(or modify renderMarkdown to return sanitized output).
In `@app/javascript/components/product/ProductImageGallery.tsx`:
- Around line 11-16: The component ProductImageGallery can crash when images is
an empty array because selectedImage becomes undefined; update the logic to
guard against empty images by computing selectedImage as images[selectedIndex]
?? images[0] ?? null, and then either early-return null/a placeholder UI if
selectedImage is null or wrap all accesses (e.g., selectedImage.url in the
render and any image props) with a conditional check so you never dereference
undefined; adjust useState initialization or render path accordingly to ensure
no code reads properties off selectedImage when images.length === 0.
In `@app/javascript/components/product/ProductInfo.tsx`:
- Around line 62-69: The discount badge currently renders whenever
product.original_price exists even though product.discount_percentage can be
null; in the ProductInfo component change the rendering logic to only render the
discount <span> when product.discount_percentage is not null/undefined (and
ideally > 0) — keep the existing original_price block but add a second condition
checking product.discount_percentage (reference symbols: product.original_price
and product.discount_percentage in ProductInfo.tsx) and avoid interpolating a
null value into the badge text so it never shows "null% OFF".
In `@app/javascript/startup/ProductPageClient.server.tsx`:
- Line 1: The file ProductPageClient.server.tsx is a server-only entry that uses
renderToString and should not be marked as a client component; remove the "'use
client';" directive at the top (or replace it with a server-only marker if your
framework requires one) so the module is treated as a server component and the
bundler won't treat ProductPageClient.server.tsx as a client-bound module.
In `@app/models/product.rb`:
- Around line 20-39: The review_stats method mixes live and cached values
causing inconsistent payloads: compute average_rating and total_reviews from the
same live source as distribution (product_reviews) instead of using cached
columns (average_rating, review_count); update review_stats to calculate average
(e.g., from product_reviews.average(:rating) or by aggregating stats) and total
(e.g., stats.values.sum or product_reviews.count) so the returned
average_rating, total_reviews, and distribution are all derived from
product_reviews.
In `@config/database.yml`:
- Around line 16-17: The current config uses permissive fallbacks for the
database and username keys which can silently allow production to boot with
defaults; change the ERB so production requires environment variables: for the
database key replace the value with a conditional that calls
ENV.fetch('DATABASE_NAME') (no default) when Rails.env.production? and falls
back to the existing default ('localhub_demo_production') otherwise, and for the
username key require ENV.fetch('DATABASE_USERNAME') (no default) in production
but keep the current chained fallback (ENV['DATABASE_USERNAME'] ||
ENV['LOCALHUB_DEMO_DATABASE_USERNAME'] || 'localhub_demo') for non-production;
update the ERB around the database and username symbols to implement this
conditional behavior.
In `@db/migrate/20260223101148_create_products.rb`:
- Around line 13-15: Columns images, specs and features are defined with
defaults but still allow NULL which can break consumers; update the
migration/migrations to make them non-nullable by specifying null: false on the
t.jsonb declarations (e.g. t.jsonb :images, default: [], null: false; t.jsonb
:specs, default: {}, null: false; t.jsonb :features, default: [], null: false).
If this is an existing schema, add a new migration that (1) runs an UPDATE to
replace NULLs with the appropriate defaults for products (SET images = '[]',
specs = '{}', features = '[]' where they are NULL), (2) uses
change_column_default to set the defaults, and (3) uses change_column_null to
set null: false for each of images, specs and features so future writes cannot
persist NULL.
In `@db/migrate/20260223101156_create_product_reviews.rb`:
- Around line 7-12: Add database CHECK constraints to enforce rating and
helpful_count invariants in the CreateProductReviews migration: add a constraint
on rating to restrict it to the allowed range (e.g., 1..5) and a constraint on
helpful_count to ensure it is non-negative. Use Rails migration helpers
(add_check_constraint / remove_check_constraint) or raw SQL in the up/down (or
change) methods of the CreateProductReviews migration to create these checks for
the rating and helpful_count columns and ensure you remove them in the rollback
path.
In `@db/schema.rb`:
- Around line 101-107: The specified product columns (images, specs, features,
average_rating, review_count, stock_quantity, in_stock) currently have defaults
but allow NULLs; create a migration that hardens these by first updating any
existing NULL values to their defaults (e.g., images = [], specs = {}, features
= [], average_rating = 0.0, review_count = 0, stock_quantity = 100, in_stock =
true) and then alter each column to set null: false (use change_column_null or
appropriate migration helpers for jsonb/decimal/integer/boolean). Target the
columns named images, specs, features, average_rating, review_count,
stock_quantity and in_stock in the products table and ensure the migration is
reversible.
- Line 190: Update the foreign key definition so DB-level deletes cascade:
change the add_foreign_key call that references product_reviews -> products to
include on_delete: :cascade (or, in the create table, make the t.references/...
foreign_key option use { to_table: :products, on_delete: :cascade }); if the
original migration (20260223101156_create_product_reviews.rb) has already run in
environments, create a new migration that removes the existing FK and re-adds it
with on_delete: :cascade to enforce DB-level cascading deletes for
product_reviews.
In `@db/seed_scripts/product_seeder.rb`:
- Around line 356-357: The guard in seed_product_reviews (currently using
ProductReview.count > 0) prevents completing partially-failed runs; change it to
a per-product check so only products that already have reviews are skipped. In
seed_product_reviews, iterate over Product.find_each (or similar) and for each
product use ProductReview.where(product_id: product.id).exists? (or count) to
decide whether to seed reviews for that specific product, honoring
reviews_per_product for products that need seeding; this keeps idempotency while
allowing retries to finish incomplete products.
In `@package.json`:
- Line 39: Update the dependency version for react-on-rails-rsc: change the
version string for "react-on-rails-rsc" in package.json from "19.0.5-rc.1" to
the stable "19.0.2", then run your package manager (npm install or yarn install)
to update the lockfile and verify by running the test/build commands; ensure no
other code relies on RC-only APIs and adjust imports/usages if tests flag
incompatibilities in components that reference react-on-rails-rsc.
---
Nitpick comments:
In @.gitignore:
- Around line 117-118: The global .gitignore pattern "test-*.js" is too broad
and may hide real tests; narrow its scope by replacing it with a root-only or
directory-scoped pattern (e.g., change "test-*.js" to "/test-*.js" for only
repo-root ad-hoc scripts or to "scripts/test-*.js" if these live in a specific
folder) so legitimate test files elsewhere are not ignored.
In `@app/controllers/api/products_controller.rb`:
- Around line 41-68: Extract the duplicated serialization methods into a shared
module (e.g., ProductSerialization) and include it in both
Api::ProductsController and ProductsController; move serialize_review and
serialize_product_card into that module as private methods, keep their
signatures and return shapes identical, and update both controllers to remove
the inline methods and use the included concern to avoid duplication.
In `@app/controllers/products_controller.rb`:
- Around line 41-43: The hero_image_url method is compensating for inconsistent
key types by checking both "url" and :url; normalize product images at the model
so controllers can assume one access pattern. In the Product (or relevant) model
convert the images JSON to a HashWithIndifferentAccess (e.g., normalize in an
images accessor or a before_save/after_find hook) so `@product_data` (or
product.images) always supports indifferent access; then simplify hero_image_url
to a single dig call (referencing hero_image_url, `@product_data`, and
product.images) knowing keys are normalized.
In `@app/javascript/components/product/AddToCartSection.tsx`:
- Around line 82-87: The Buy Now button in AddToCartSection has no onClick
handler; add a handler (e.g., handleBuyNow) inside the AddToCartSection
component and wire it to the button's onClick to perform the intended
fast-checkout flow: call the existing addToCart (or create
addToCartAndCheckout/proceedToCheckout) to add the current SKU/quantity, then
redirect to the checkout page (or open the hosted checkout), handling errors and
preventing double-submits via a local isBuying/loading state; if this is
intentionally a placeholder instead of implementing checkout, add a clear TODO
comment on the button and/or wire it to a no-op stub that logs the intent.
Ensure the handler respects the inStock prop and updates UI state
(disabled/loading) accordingly.
In `@app/javascript/components/product/ProductPageClient.tsx`:
- Around line 15-31: Extract the inline skeleton JSX used as the fallback for
loadable AsyncProductContent into a reusable component (e.g., ProductSkeleton)
inside ProductSkeletons.tsx and replace the inline fallback with that component;
specifically, create/export ProductSkeleton in ProductSkeletons.tsx containing
the current animate-pulse markup, then update the AsyncProductContent loadable
call (the fallback option in the loadable(...) invocation) to render
<ProductSkeleton /> so the skeleton is centralized and reusable across the
product components.
In `@app/javascript/components/product/RelatedProducts.tsx`:
- Around line 17-50: The product cards in RelatedProducts.tsx are rendered as
non-interactive divs (the element keyed by product.id) while using hover styles
that imply clickability; wrap the card content (the outer div currently applying
classes like "group bg-white rounded-xl ...") in a semantic clickable element
such as a Next.js <Link> or an <a> that navigates to the product detail route
(e.g., `/products/${product.handle}` or product.id), move the key from the
wrapper if necessary to the Link, and ensure accessible attributes are added
(aria-label or meaningful link text) and any onClick handlers propagate; this
preserves the hover styles and makes StarRating and image interactions work as
real navigation controls.
- Around line 23-28: The component RelatedProducts.tsx currently falls back to
an external placeholder URL in the img src (product.images[0]?.url ||
'https://via.placeholder.com/400'); replace that external dependency with a
local or CDN-hosted asset by referencing a local constant (e.g.,
PLACEHOLDER_IMAGE) or a public/static path (e.g.,
'/images/placeholder-400.png'), update the src expression to use that constant
(product.images[0]?.url || PLACEHOLDER_IMAGE), and ensure the placeholder file
is added to your repo's public/static assets or imported where appropriate so
the component no longer depends on the external service.
In `@app/javascript/components/product/ReviewCard.tsx`:
- Around line 14-16: The code currently calls parseISO(review.created_at) and
then uses formatDistanceToNow and format on the result; add defensive validation
around this flow by checking that review.created_at is present and that
parseISO(review.created_at) yields a valid Date (e.g.,
!isNaN(reviewDate.getTime())) before computing relativeTime and absoluteDate; if
invalid, set relativeTime and absoluteDate to safe fallbacks (empty string or
"Unknown date") and avoid calling formatDistanceToNow/format to prevent "Invalid
Date" output—update the logic around reviewDate, relativeTime, absoluteDate in
ReviewCard (where parseISO, formatDistanceToNow, and format are used).
In `@config/database.yml`:
- Line 19: Replace the verbose ENV.fetch call for the database host with the
simpler ENV access: update the host entry (the line setting host / referencing
DATABASE_HOST) to use ENV['DATABASE_HOST'] instead of ENV.fetch('DATABASE_HOST',
nil) to improve readability while preserving the same behavior.
In `@db/schema.rb`:
- Around line 69-70: The duplicate B-tree indexes likely make
idx_orders_placed_at redundant because idx_orders_placed_at_status already
provides a leftmost-prefix on placed_at; verify by examining query patterns and
explain/remove the redundant index: inspect queries using placed_at (e.g.,
find-by or range filters) and confirm they can use idx_orders_placed_at_status,
run EXPLAIN/EXPLAIN ANALYZE against representative queries to see index usage,
then either delete the separate index named idx_orders_placed_at from the
schema/migration or add a comment in schema.rb documenting why both indexes are
required if you find a case where idx_orders_placed_at is needed (reference the
indexes idx_orders_placed_at_status and idx_orders_placed_at and the columns
placed_at and status).
In `@db/seed_scripts/product_seeder.rb`:
- Around line 4-75: RuboCop is complaining about missing trailing commas in
multiline array literals; update each multiline array (REVIEW_TITLES_5,
REVIEW_TITLES_4, REVIEW_TITLES_3, REVIEW_TITLES_2, REVIEW_TITLES_1) and every
array value inside the REVIEW_COMMENTS hash (keys 5,4,3,2,1) to include a
trailing comma after the last element so they end with a comma before the
closing ] while preserving the existing .freeze and formatting.
In `@db/seeds.rb`:
- Around line 19-21: The current seeding sets reviews_count via reviews_count =
mode == 'full' ? 100_000 : 50_000, which makes the non-full (small) mode still
seed 50,000 reviews per product; change the conditional so 'small' uses a much
lower number (e.g., 5_000) and keep 'full' at 100_000 — update the reviews_count
assignment (or replace it with a case/if on mode) and then call
ProductSeeder.seed!(reviews_per_product: reviews_count) unchanged; specifically
modify the reviews_count logic that references mode and the variable
reviews_count to return lower counts for 'small' (and optionally a medium value
for 'medium') to speed local development.
In `@scripts/lib/collectors.mjs`:
- Around line 20-23: The getCollectorScript function currently interpolates
selector/text values (likeSelector, headingSelector, headingText derived from
SELECTORS) directly into a template literal; confirm these values remain
hardcoded in constants.mjs/PAGES so no escaping is needed now, and if any of
these (likeButton, relatedHeadingText) become externally configurable later,
update getCollectorScript to serialize/escape them (e.g., via JSON.stringify)
before interpolation or validate/whitelist inputs; ensure references are to
getCollectorScript, likeSelector, headingSelector, headingText, SELECTORS and
the constants file when making this change.
In `@scripts/lib/constants.mjs`:
- Around line 12-14: The likeButton selector for the product pages
('product-ssr', 'product-client', 'product-rsc') is too generic ('button') and
can match unrelated buttons; update the selectors.likeButton value for those
entries to a more specific DOM target (for example a data-testid or
component-specific class) so the benchmarking script consistently targets the
intended interactive element—locate the objects named
product-ssr/product-client/product-rsc in scripts/lib/constants.mjs and replace
selectors.likeButton with a precise selector such as a data-testid or unique
class used by the product page.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
.gitignoreapp/controllers/api/products_controller.rbapp/controllers/products_controller.rbapp/javascript/components/product/AddToCartSection.tsxapp/javascript/components/product/AsyncProductContent.tsxapp/javascript/components/product/AsyncProductDetailsRSC.tsxapp/javascript/components/product/AsyncRelatedProductsRSC.tsxapp/javascript/components/product/AsyncReviewStatsRSC.tsxapp/javascript/components/product/AsyncReviewsRSC.tsxapp/javascript/components/product/ProductDescription.tsxapp/javascript/components/product/ProductFeatures.tsxapp/javascript/components/product/ProductImageGallery.tsxapp/javascript/components/product/ProductInfo.tsxapp/javascript/components/product/ProductPageClient.tsxapp/javascript/components/product/ProductPageRSC.tsxapp/javascript/components/product/ProductPageSSR.tsxapp/javascript/components/product/ProductSkeletons.tsxapp/javascript/components/product/ProductSpecs.tsxapp/javascript/components/product/RelatedProducts.tsxapp/javascript/components/product/ReviewCard.tsxapp/javascript/components/product/ReviewDistributionChart.tsxapp/javascript/components/product/ReviewsList.tsxapp/javascript/startup/ProductPageClient.client.tsxapp/javascript/startup/ProductPageClient.server.tsxapp/javascript/startup/ProductPageRSC.tsxapp/javascript/startup/ProductPageSSR.tsxapp/javascript/types/product.tsapp/models/product.rbapp/models/product_review.rbapp/views/home/index.html.erbapp/views/products/show_client.html.erbapp/views/products/show_rsc.html.erbapp/views/products/show_ssr.html.erbconfig/database.ymlconfig/environments/production.rbconfig/routes.rbdb/migrate/20260223101148_create_products.rbdb/migrate/20260223101156_create_product_reviews.rbdb/schema.rbdb/seed_scripts/product_seeder.rbdb/seeds.rbpackage.jsonscripts/lib/collectors.mjsscripts/lib/constants.mjsscripts/lib/runner.mjs
Comment on lines
+29
to
+37
| def related_products | ||
| product = Product.find(params[:id]) | ||
| related = product.related_products(4) | ||
|
|
||
| render json: { | ||
| products: related.map { |p| serialize_product_card(p) }, | ||
| timestamp: Time.current.iso8601 | ||
| } | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Product model
fd -t f -name "*.rb" | xargs grep -l "class Product" | head -5Repository: shakacode/react-server-components-marketplace-demo
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Search for the related_products method definition
rg "def related_products" -A 10Repository: shakacode/react-server-components-marketplace-demo
Length of output: 1386
🏁 Script executed:
#!/bin/bash
# Search for serialize_product_card method
rg "def serialize_product_card" -A 10Repository: shakacode/react-server-components-marketplace-demo
Length of output: 1744
Potential N+1 query in related_products serialization.
The related_products method in the Product model doesn't eager-load the images association. Since serialize_product_card accesses product.images for each product, this will trigger a separate database query for each of the 4 related products. Add .includes(:images) to the query in the model method to preload associations:
def related_products(limit = 4)
Product
.where(category: category)
.where.not(id: id)
.order(average_rating: :desc, review_count: :desc)
.includes(:images)
.limit(limit)
end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/controllers/api/products_controller.rb` around lines 29 - 37, The
related_products method on Product is causing an N+1 because
serialize_product_card reads product.images for each related product; update the
Product#related_products implementation to eager-load images (use
includes(:images)) so the query preloads images before limit is applied, keeping
the same filtering and ordering logic and the method name related_products to
locate the change.
Comment on lines
+23
to
+26
| const handleAddToCart = useCallback(() => { | ||
| setAddedToCart(true); | ||
| setTimeout(() => setAddedToCart(false), 2000); | ||
| }, []); |
There was a problem hiding this comment.
Potential memory leak: setTimeout not cleared on unmount.
If the component unmounts before the 2-second timeout completes, setAddedToCart(false) will attempt to update state on an unmounted component. Use useEffect cleanup or a ref to track mount status.
🛠️ Proposed fix using useEffect cleanup
+import React, { useState, useCallback, useEffect, useRef } from 'react';
// Inside the component:
+ const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) clearTimeout(timeoutRef.current);
+ };
+ }, []);
+
const handleAddToCart = useCallback(() => {
setAddedToCart(true);
- setTimeout(() => setAddedToCart(false), 2000);
+ timeoutRef.current = setTimeout(() => setAddedToCart(false), 2000);
}, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/product/AddToCartSection.tsx` around lines 23 - 26,
The handleAddToCart callback sets a 2s timeout via setTimeout but never clears
it, risking setAddedToCart(false) running after unmount; modify AddToCartSection
to store the timeout ID (via a ref like timeoutRef) when calling setTimeout in
handleAddToCart and add a useEffect cleanup that clears the timeout
(clearTimeout(timeoutRef.current)) on unmount, or alternatively track mounted
state in a ref and check it before calling setAddedToCart(false); ensure
references to handleAddToCart and setAddedToCart remain consistent and the
timeout ID is cleared to prevent the memory leak.
Comment on lines
+30
to
+43
| fetch(`/api/products/${product.id}/review_stats`, opts) | ||
| .then((r) => r.json()) | ||
| .then(setReviewStats) | ||
| .catch(() => {}); | ||
|
|
||
| fetch(`/api/products/${product.id}/reviews`, opts) | ||
| .then((r) => r.json()) | ||
| .then((data) => setReviews(data.reviews)) | ||
| .catch(() => {}); | ||
|
|
||
| fetch(`/api/products/${product.id}/related_products`, opts) | ||
| .then((r) => r.json()) | ||
| .then((data) => setRelatedProducts(data.products)) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
Silent error handling leaves users with infinite loading state.
The empty .catch(() => {}) blocks swallow all errors, including network failures and server errors. If any fetch fails, the corresponding section shows a skeleton forever with no user feedback.
Additionally, there's no response.ok check before parsing JSON—4xx/5xx responses may have non-JSON bodies or error structures that won't match expected types.
🐛 Proposed fix with error handling
+ const [error, setError] = useState<string | null>(null);
useEffect(() => {
const controller = new AbortController();
const opts = { signal: controller.signal };
+ const handleFetch = async <T,>(url: string, setter: (data: T) => void, transform?: (data: any) => T) => {
+ try {
+ const r = await fetch(url, opts);
+ if (!r.ok) throw new Error(`HTTP ${r.status}`);
+ const data = await r.json();
+ setter(transform ? transform(data) : data);
+ } catch (e) {
+ if (e instanceof Error && e.name !== 'AbortError') {
+ setError('Failed to load some content');
+ }
+ }
+ };
- fetch(`/api/products/${product.id}/review_stats`, opts)
- .then((r) => r.json())
- .then(setReviewStats)
- .catch(() => {});
+ handleFetch(`/api/products/${product.id}/review_stats`, setReviewStats);
- fetch(`/api/products/${product.id}/reviews`, opts)
- .then((r) => r.json())
- .then((data) => setReviews(data.reviews))
- .catch(() => {});
+ handleFetch(`/api/products/${product.id}/reviews`, setReviews, (d) => d.reviews);
- fetch(`/api/products/${product.id}/related_products`, opts)
- .then((r) => r.json())
- .then((data) => setRelatedProducts(data.products))
- .catch(() => {});
+ handleFetch(`/api/products/${product.id}/related_products`, setRelatedProducts, (d) => d.products);
return () => controller.abort();
}, [product.id]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/product/AsyncProductContent.tsx` around lines 30 -
43, The three fetch calls (for review_stats, reviews, related_products)
currently parse JSON without checking response.ok and swallow errors via empty
.catch, causing perpetual skeletons; update each fetch to first check
response.ok (e.g., fetch(...).then(r => { if (!r.ok) throw new
Error(`${r.status} ${r.statusText}`); return r.json(); }) ), then in the .then
handlers call setReviewStats / setReviews / setRelatedProducts as before, and in
the .catch handlers do meaningful error handling: console.error the error and
set sensible fallbacks (e.g., setReviewStats(null) or empty objects/arrays)
and/or flip any loading flags (use setXLoading if present) so the UI stops
showing skeletons; keep opts usage unchanged and apply the same pattern to all
three fetches.
Comment on lines
+14
to
+20
| const details = await getReactOnRailsAsyncProp('product_details'); | ||
|
|
||
| return ( | ||
| <> | ||
| <ProductDescription description={details.description} /> | ||
| <ProductFeatures features={details.features} /> | ||
| <ProductSpecs specs={details.specs} /> |
There was a problem hiding this comment.
Guard async payload fields before passing to child components.
If product_details is partial/null, Line 19–20 can crash (features.length, Object.entries(specs)). Normalize defaults before rendering.
💡 Suggested defensive normalization
export default async function AsyncProductDetailsRSC({ getReactOnRailsAsyncProp }: Props) {
const details = await getReactOnRailsAsyncProp('product_details');
+ const description = typeof details?.description === 'string' ? details.description : '';
+ const features = Array.isArray(details?.features) ? details.features : [];
+ const specs = details?.specs && typeof details.specs === 'object' ? details.specs : {};
return (
<>
- <ProductDescription description={details.description} />
- <ProductFeatures features={details.features} />
- <ProductSpecs specs={details.specs} />
+ <ProductDescription description={description} />
+ <ProductFeatures features={features} />
+ <ProductSpecs specs={specs} />
</>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/product/AsyncProductDetailsRSC.tsx` around lines 14
- 20, The async payload from getReactOnRailsAsyncProp('product_details') may be
null/partial; normalize it before rendering by ensuring the local details object
always has safe defaults (e.g., description: '', features: [], specs: {}) so
ProductDescription, ProductFeatures and ProductSpecs never receive undefined;
update the code around the details assignment (where getReactOnRailsAsyncProp is
called) to coerce/merge missing fields into those defaults (or use a helper
normalizeProductDetails) and then pass the normalized details to the child
components.
Comment on lines
+11
to
+20
| export function ProductDescription({ description }: Props) { | ||
| const html = renderMarkdown(description); | ||
|
|
||
| return ( | ||
| <section className="border-t border-gray-200 pt-8"> | ||
| <h2 className="text-xl font-bold text-gray-900 mb-6">Product Description</h2> | ||
| <div | ||
| className="prose prose-gray max-w-none prose-headings:text-gray-900 prose-headings:font-semibold prose-a:text-indigo-600 prose-code:text-indigo-600 prose-pre:bg-gray-900 prose-pre:text-gray-100" | ||
| dangerouslySetInnerHTML={{ __html: html }} | ||
| /> |
There was a problem hiding this comment.
Sanitize markdown output to prevent XSS.
The marked library does not sanitize HTML by default. If product descriptions contain malicious content (even from compromised admin accounts or imported data), the unsanitized HTML will be served to all users.
Consider using a sanitization library like sanitize-html or isomorphic-dompurify on the server:
🛡️ Proposed fix with sanitization
import React from 'react';
import { renderMarkdown } from '../../utils/renderMarkdown';
+import sanitizeHtml from 'sanitize-html';
interface Props {
description: string;
}
export function ProductDescription({ description }: Props) {
- const html = renderMarkdown(description);
+ const rawHtml = renderMarkdown(description);
+ const html = sanitizeHtml(rawHtml, {
+ allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img', 'pre', 'code']),
+ allowedAttributes: {
+ ...sanitizeHtml.defaults.allowedAttributes,
+ code: ['class'],
+ pre: ['class'],
+ span: ['class'],
+ },
+ });
return (📝 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.
Suggested change
| export function ProductDescription({ description }: Props) { | |
| const html = renderMarkdown(description); | |
| return ( | |
| <section className="border-t border-gray-200 pt-8"> | |
| <h2 className="text-xl font-bold text-gray-900 mb-6">Product Description</h2> | |
| <div | |
| className="prose prose-gray max-w-none prose-headings:text-gray-900 prose-headings:font-semibold prose-a:text-indigo-600 prose-code:text-indigo-600 prose-pre:bg-gray-900 prose-pre:text-gray-100" | |
| dangerouslySetInnerHTML={{ __html: html }} | |
| /> | |
| import React from 'react'; | |
| import { renderMarkdown } from '../../utils/renderMarkdown'; | |
| import sanitizeHtml from 'sanitize-html'; | |
| interface Props { | |
| description: string; | |
| } | |
| export function ProductDescription({ description }: Props) { | |
| const rawHtml = renderMarkdown(description); | |
| const html = sanitizeHtml(rawHtml, { | |
| allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img', 'pre', 'code']), | |
| allowedAttributes: { | |
| ...sanitizeHtml.defaults.allowedAttributes, | |
| code: ['class'], | |
| pre: ['class'], | |
| span: ['class'], | |
| }, | |
| }); | |
| return ( | |
| <section className="border-t border-gray-200 pt-8"> | |
| <h2 className="text-xl font-bold text-gray-900 mb-6">Product Description</h2> | |
| <div | |
| className="prose prose-gray max-w-none prose-headings:text-gray-900 prose-headings:font-semibold prose-a:text-indigo-600 prose-code:text-indigo-600 prose-pre:bg-gray-900 prose-pre:text-gray-100" | |
| dangerouslySetInnerHTML={{ __html: html }} | |
| /> | |
| </section> | |
| ); | |
| } |
🧰 Tools
🪛 ast-grep (0.41.0)
[warning] 18-18: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.4.4)
[error] 19-19: Avoid passing content using the dangerouslySetInnerHTML prop.
(lint/security/noDangerouslySetInnerHtml)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/javascript/components/product/ProductDescription.tsx` around lines 11 -
20, The ProductDescription component currently inserts unsanitized HTML from
renderMarkdown into the DOM via dangerouslySetInnerHTML, which exposes XSS risk;
update renderMarkdown (or wrap its output before assignment to html) to run the
generated HTML through a sanitizer (e.g., sanitize-html or isomorphic-dompurify)
and ensure sanitization happens server-side or at least on the client before
setting dangerouslySetInnerHTML; reference the ProductDescription component, the
renderMarkdown function, and the html variable and replace the direct assignment
with the sanitized HTML string (or modify renderMarkdown to return sanitized
output).
Comment on lines
+77
to
+91
| create_table "product_reviews", force: :cascade do |t| | ||
| t.bigint "product_id", null: false | ||
| t.integer "rating", null: false | ||
| t.string "title" | ||
| t.text "comment" | ||
| t.string "reviewer_name", null: false | ||
| t.boolean "verified_purchase", default: false | ||
| t.integer "helpful_count", default: 0 | ||
| t.datetime "created_at", null: false | ||
| t.datetime "updated_at", null: false | ||
| t.index ["product_id", "created_at"], name: "index_product_reviews_on_product_id_and_created_at" | ||
| t.index ["product_id", "helpful_count"], name: "index_product_reviews_on_product_id_and_helpful_count" | ||
| t.index ["product_id", "rating"], name: "index_product_reviews_on_product_id_and_rating" | ||
| t.index ["product_id"], name: "index_product_reviews_on_product_id" | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the schema.rb file to see the full context
cat -n db/schema.rb | sed -n '70,100p'Repository: shakacode/react-server-components-marketplace-demo
Length of output: 1872
🏁 Script executed:
# Search for the ProductReview model to see what validations exist
fd -e rb | xargs rg -l "class ProductReview" | head -5Repository: shakacode/react-server-components-marketplace-demo
Length of output: 122
🏁 Script executed:
# Look for any migrations that add check constraints to product_reviews
fd -e rb migration_files | xargs rg "add_check_constraint.*product_reviews" 2>/dev/null || rg "add_check_constraint.*product_reviews"Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
🏁 Script executed:
# Check if there are any existing check constraints in the schema
rg "check_constraint" db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 176
🏁 Script executed:
# Search for where the check_constraint appears in schema.rb
rg -n "check_constraint" db/schema.rb -B 5 -A 2Repository: shakacode/react-server-components-marketplace-demo
Length of output: 495
🏁 Script executed:
# Check the ProductReview model to see validations
cat app/models/product_review.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 295
🏁 Script executed:
# Search for helpful_count constraint
rg -n "helpful_count" db/schema.rb -B 2 -A 2Repository: shakacode/react-server-components-marketplace-demo
Length of output: 709
🏁 Script executed:
# Let's view the complete product_reviews section in schema.rb to confirm
sed -n '77,91p' db/schema.rb | cat -nRepository: shakacode/react-server-components-marketplace-demo
Length of output: 967
🏁 Script executed:
# Check if there are any other migrations that add constraints to product_reviews
fd -e rb db/migrate | xargs rg "product_reviews.*constraint" 2>/dev/null || echo "No product_reviews constraints found in migrations"Repository: shakacode/react-server-components-marketplace-demo
Length of output: 507
Add DB-level check constraints to product_reviews table.
The rating field (line 79) is non-null but lacks database validation for allowed bounds. The model has app-level validation (inclusion: { in: 1..5 }), but this can be bypassed by direct SQL or third-party tools. Additionally, helpful_count defaults to 0 but has no constraint preventing negative values.
Suggested migration
+class AddProductReviewCheckConstraints < ActiveRecord::Migration[7.2]
+ def change
+ add_check_constraint :product_reviews, "rating >= 1 AND rating <= 5", name: "product_reviews_rating_check"
+ add_check_constraint :product_reviews, "helpful_count >= 0", name: "product_reviews_helpful_count_check"
+ end
+end
Comment on lines
+101
to
+107
| t.jsonb "images", default: [] | ||
| t.jsonb "specs", default: {} | ||
| t.jsonb "features", default: [] | ||
| t.decimal "average_rating", precision: 3, scale: 2, default: "0.0" | ||
| t.integer "review_count", default: 0 | ||
| t.integer "stock_quantity", default: 100 | ||
| t.boolean "in_stock", default: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the schema file
find . -name "schema.rb" -type fRepository: shakacode/react-server-components-marketplace-demo
Length of output: 108
🏁 Script executed:
# Check the file size and then read the relevant section
wc -l db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 110
🏁 Script executed:
# Read lines 95-115 to get context around lines 101-107
sed -n '95,115p' db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 1066
🏁 Script executed:
# Search for the products table definition to see its full context
grep -n "create_table.*products" db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 146
🏁 Script executed:
# Look for any migrations that modify nullability constraints
find . -path ./node_modules -prune -o -name "*.rb" -type f -exec grep -l "change_column_null.*products" {} \;Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
Add null: false constraints to product columns with defaults to prevent null state drift.
Lines 101-107 define defaults (empty arrays/objects, zero values, true) but lack null: false constraints. This allows explicit NULL values despite the defaults, breaking assumptions in code that expects these columns to always contain the default values.
🔧 Suggested migration hardening
+class HardenProductsNullability < ActiveRecord::Migration[7.2]
+ def change
+ change_column_null :products, :images, false, []
+ change_column_null :products, :specs, false, {}
+ change_column_null :products, :features, false, []
+ change_column_null :products, :average_rating, false, 0.0
+ change_column_null :products, :review_count, false, 0
+ change_column_null :products, :stock_quantity, false, 100
+ change_column_null :products, :in_stock, false, true
+ end
+end📝 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.
Suggested change
| t.jsonb "images", default: [] | |
| t.jsonb "specs", default: {} | |
| t.jsonb "features", default: [] | |
| t.decimal "average_rating", precision: 3, scale: 2, default: "0.0" | |
| t.integer "review_count", default: 0 | |
| t.integer "stock_quantity", default: 100 | |
| t.boolean "in_stock", default: true | |
| t.jsonb "images", default: [], null: false | |
| t.jsonb "specs", default: {}, null: false | |
| t.jsonb "features", default: [], null: false | |
| t.decimal "average_rating", precision: 3, scale: 2, default: "0.0", null: false | |
| t.integer "review_count", default: 0, null: false | |
| t.integer "stock_quantity", default: 100, null: false | |
| t.boolean "in_stock", default: true, null: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/schema.rb` around lines 101 - 107, The specified product columns (images,
specs, features, average_rating, review_count, stock_quantity, in_stock)
currently have defaults but allow NULLs; create a migration that hardens these
by first updating any existing NULL values to their defaults (e.g., images = [],
specs = {}, features = [], average_rating = 0.0, review_count = 0,
stock_quantity = 100, in_stock = true) and then alter each column to set null:
false (use change_column_null or appropriate migration helpers for
jsonb/decimal/integer/boolean). Target the columns named images, specs,
features, average_rating, review_count, stock_quantity and in_stock in the
products table and ensure the migration is reversible.
| add_foreign_key "order_lines", "menu_items", on_delete: :restrict | ||
| add_foreign_key "order_lines", "orders", on_delete: :cascade | ||
| add_foreign_key "orders", "restaurants" | ||
| add_foreign_key "product_reviews", "products" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the schema.rb file around line 190
fd schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 106
🏁 Script executed:
# Read schema.rb around line 190 to see the FK definition
wc -l db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 110
🏁 Script executed:
# Read schema.rb around line 190 to see the FK definition
sed -n '185,194p' db/schema.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 543
🏁 Script executed:
# Search for ProductReview model
fd -e rb | xargs rg -l "class ProductReview" 2>/dev/nullRepository: shakacode/react-server-components-marketplace-demo
Length of output: 122
🏁 Script executed:
# Read the ProductReview model
cat -n app/models/product_review.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 351
🏁 Script executed:
# Search for Product model
fd -e rb -path "*/models/*" | xargs rg -l "class Product" | grep -v ReviewRepository: shakacode/react-server-components-marketplace-demo
Length of output: 446
🏁 Script executed:
# Read the Product model
cat -n app/models/product.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 2112
🏁 Script executed:
# Search for migrations that created or modified product_reviews table
fd "*.rb" db/migrate | xargs rg "product_reviews" -l 2>/dev/null | head -5Repository: shakacode/react-server-components-marketplace-demo
Length of output: 515
🏁 Script executed:
# Read the migration that created product_reviews
cat -n db/migrate/20260223101156_create_product_reviews.rbRepository: shakacode/react-server-components-marketplace-demo
Length of output: 868
🏁 Script executed:
# Check if there are any raw SQL operations on product_reviews
rg "product_reviews" --type rb | rg -i "execute|update_all|delete_all|raw" | head -20Repository: shakacode/react-server-components-marketplace-demo
Length of output: 120
🏁 Script executed:
# Check for raw SQL operations on product_reviews
rg "product_reviews" -A 2 | rg -i "execute|update_all|delete_all|raw|sql" -B 1 -A 1Repository: shakacode/react-server-components-marketplace-demo
Length of output: 76
Add cascading delete to the product_reviews foreign key for data integrity.
Line 190 adds the FK without on_delete: :cascade. While the Product model includes dependent: :destroy, this only protects against application-level deletes through ActiveRecord. Direct database operations would leave orphan reviews. The migration at line 6 of 20260223101156_create_product_reviews.rb should specify the cascade option.
🧩 Suggested migration update
+class UpdateProductReviewsForeignKeyOnDelete < ActiveRecord::Migration[7.2]
+ def change
+ remove_foreign_key :product_reviews, :products
+ add_foreign_key :product_reviews, :products, on_delete: :cascade
+ end
+end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/schema.rb` at line 190, Update the foreign key definition so DB-level
deletes cascade: change the add_foreign_key call that references product_reviews
-> products to include on_delete: :cascade (or, in the create table, make the
t.references/... foreign_key option use { to_table: :products, on_delete:
:cascade }); if the original migration
(20260223101156_create_product_reviews.rb) has already run in environments,
create a new migration that removes the existing FK and re-adds it with
on_delete: :cascade to enforce DB-level cascading deletes for product_reviews.
Comment on lines
+356
to
+357
| def seed_product_reviews(reviews_per_product:) | ||
| return if ProductReview.count > 0 |
There was a problem hiding this comment.
Guard condition may cause issues with partial seeding.
If a previous seed attempt failed partway through (e.g., after seeding some products but before completing reviews), this check would skip review seeding entirely on retry since ProductReview.count > 0. Consider checking per-product or using a more granular condition.
🛠️ Alternative approach: check per product
- def seed_product_reviews(reviews_per_product:)
- return if ProductReview.count > 0
+ def seed_product_reviews(reviews_per_product:)
+ product_ids = Product.left_joins(:product_reviews)
+ .group(:id)
+ .having('COUNT(product_reviews.id) = 0')
+ .pluck(:id)
+ return if product_ids.empty?
- product_ids = Product.pluck(:id)
total = product_ids.size * reviews_per_product🧰 Tools
🪛 RuboCop (1.84.2)
[convention] 356-404: Assignment Branch Condition size for seed_product_reviews is too high. [<18, 56, 9> 59.51/23]
(Metrics/AbcSize)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@db/seed_scripts/product_seeder.rb` around lines 356 - 357, The guard in
seed_product_reviews (currently using ProductReview.count > 0) prevents
completing partially-failed runs; change it to a per-product check so only
products that already have reviews are skipped. In seed_product_reviews, iterate
over Product.find_each (or similar) and for each product use
ProductReview.where(product_id: product.id).exists? (or count) to decide whether
to seed reviews for that specific product, honoring reviews_per_product for
products that need seeding; this keeps idempotency while allowing retries to
finish incomplete products.
| "react-on-rails-pro": "file:.yalc/react-on-rails-pro", | ||
| "react-on-rails-pro-node-renderer": "file:.yalc/react-on-rails-pro-node-renderer", | ||
| "react-on-rails-rsc": "19.0.3", | ||
| "react-on-rails-rsc": "19.0.5-rc.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-on-rails-rsc npm latest stable version release
💡 Result:
The latest stable release of the react-on-rails-rsc npm package (a.k.a. @shakacode-tools/react-on-rails-rsc) is 19.0.2. [1]
Update react-on-rails-rsc to a stable release.
react-on-rails-rsc is pinned to 19.0.5-rc.1, a pre-release version. The latest stable release is 19.0.2. RC versions may contain bugs or breaking changes before the final release; use a stable version instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 39, Update the dependency version for
react-on-rails-rsc: change the version string for "react-on-rails-rsc" in
package.json from "19.0.5-rc.1" to the stable "19.0.2", then run your package
manager (npm install or yarn install) to update the lockfile and verify by
running the test/build commands; ensure no other code relies on RC-only APIs and
adjust imports/usages if tests flag incompatibilities in components that
reference react-on-rails-rsc.
This reverts commit 1f56465.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
measure-vitals.mjs) for automated Core Web Vitals measurement with throttling supportreact-on-rails-rscfrom 19.0.3 to 19.0.5-rc.1What's in the demo
Routes
/product/ssr— Server-Side Rendered (V1): all data fetched server-side, full HTML sent, then hydrated client-side with all JS/product/client— Client-Side (V2): minimal HTML shell, data fetched via API after hydration, loadable components/product/rsc— React Server Components (V3): server components stream HTML progressively, only interactive components ship JS to the clientKey architectural differences
markdown-libs(246KB gzipped / 1091KB decoded) to the client for rendering product descriptionsRSC Performance Improvements
Normal conditions (no throttling)
Throttled conditions (4x CPU slowdown + Slow 3G)
Test plan
/product/ssr,/product/client,/product/rscand verify all three render correctlypnpm vitals -- --pages product-ssr,product-client,product-rscto reproduce benchmarks--throttleflag to verify throttled performance🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores