PLP performance: narrow fields, derive hasMore, drop dead memo - #123
Conversation
Three Vercel React best-practices fixes to the product listing: 1. Pass a fields list to the listing fetches (`PRODUCT_CARD_FIELDS`) so Spree returns only the columns ProductCard reads. Shrinks the cached entry, the RSC→client serialization, and the streaming HTML size. Applied to both ProductListing (category/search PLPs) and FeaturedProducts (homepage carousel). Same pattern SearchBar already uses for quick-search suggestions. 2. Derive `hasMore` in InfiniteProductList from `currentPage < knownPages` rather than mirroring it in its own useState. Introduce a separate `hasError` flag so the "nothing left to load" and "fetch failed" signals no longer share state, per the "don't mirror derivable values into state" rule. 3. Drop the manual useMemo wrapping the product grid JSX. reactCompiler is enabled in next.config, so the compiler handles this automatically — the manual memo was redundant.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 31 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR optimizes product data fetching by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/products/ProductListing.tsx (1)
100-105: PreventbaseParamsfrom overriding the narrowedfieldsset.Line 104 can override Line 102 when
baseParams.fieldsis present, which weakens the “always narrow to card fields” optimization.Suggested diff
const listParams: ProductListParams = { limit: PAGE_SIZE, - fields: PRODUCT_CARD_FIELDS, ...queryParams, ...baseParams, + fields: PRODUCT_CARD_FIELDS, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductListing.tsx` around lines 100 - 105, listParams currently spreads baseParams after setting fields, so baseParams.fields can override the enforced PRODUCT_CARD_FIELDS; update the construction of listParams (ProductListParams) so PRODUCT_CARD_FIELDS is applied last (or explicitly assign fields = PRODUCT_CARD_FIELDS after spreading queryParams and baseParams) ensuring PAGE_SIZE and other params remain, but fields cannot be overridden by baseParams or queryParams.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/products/InfiniteProductList.tsx`:
- Around line 66-67: The current hasMore definition (hasMore = !hasError &&
currentPage < knownPages) causes the UI to think there are no more products on
fetch failure; change the logic so hasMore only reflects pagination (e.g.,
hasMore = currentPage < knownPages) and keep error state separate, then update
the noMoreProducts rendering to only show when there is truly no more data
(e.g., !hasError && !isLoading && !hasMore). Locate the hasMore variable and the
noMoreProducts render in the InfiniteProductList component and adjust them so
errors do not trigger the "no more products" message.
In `@src/lib/data/cached.ts`:
- Around line 23-32: PRODUCT_CARD_FIELDS currently omits product.categories
which prevents GTM mapping from populating GA4 item_category; update the
PRODUCT_CARD_FIELDS array in cached.ts (the exported constant
PRODUCT_CARD_FIELDS) to include "categories" so product objects returned by
cached data include categories and analytics/gtm.ts can read item_category as
before.
---
Nitpick comments:
In `@src/components/products/ProductListing.tsx`:
- Around line 100-105: listParams currently spreads baseParams after setting
fields, so baseParams.fields can override the enforced PRODUCT_CARD_FIELDS;
update the construction of listParams (ProductListParams) so PRODUCT_CARD_FIELDS
is applied last (or explicitly assign fields = PRODUCT_CARD_FIELDS after
spreading queryParams and baseParams) ensuring PAGE_SIZE and other params
remain, but fields cannot be overridden by baseParams or queryParams.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77dca992-1fb7-4e2a-991d-03f28482a33c
📒 Files selected for processing (4)
src/components/products/FeaturedProducts.tsxsrc/components/products/InfiniteProductList.tsxsrc/components/products/ProductListing.tsxsrc/lib/data/cached.ts
- InfiniteProductList: decouple hasMore from hasError so fetch failures no longer show "no more products to load". hasMore is now pure pagination state (currentPage < knownPages); the render gate for the exhausted message checks !hasError && !hasMore. - PRODUCT_CARD_FIELDS: include `categories` so mapProductToGA4Item can continue to populate GA4 item_category on view_item_list and select_item events. Without it, analytics silently lost category attribution for every listing interaction. - ProductListing: move `fields: PRODUCT_CARD_FIELDS` to the end of the listParams spread so baseParams / queryParams can't accidentally override the narrowed set.
1 participant
Three Vercel React best-practices fixes to the product listing:
Pass a fields list to the listing fetches (
PRODUCT_CARD_FIELDS) so Spree returns only the columns ProductCard reads. Shrinks the cached entry, the RSC→client serialization, and the streaming HTML size. Applied to both ProductListing (category/search PLPs) and FeaturedProducts (homepage carousel). Same pattern SearchBar already uses for quick-search suggestions.Derive
hasMorein InfiniteProductList fromcurrentPage < knownPagesrather than mirroring it in its own useState. Introduce a separatehasErrorflag so the "nothing left to load" and "fetch failed" signals no longer share state, per the "don't mirror derivable values into state" rule.Drop the manual useMemo wrapping the product grid JSX. reactCompiler is enabled in next.config, so the compiler handles this automatically — the manual memo was redundant.
Summary by CodeRabbit
Bug Fixes
Refactor