Fix LCP/Lighthouse performance scores#118
Merged
damianlegawiec merged 29 commits intoApr 10, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughEnabled React Compiler optimizations, extracted home sections into async components with Suspense, introduced cached server-side product listing, switched several image elements to explicit fetchPriority/loading, added dynamic imports for UI modules, and updated navigation, CSS, and test image mock behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ServerComponent as FeaturedProducts (Server)
participant Auth as AccessTokenService
participant Cache as RemoteCacheLayer
participant SpreeAPI as Spree API
Browser->>ServerComponent: Request page (SSR/SSG)
ServerComponent->>Auth: getAccessToken()
Auth-->>ServerComponent: userToken
ServerComponent->>Cache: cachedListProducts(params, locale, country, _userToken)
Cache->>SpreeAPI: fetch products (cache miss or validate)
SpreeAPI-->>Cache: product payload
Cache-->>ServerComponent: cached product payload
ServerComponent-->>Browser: render page with products and dynamic carousel placeholder
Browser->>Browser: load dynamic ProductCarousel bundle (client)
Browser-->>ProductCarousel: hydrate interactive carousel with server-provided products
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 |
Fix LCP (2.9s -> expected ~1s) by adding priority loading to first visible product images in carousels and grids. Add preconnect hint for cdn.vendo.dev, font display swap, browserslist for modern browsers to reduce polyfills (~13.6 KiB), and fix footer contrast for accessibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fetch products server-side in the homepage and pass them as props to ProductCarousel, so product image URLs are in the initial HTML and discoverable by the browser preload scanner. This eliminates the client-side fetch waterfall (HTML → JS → hydration → useEffect → API → render) that caused ~2.9s LCP resource load delay. Also removes unused useCarouselProducts hook and reverts ineffective browserslist config (Next.js SWC ignores package.json browserslist). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add CSS rules matching Swiper breakpoints and spaceBetween so slides have correct widths and margins before JS hydrates. This eliminates layout shift caused by Swiper switching from default 100%-width slides to the configured 1/2/3/4 per view layout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Next.js 16 deprecates the `priority` prop on Image in favor of the standard HTML `fetchPriority` attribute. Replace all usages across ProductImage, ProductCard, ProductCarousel, ProductGrid, Header, MediaGallery, and checkout layout. ProductImage now explicitly sets loading="eager" when fetchPriority="high" to match prior behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add "use cache: remote" with cacheLife("minutes") and
cacheTag("products") to getProducts(), matching the existing pattern
used by getCategories(). Without caching, every page load made a fresh
Spree API call (~500ms), which delayed SSR and caused 681ms resource
load delay on the LCP image. Cached responses eliminate this bottleneck
on subsequent requests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move hero section and featured products section into client components using useTranslations and useParams instead of server-side getTranslations + await params. This removes all async operations from the page component, allowing faster PPR static shell rendering. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move hero section into HeroSection "use client" component using useTranslations + useParams (synchronous, no await). Move featured products section into FeaturedProductsSection async server component. Simplifies page.tsx and separates concerns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cichorek
force-pushed
the
feature/v-3389-storefront-fix-lcplightspeed-scores
branch
from
ebca08c to
9c0df28
Compare
Product cache: add userToken to cache key so authenticated users get per-user cache (B2B pricing, loyalty tiers). Guest users share a common cache entry (userToken=undefined). Switching accounts invalidates cache via different JWT. Hero: convert HeroSection from client component (useTranslations + useParams) back to async Server Component (getTranslations). Hero text is now pure SSR HTML that paints without waiting for React hydration. Code review fixes: - LazyMobileMenu: add loading placeholder matching button size - Storefront layout: add comment on footer Suspense without fallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cichorek
force-pushed
the
feature/v-3389-storefront-fix-lcplightspeed-scores
branch
from
9c0df28 to
3a86e6d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
src/components/home/HeroSection.tsx (1)
6-15: Tighten locale typing and remove the cast; add explicit return type.
localecan be typed asLocalein props so the cast is unnecessary, and the exported async component should declare a return type.♻️ Suggested patch
+import type React from "react"; import Link from "next/link"; import { getTranslations } from "next-intl/server"; import { Button } from "@/components/ui/button"; import { getStoreName } from "@/lib/store"; interface HeroSectionProps { basePath: string; - locale: string; + locale: Locale; } -export async function HeroSection({ basePath, locale }: HeroSectionProps) { +export async function HeroSection({ + basePath, + locale, +}: HeroSectionProps): Promise<React.ReactElement> { const t = await getTranslations({ - locale: locale as Locale, + locale, namespace: "home", });As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions, use 'satisfies' for type checking object literals, and avoid 'any' (use 'unknown' instead)."
Also applies to: 11-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/home/HeroSection.tsx` around lines 6 - 15, Update the HeroSectionProps and function signature so locale is strongly typed and the component has an explicit return type: change HeroSectionProps.locale from string to Locale (so you can remove the cast "as Locale" on the getTranslations call) and declare the exported async function HeroSection to return Promise<JSX.Element> (or Promise<React.ReactElement>), ensuring you import Locale if not already; then update any usages inside HeroSection that relied on the previous string type accordingly.src/components/products/LazyProductCarousel.tsx (1)
28-33: Add an explicit return type to the exported component.Please annotate the return type for
LazyProductCarouselto satisfy strict TS conventions.♻️ Suggested patch
+import type { ReactElement } from "react"; ... export function LazyProductCarousel({ products, basePath, -}: LazyProductCarouselProps) { +}: LazyProductCarouselProps): ReactElement { return <ProductCarousel products={products} basePath={basePath} />; }As per coding guidelines, "Use strict TypeScript type checking. Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/LazyProductCarousel.tsx` around lines 28 - 33, The exported function LazyProductCarousel is missing an explicit return type; update its signature to declare a JSX return type (e.g., LazyProductCarousel(props: LazyProductCarouselProps): JSX.Element) so it satisfies strict TS rules, ensure the file imports React types if necessary, and keep the body returning <ProductCarousel products={products} basePath={basePath} /> unchanged.src/components/layout/SearchToggle.tsx (1)
10-14: Consider adding a loading fallback for the dynamically imported SearchBar.The
SearchBaris lazily loaded without aloadingfallback. While the component is hidden untilsearchOpenis true (minimizing visual impact), users may briefly see an empty search area on slow connections when they first open search.♻️ Optional: Add skeleton fallback
const SearchBar = dynamic(() => import("@/components/search/SearchBar").then((mod) => ({ default: mod.SearchBar, })), + { + loading: () => ( + <div className="h-10 bg-gray-100 rounded-lg animate-pulse" /> + ), + }, );As per coding guidelines: "Use dynamic() import with loading fallback for heavy components like ProductReviews to enable code splitting."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/SearchToggle.tsx` around lines 10 - 14, The dynamic import for SearchBar lacks a loading fallback which can cause a blank area on slow connections; update the SearchBar dynamic(...) call to include a loading option that returns a lightweight fallback (for example a simple skeleton or spinner component) so users see a placeholder while the heavy SearchBar loads; reference the SearchBar variable and the dynamic(...) invocation and create or reuse a small SearchBarSkeleton (or LoadingSpinner) component and pass it as loading: () => <SearchBarSkeleton /> in the dynamic options.src/components/layout/Header.tsx (1)
36-45: Verify the logo image styling doesn't cause layout shift.Setting
style={{ width: "auto", height: "auto" }}while specifyingwidth={90}andheight={32}may cause the image to render at its intrinsic size rather than the specified dimensions. This could lead to CLS if the actual image dimensions differ from 90×32.Consider using explicit dimensions or
aspect-ratioto reserve space:♻️ Option: Use explicit sizing to prevent CLS
<Image src="/spree.png" alt={storeName} width={90} height={32} - className="max-w-full object-contain" - style={{ width: "auto", height: "auto" }} + className="object-contain" + style={{ width: 90, height: 32 }} fetchPriority="high" loading="eager" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/Header.tsx` around lines 36 - 45, The Image rendering for the logo (the Image component that uses src "/spree.png" and alt={storeName}) risks CLS because inline style sets width/height to "auto" while width={90} and height={32} are provided; remove the conflicting style and instead reserve the exact space by either removing style={{ width: "auto", height: "auto" }} and relying on width={90} height={32}, or replace it with explicit sizing (e.g., set style or className to width:90px and height:32px or an aspect-ratio that matches 90/32) so the Image component always reserves the intended layout space.src/components/products/FeaturedProducts.tsx (1)
10-26: Clean async server component implementation.The component correctly:
- Uses
cachedListProductsfor server-side caching- Handles potential null/undefined with
?? []- Delegates carousel rendering to the lazy-loaded client component
One minor improvement for strict TypeScript compliance:
💡 Optional: Add explicit return type
export async function FeaturedProducts({ basePath, locale, country, -}: FeaturedProductsProps) { +}: FeaturedProductsProps): Promise<React.JSX.Element> {As per coding guidelines: "Always define explicit return types for functions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/FeaturedProducts.tsx` around lines 10 - 26, Add an explicit return type to the async server component FeaturedProducts to satisfy TypeScript strict rules: update the function signature for FeaturedProducts to return Promise<JSX.Element> (or Promise<React.ReactElement>) and ensure any necessary React types are imported; this affects the FeaturedProducts declaration that calls cachedListProducts and returns the LazyProductCarousel component so the signature clearly indicates an async component returning JSX.src/components/home/FeaturedProductsSection.tsx (1)
8-16: Consider matching skeleton count to visible items.
CarouselSkeletonalways renders 4 skeletons, but on mobile only 1 is visible. This is a minor optimization opportunity, though the impact is negligible since skeletons are lightweight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/home/FeaturedProductsSection.tsx` around lines 8 - 16, CarouselSkeleton currently renders a fixed 4 placeholders which over-renders on smaller viewports; update CarouselSkeleton to accept a visibleCount prop (or compute visibleCount from window.innerWidth / media query in a useEffect) and use that value to render ProductCardSkeleton instances instead of the hard-coded 4, e.g., replace the Array(4) mapping with Array(visibleCount) and pass a sensible default (1 or 4) so the component still works when the prop is not provided.src/components/ui/product-image.tsx (1)
39-41: Consider handlingfetchPriority="auto"explicitly.The conditional
fetchPriority === "high" ? "eager" : "lazy"works for"high"andundefined, but whenfetchPriority="auto"is passed (a valid value perProductCardPropsin context), it will setloading="lazy". This is likely fine, but consider whether"auto"should defer to the browser's default loading behavior instead:💡 Optional: More nuanced loading behavior
- loading={fetchPriority === "high" ? "eager" : "lazy"} + loading={fetchPriority === "high" ? "eager" : fetchPriority === "auto" ? undefined : "lazy"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/product-image.tsx` around lines 39 - 41, The current ternary sets loading to "lazy" for any non-"high" fetchPriority, which causes fetchPriority="auto" to be treated as "lazy"; update the ProductImage component to handle fetchPriority explicitly: if fetchPriority === "high" set loading="eager", if fetchPriority === "auto" set loading="auto" (or omit the loading prop to let the browser default), otherwise set loading="lazy"; locate the fetchPriority prop usage and the loading attribute in product-image.tsx and change the conditional accordingly.src/components/layout/StorefrontShell.tsx (2)
33-40: Consider logging errors for debugging.The catch block silently swallows errors, which could make debugging production issues difficult. Adding error logging would help identify category fetch failures.
♻️ Suggested refactor with error logging
-async function fetchCategories() { - return getCategories({ +async function fetchCategories(): Promise<Category[]> { + try { + const res = await getCategories({ depth_eq: 0, expand: ["children.children"], - }) - .then((res) => res.data) - .catch(() => [] as Category[]); + }); + return res.data; + } catch (error) { + console.error("Failed to fetch categories:", error); + return []; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/StorefrontShell.tsx` around lines 33 - 40, The fetchCategories function currently swallows errors in its catch; update the catch on getCategories (used in fetchCategories) to log the error details (e.g., via console.error or the app's logger) along with context before returning the fallback ([] as Category[]). Ensure the log references the function name (fetchCategories) and the caught error so failures to load categories can be diagnosed while preserving the existing return behavior.
6-25: Consider using Next.jsLinkfor internal navigation.The
CategoryLinkscomponent uses plain<a>tags. While this is inside ansr-onlynav primarily for SEO/accessibility, using Next.jsLinkwould still provide prefetching benefits and proper client-side navigation if a screen reader user activates these links.♻️ Suggested refactor
+import Link from "next/link"; import type { Category } from "@spree/sdk"; import { Footer } from "@/components/layout/Footer"; import { Header } from "@/components/layout/Header"; import { getCategories } from "@/lib/data/categories"; function CategoryLinks({ categories, basePath, }: { categories: Category[]; basePath: string; -}) { +}): React.ReactElement { return ( <ul> {categories.map((category) => ( <li key={category.id}> - <a href={`${basePath}/c/${category.permalink}`}>{category.name}</a> + <Link href={`${basePath}/c/${category.permalink}`}>{category.name}</Link> {category.children && category.children.length > 0 && ( <CategoryLinks categories={category.children} basePath={basePath} /> )} </li> ))} </ul> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/StorefrontShell.tsx` around lines 6 - 25, Replace the plain anchor tags in CategoryLinks with Next.js Link to enable client-side navigation and prefetching: import Link from 'next/link', change the <a href={`${basePath}/c/${category.permalink}`}>{category.name}</a> usage to <Link href={`${basePath}/c/${category.permalink}`}><a>{category.name}</a></Link> (or the supported Link children pattern for your Next.js version), preserving the existing key on the <li>, the recursive CategoryLinks call, and any accessibility attributes.
🤖 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/home/FeaturedProductsSection.tsx`:
- Around line 29-32: The code unsafely casts the URL param locale to Locale when
calling getTranslations; add a runtime guard to ensure locale is one of the
allowed values before casting. In FeaturedProductsSection (or wherever locale is
read) define the allowedLocales set (matching the Locale union), check
includes(locale) and assign a safeLocale fallback (e.g., "en") if check fails,
then call getTranslations({ locale: safeLocale as Locale, namespace: "home" });
this protects getTranslations and removes the unchecked locale as Locale
assertion.
In `@src/components/layout/Footer.tsx`:
- Around line 119-123: The external Link in the Footer component opens with
target="_blank" but lacks the rel attribute; update the Link element in Footer
(component Footer / JSX Link instance) to include rel="noopener noreferrer"
alongside target="_blank" to prevent tabnabbing and ensure safe external
linking, i.e., add the rel prop to the same Link that currently has
href="https://spreecommerce.org" and className="...".
In `@src/components/layout/StorefrontShell.tsx`:
- Around line 42-63: The StorefrontHeader function casts locale as Locale but
Locale is not imported; add "import { Locale } from 'next-intl'" to
src/components/layout/StorefrontShell.tsx and likewise add the same import to
the Header component file (src/components/layout/Header.tsx) where Header's
props reference Locale so TypeScript recognizes the type; update imports and
ensure both files use Locale from next-intl rather than any local config.
In `@src/lib/data/products.ts`:
- Around line 17-26: The PR's cachedListProducts function is set up for remote
caching but your app config must enable it; add cacheComponents: true to your
Next config (next.config.ts) so remote-cached server functions like
cachedListProducts will actually use remote caching, and ensure any
request-specific data (e.g., cookies or headers) is passed into
cachedListProducts as parameters (do not call cookies()/headers() inside the
function) to maintain correct per-user cache segmentation.
---
Nitpick comments:
In `@src/components/home/FeaturedProductsSection.tsx`:
- Around line 8-16: CarouselSkeleton currently renders a fixed 4 placeholders
which over-renders on smaller viewports; update CarouselSkeleton to accept a
visibleCount prop (or compute visibleCount from window.innerWidth / media query
in a useEffect) and use that value to render ProductCardSkeleton instances
instead of the hard-coded 4, e.g., replace the Array(4) mapping with
Array(visibleCount) and pass a sensible default (1 or 4) so the component still
works when the prop is not provided.
In `@src/components/home/HeroSection.tsx`:
- Around line 6-15: Update the HeroSectionProps and function signature so locale
is strongly typed and the component has an explicit return type: change
HeroSectionProps.locale from string to Locale (so you can remove the cast "as
Locale" on the getTranslations call) and declare the exported async function
HeroSection to return Promise<JSX.Element> (or Promise<React.ReactElement>),
ensuring you import Locale if not already; then update any usages inside
HeroSection that relied on the previous string type accordingly.
In `@src/components/layout/Header.tsx`:
- Around line 36-45: The Image rendering for the logo (the Image component that
uses src "/spree.png" and alt={storeName}) risks CLS because inline style sets
width/height to "auto" while width={90} and height={32} are provided; remove the
conflicting style and instead reserve the exact space by either removing
style={{ width: "auto", height: "auto" }} and relying on width={90} height={32},
or replace it with explicit sizing (e.g., set style or className to width:90px
and height:32px or an aspect-ratio that matches 90/32) so the Image component
always reserves the intended layout space.
In `@src/components/layout/SearchToggle.tsx`:
- Around line 10-14: The dynamic import for SearchBar lacks a loading fallback
which can cause a blank area on slow connections; update the SearchBar
dynamic(...) call to include a loading option that returns a lightweight
fallback (for example a simple skeleton or spinner component) so users see a
placeholder while the heavy SearchBar loads; reference the SearchBar variable
and the dynamic(...) invocation and create or reuse a small SearchBarSkeleton
(or LoadingSpinner) component and pass it as loading: () => <SearchBarSkeleton
/> in the dynamic options.
In `@src/components/layout/StorefrontShell.tsx`:
- Around line 33-40: The fetchCategories function currently swallows errors in
its catch; update the catch on getCategories (used in fetchCategories) to log
the error details (e.g., via console.error or the app's logger) along with
context before returning the fallback ([] as Category[]). Ensure the log
references the function name (fetchCategories) and the caught error so failures
to load categories can be diagnosed while preserving the existing return
behavior.
- Around line 6-25: Replace the plain anchor tags in CategoryLinks with Next.js
Link to enable client-side navigation and prefetching: import Link from
'next/link', change the <a
href={`${basePath}/c/${category.permalink}`}>{category.name}</a> usage to <Link
href={`${basePath}/c/${category.permalink}`}><a>{category.name}</a></Link> (or
the supported Link children pattern for your Next.js version), preserving the
existing key on the <li>, the recursive CategoryLinks call, and any
accessibility attributes.
In `@src/components/products/FeaturedProducts.tsx`:
- Around line 10-26: Add an explicit return type to the async server component
FeaturedProducts to satisfy TypeScript strict rules: update the function
signature for FeaturedProducts to return Promise<JSX.Element> (or
Promise<React.ReactElement>) and ensure any necessary React types are imported;
this affects the FeaturedProducts declaration that calls cachedListProducts and
returns the LazyProductCarousel component so the signature clearly indicates an
async component returning JSX.
In `@src/components/products/LazyProductCarousel.tsx`:
- Around line 28-33: The exported function LazyProductCarousel is missing an
explicit return type; update its signature to declare a JSX return type (e.g.,
LazyProductCarousel(props: LazyProductCarouselProps): JSX.Element) so it
satisfies strict TS rules, ensure the file imports React types if necessary, and
keep the body returning <ProductCarousel products={products} basePath={basePath}
/> unchanged.
In `@src/components/ui/product-image.tsx`:
- Around line 39-41: The current ternary sets loading to "lazy" for any
non-"high" fetchPriority, which causes fetchPriority="auto" to be treated as
"lazy"; update the ProductImage component to handle fetchPriority explicitly: if
fetchPriority === "high" set loading="eager", if fetchPriority === "auto" set
loading="auto" (or omit the loading prop to let the browser default), otherwise
set loading="lazy"; locate the fetchPriority prop usage and the loading
attribute in product-image.tsx and change the conditional accordingly.
🪄 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: 83a8fa56-9be1-4101-8437-17089e36ac9b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
next.config.tspackage.jsonsrc/__tests__/setup.tsxsrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(storefront)/layout.tsxsrc/app/[country]/[locale]/(storefront)/page.tsxsrc/app/globals.csssrc/app/layout.tsxsrc/components/home/FeaturedProductsSection.tsxsrc/components/home/HeroSection.tsxsrc/components/layout/CartButton.tsxsrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/components/layout/LazyCountrySwitcher.tsxsrc/components/layout/LazyMobileMenu.tsxsrc/components/layout/SearchToggle.tsxsrc/components/layout/StorefrontShell.tsxsrc/components/products/FeaturedProducts.tsxsrc/components/products/LazyProductCarousel.tsxsrc/components/products/MediaGallery.tsxsrc/components/products/ProductCard.tsxsrc/components/products/ProductCarousel.tsxsrc/components/products/ProductGrid.tsxsrc/components/products/ProductListingLayout.tsxsrc/components/ui/product-image.tsxsrc/hooks/useCarouselProducts.tssrc/lib/data/products.ts
💤 Files with no reviewable changes (1)
- src/hooks/useCarouselProducts.ts
- Footer: add rel="noopener noreferrer" on external Spree Commerce link
- SearchToggle: add loading skeleton for dynamically imported SearchBar
- StorefrontShell: log category fetch errors instead of silent swallow,
replace <a> with <Link> for client-side navigation in CategoryLinks
- ProductImage: use loading={undefined} instead of "lazy" when
fetchPriority is not "high", letting Next.js Image decide the default
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/layout/StorefrontShell.tsx (1)
30-33:⚠️ Potential issue | 🔴 CriticalImport
Localeand typelocaledirectly to prevent TS build failure.
Localeis referenced on Line 60 and Line 82 but not imported in this file. Also,localeis currently typed asstringand then cast, which is avoidable.🐛 Proposed fix
import type { Category } from "@spree/sdk"; +import type { Locale } from "next-intl"; import Link from "next/link"; import { Footer } from "@/components/layout/Footer"; import { Header } from "@/components/layout/Header"; import { getCategories } from "@/lib/data/categories"; @@ interface ShellProps { basePath: string; - locale: string; + locale: Locale; country: string; } @@ <Header rootCategories={rootCategories} basePath={basePath} - locale={locale as Locale} + locale={locale} /> @@ <Footer rootCategories={rootCategories} basePath={basePath} - locale={locale as Locale} + locale={locale} />#!/bin/bash # Verify unresolved Locale usage pattern in this file cat -n src/components/layout/StorefrontShell.tsx | sed -n '1,120p' rg -n 'import type \{ Locale \} from "next-intl"' src/components/layout/StorefrontShell.tsx rg -n 'as Locale|locale=\{locale as Locale\}' src/components/layout/StorefrontShell.tsxAlso applies to: 60-61, 82-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/StorefrontShell.tsx` around lines 30 - 33, The file is missing an import for Locale and currently types locale as string then casts it; import type { Locale } from "next-intl" and update the ShellProps interface to use locale: Locale instead of string (the interface name is ShellProps and the prop is locale), then remove the unnecessary casts (e.g., occurrences of locale as Locale) so the component can use locale directly; ensure the import and type change are applied in StorefrontShell.tsx wherever locale is referenced (including the places that previously cast locale).
🧹 Nitpick comments (1)
src/components/layout/StorefrontShell.tsx (1)
7-13: Add explicit return types for all functions in this file.
CategoryLinks,fetchCategories,StorefrontHeader, andStorefrontFooterare missing explicit return type annotations. Add:
CategoryLinks:JSX.ElementfetchCategories:Promise<Category[]>StorefrontHeader:Promise<JSX.Element>StorefrontFooter:Promise<JSX.Element>♻️ Proposed refactor
function CategoryLinks({ categories, basePath, }: { categories: Category[]; basePath: string; -}) { +}): JSX.Element { @@ -async function fetchCategories() { +async function fetchCategories(): Promise<Category[]> { @@ export async function StorefrontHeader({ basePath, locale, country: _country, -}: ShellProps) { +}: ShellProps): Promise<JSX.Element> { @@ export async function StorefrontFooter({ basePath, locale, country: _country, -}: ShellProps) { +}: ShellProps): Promise<JSX.Element> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/layout/StorefrontShell.tsx` around lines 7 - 13, Add explicit return type annotations to the functions named in this file: annotate CategoryLinks with : JSX.Element, annotate fetchCategories with : Promise<Category[]>, and annotate both StorefrontHeader and StorefrontFooter with : Promise<JSX.Element>; update their function signatures (e.g., function CategoryLinks(...) : JSX.Element { ... }, async function fetchCategories(...) : Promise<Category[]> { ... }, async function StorefrontHeader(...) : Promise<JSX.Element> { ... }, async function StorefrontFooter(...) : Promise<JSX.Element> { ... }) and ensure any exported/used signatures match these types so the TS compiler recognizes the explicit returns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/layout/StorefrontShell.tsx`:
- Around line 30-33: The file is missing an import for Locale and currently
types locale as string then casts it; import type { Locale } from "next-intl"
and update the ShellProps interface to use locale: Locale instead of string (the
interface name is ShellProps and the prop is locale), then remove the
unnecessary casts (e.g., occurrences of locale as Locale) so the component can
use locale directly; ensure the import and type change are applied in
StorefrontShell.tsx wherever locale is referenced (including the places that
previously cast locale).
---
Nitpick comments:
In `@src/components/layout/StorefrontShell.tsx`:
- Around line 7-13: Add explicit return type annotations to the functions named
in this file: annotate CategoryLinks with : JSX.Element, annotate
fetchCategories with : Promise<Category[]>, and annotate both StorefrontHeader
and StorefrontFooter with : Promise<JSX.Element>; update their function
signatures (e.g., function CategoryLinks(...) : JSX.Element { ... }, async
function fetchCategories(...) : Promise<Category[]> { ... }, async function
StorefrontHeader(...) : Promise<JSX.Element> { ... }, async function
StorefrontFooter(...) : Promise<JSX.Element> { ... }) and ensure any
exported/used signatures match these types so the TS compiler recognizes the
explicit returns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b86ff32-5175-423b-a0e8-ba0654f5020d
📒 Files selected for processing (4)
src/components/layout/Footer.tsxsrc/components/layout/SearchToggle.tsxsrc/components/layout/StorefrontShell.tsxsrc/components/ui/product-image.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/layout/Footer.tsx
- src/components/layout/SearchToggle.tsx
- src/components/ui/product-image.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/app/layout.tsx`:
- Around line 12-14: The current module-level expression that computes
spreeApiOrigin using new URL(process.env.SPREE_API_URL) may throw for malformed
or empty env values and crash the app at module load; update the logic around
spreeApiOrigin to safely validate and parse process.env.SPREE_API_URL (e.g.,
check truthiness, wrap URL parsing in a try/catch or use a safe URL parse
helper) so that on parse failure spreeApiOrigin becomes undefined (or falls back
to a sane default) instead of throwing; change the code where spreeApiOrigin is
computed (reference symbol spreeApiOrigin and the use of new
URL(process.env.SPREE_API_URL)) to perform this guarded parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Replace hardcoded cdn.vendo.dev preconnect with dynamic origin derived from SPREE_API_URL env variable. Preconnect is omitted when the env var is not set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cichorek
force-pushed
the
feature/v-3389-storefront-fix-lcplightspeed-scores
branch
from
00d8e7c to
0456618
Compare
Move dynamic imports directly into Header and FeaturedProducts instead of separate LazyMobileMenu, LazyCountrySwitcher, LazyProductCarousel wrapper files. Remove StorefrontShell and restore direct category fetching in storefront layout with error logging and Link components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
getProducts should remain a simple, direct call without caching indirection. Caching is only needed for featured products on the homepage. Also adds the missing userToken to cachedListProducts for per-user cache segmentation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ront-fix-lcplightspeed-scores # Conflicts: # src/app/[country]/[locale]/(storefront)/page.tsx
Update HeroSection with demo CTAs (Fork on GitHub, Quickstart Guide) and style tweaks (text-lg/gray-600, flex-wrap) from main branch merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
damianlegawiec
approved these changes
Apr 10, 2026
This was referenced Apr 10, 2026
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
Comprehensive Lighthouse performance and accessibility optimization for the Next.js Spree storefront.
Performance
useEffectto async Server Component with Suspense streaming, so product image URLs are in the initial HTML"use cache: remote"withcacheLife("minutes")to product list. Cache key includes locale, country, and user token for per-user segmentation (B2B pricing, loyalty tiers)cookies()callspaceBetweenin CSS so slides have correct widths/margins before JS hydrates, eliminating CLSmin-h-[823px]pushes carousel below Lighthouse mobile viewport fold, making hero text the LCP elementcontent-visibility: autoon featured products section (mobile only) to skip below-fold renderingpriority: Migrate allnext/imageusages from deprecatedprioritytofetchPriority="high"+loading="eager"<link rel="preconnect">fromSPREE_API_URLenv with safe URL parsingdisplay: "swap"to Geist font configCode Quality
console.errorinstead of silent swallowrel="noopener noreferrer"on external footer link<a>with<Link>in category navigation for client-side routingstyle={{ width: "auto", height: "auto" }}), explicitloadinghandling in ProductImagemountedguard prevents hydration mismatch from Suspense timingHeroSection(server) andFeaturedProductsSection(server) in separate filesAccessibility
text-neutral-500→text-neutral-400underlinefor non-color distinguishabilityCleanup
useCarouselProductshookLazyProductCarousel,LazyMobileMenu,LazyCountrySwitcher,StorefrontShellwrapper filesLighthouse results
Desktop (best)
Mobile (best)
Test plan
<head>🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Performance Enhancements
Bug Fixes
Style