Add Swiper carousel for products display#26
Conversation
|
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:
WalkthroughRemoved the FeaturedProducts component and replaced it with a new ProductCarousel (uses Swiper), added useCarouselProducts hook for product fetching, updated the storefront page to render ProductCarousel, added CSS to hide default Swiper nav buttons, and added the "swiper" dependency. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Home Page (page.tsx)
participant Carousel as ProductCarousel
participant Hook as useCarouselProducts
participant Store as StoreContext
participant API as getProducts / getTaxonProducts
participant UI as Swiper + ProductCard
Page->>Carousel: render(basePath, taxonId?, limit?)
Carousel->>Hook: call useCarouselProducts(taxonId, limit)
Hook->>Store: read currency, locale, storeLoading
Hook->>API: fetch products (currency, locale, taxonId?, limit)
API-->>Hook: products[] / error
Hook-->>Carousel: products[], loading, error
Carousel->>UI: initialize Swiper with products, set nav refs/state
UI-->>Page: interactive carousel rendered
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 (2)
src/components/products/ProductCarousel.tsx (2)
29-61:useEffectfor data fetching triggered by context state violates the project's guidelines.The fetch is driven by
currency,locale, andstoreLoadingfromStoreContext— all reactive state changes. This is exactly the pattern the guidelines prohibit.The recommended approach is to lift data-fetching into a Server Component and pass
productsas a prop, using React 19'suse()with a Suspense boundary for loading states. If thecurrency/localefromStoreContextmust remain client-side, the component should at minimum accept them as props so the parent controls when fetching happens rather than triggering side effects inside the component.As per coding guidelines: "Avoid using useEffect for data fetching triggered by state changes; use event handlers or Server Actions instead" and "Use React 19 use() function to unwrap Promises in Server Components alongside Suspense boundaries for better loading states."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCarousel.tsx` around lines 29 - 61, The ProductCarousel component currently performs client-side fetching inside its useEffect (see useEffect, fetchProducts, getProducts, getTaxonProducts) driven by StoreContext values (currency, locale, storeLoading), which violates the guideline; refactor by moving data fetching out of the component: convert the parent into a Server Component that calls getProducts/getTaxonProducts (or a dedicated fetchProducts helper) and resolves the promise with React 19's use() inside a Suspense boundary, then pass the resulting products array plus currency/locale as props into ProductCarousel; if currency/locale must stay client-side, change ProductCarousel to accept products, currency, and locale as props and remove the useEffect/fetchProducts logic so the parent controls when fetching occurs.
20-24: Missing explicit return type on the exported function.As per coding guidelines: "Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type."
-export function ProductCarousel({ +export function ProductCarousel({ taxonId, limit = 8, basePath, -}: ProductCarouselProps) { +}: ProductCarouselProps): JSX.Element {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCarousel.tsx` around lines 20 - 24, The ProductCarousel function lacks an explicit return type; update its declaration to include a precise JSX return type (e.g., change "export function ProductCarousel({ ... }: ProductCarouselProps)" to "export function ProductCarousel({ ... }: ProductCarouselProps): JSX.Element" or ": JSX.Element | null" if the component can return null) so the component signature is strictly typed (do not use any or omit the return type); reference the ProductCarousel function and ProductCarouselProps when making the change.
🤖 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/ProductCarousel.tsx`:
- Around line 87-119: The navigation currently uses global CSS selectors and
Tailwind's disabled: utilities which won't work; change the wiring to use refs
and the Swiper navigation prevEl/nextEl refs instead of ".carousel-button-prev"
/ ".carousel-button-next" selectors (create refs like prevRef/nextRef, attach
them to the two button elements, and pass prevEl/nextEl as those refs in the
Swiper navigation prop, or use Swiper's onBeforeInit to set
navigation.prevEl/nextEl from the refs). Also replace the Tailwind disabled:
utilities on the buttons with the swiper-button-disabled variant (e.g., use
[&.swiper-button-disabled]:opacity-0 and
[&.swiper-button-disabled]:pointer-events-none) or alternatively set the actual
disabled attribute in an onSwiper/onSlideChange callback when Swiper
adds/removes the swiper-button-disabled class; update references to the button
elements in ProductCarousel, Swiper navigation prop, and any onSwiper handlers
accordingly.
- Around line 47-52: Add an error state to ProductCarousel and set it inside the
catch block so network/API failures don't fall through to the empty-products UI:
in the fetch logic (the try/catch/finally around the product fetch in
ProductCarousel.tsx) call setError(error) (or setError(true)/setError(message))
in the catch and only clear it on a successful fetch; keep setLoading(false) in
finally. Then update the component render (where it currently checks
products.length === 0 and renders "No products found") to first check the new
error state and render a distinct error message/UI when error is present before
falling back to the empty-state branch. Ensure you reference the existing state
setters (setLoading, setProducts) and add the new state variable (e.g., error /
setError) used in the render and fetch flow.
---
Nitpick comments:
In `@src/components/products/ProductCarousel.tsx`:
- Around line 29-61: The ProductCarousel component currently performs
client-side fetching inside its useEffect (see useEffect, fetchProducts,
getProducts, getTaxonProducts) driven by StoreContext values (currency, locale,
storeLoading), which violates the guideline; refactor by moving data fetching
out of the component: convert the parent into a Server Component that calls
getProducts/getTaxonProducts (or a dedicated fetchProducts helper) and resolves
the promise with React 19's use() inside a Suspense boundary, then pass the
resulting products array plus currency/locale as props into ProductCarousel; if
currency/locale must stay client-side, change ProductCarousel to accept
products, currency, and locale as props and remove the useEffect/fetchProducts
logic so the parent controls when fetching occurs.
- Around line 20-24: The ProductCarousel function lacks an explicit return type;
update its declaration to include a precise JSX return type (e.g., change
"export function ProductCarousel({ ... }: ProductCarouselProps)" to "export
function ProductCarousel({ ... }: ProductCarouselProps): JSX.Element" or ":
JSX.Element | null" if the component can return null) so the component signature
is strictly typed (do not use any or omit the return type); reference the
ProductCarousel function and ProductCarouselProps when making the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/hooks/useCarouselProducts.ts (1)
26-62: Consider migrating data fetching away fromuseEffectper coding guidelines.The coding guidelines state to avoid
useEffectfor data fetching triggered by state changes and to prefer event handlers or Server Actions. Here, currency/locale/taxonId changes drive re-fetches viauseEffect. Since this is a Next.js app, consider:
- Lifting product fetching to a Server Component (pass products as props to the carousel), or
- Using React 19's
use()+ Suspense if the data must be fetched client-side.Either approach would eliminate the manual
loading/error/cancelledstate management. Given the component is already"use client"with context-dependent params, this is a pragmatic pattern for now, but worth refactoring to align with project conventions. As per coding guidelines: "Avoid using useEffect for data fetching triggered by state changes; use event handlers or Server Actions instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCarouselProducts.ts` around lines 26 - 62, The hook useCarouselProducts uses useEffect to fetch data (fetchProducts calling getProducts/getTaxonProducts) on changes to currency/locale/taxonId/limit which violates the guideline to avoid useEffect-driven fetching; refactor by moving product fetching out of the client hook—either fetch products in a Server Component and pass them into the carousel hook as props, or replace this effect with React 19's use() + Suspense in a client wrapper so you can await getProducts/getTaxonProducts directly; remove the cancelled/loading/error effect logic from useCarouselProducts and instead rely on server-provided props or use() for pending/error states.src/components/products/ProductCarousel.tsx (2)
5-9: Consider dynamically importing Swiper to improve initial bundle size.Swiper is a non-trivial client-side library. Since the carousel is below the fold on the homepage, a dynamic import with a loading fallback would improve code splitting and initial page load. As per coding guidelines: "Use dynamic imports with loading fallback for non-critical components to improve code splitting."
Example approach
import dynamic from "next/dynamic"; const SwiperComponent = dynamic( () => import("swiper/react").then((mod) => mod.Swiper), { ssr: false, loading: () => <SkeletonPlaceholder /> } );Alternatively, you could wrap the entire
ProductCarouselin a dynamic import at the page level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCarousel.tsx` around lines 5 - 9, ProductCarousel currently imports Swiper (SwiperComponent, SwiperSlide, Navigation and CSS) statically; change it to dynamically import the heavy "swiper/react" module for SwiperComponent (and SwiperSlide if needed) using next/dynamic with ssr: false and a loading fallback (e.g., SkeletonPlaceholder) to reduce initial bundle size, while keeping the Navigation module import as-is or loading it client-side; ensure CSS is loaded client-side or moved to a dynamic import strategy so styles apply when the component mounts; update references to SwiperComponent and SwiperSlide inside ProductCarousel to use the dynamically imported components.
47-58: Skeleton count doesn't adapt to breakpoints.The loading skeleton always renders 4 items in a 4-column grid, but on mobile (
slidesPerView: 1) users see 4 stacked skeletons taking up the full page height. Consider matching the skeleton count to the viewport or showing fewer skeletons (e.g., 1–2) since the carousel will only display 1 slide on mobile anyway.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCarousel.tsx` around lines 47 - 58, The loading skeleton in ProductCarousel.tsx always renders 4 placeholders regardless of viewport causing excessive height on mobile; update the loading branch (the code that checks the loading variable inside the ProductCarousel component) to compute a responsive skeletonCount (e.g., derive from the carousel's slidesPerView prop or current breakpoint via a small helper like getSlidesPerViewFromBreakpoints or a useWindowWidth hook) and render [...Array(skeletonCount)].map(...) instead of Array(4), ensuring mobile returns 1–2 skeletons while desktop can keep 3–4.
🤖 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/ProductCarousel.tsx`:
- Around line 79-93: The nav buttons in ProductCarousel (the button elements
using refs prevRef and nextRef and className based on NAV_BUTTON_BASE) are
visually hidden via "opacity-0 pointer-events-none" when isBeginning / isEnd is
true but remain focusable; update those buttons so when isBeginning/isEnd is
true you also set aria-hidden="true" and tabIndex={-1} (or the disabled
attribute) on the respective button to remove them from the accessibility tree
and prevent keyboard focus; apply this change to both the previous and next
buttons, keeping the existing className conditional but adding the accessibility
attributes tied to isBeginning / isEnd.
In `@src/hooks/useCarouselProducts.ts`:
- Around line 26-28: Hoist the local "cancelled" flag and always return a
cleanup from the useEffect in useCarouselProducts (i.e., declare let cancelled =
false before the if (storeLoading) guard), move any fetch/start logic inside the
guard but ensure the effect returns () => { cancelled = true } on every
invocation, and update any fetch logic (e.g., the async fetch function
referenced in the effect) to check the cancelled flag before calling setState or
dispatch so overlapping storeLoading transitions cannot update state from stale
fetches.
---
Nitpick comments:
In `@src/components/products/ProductCarousel.tsx`:
- Around line 5-9: ProductCarousel currently imports Swiper (SwiperComponent,
SwiperSlide, Navigation and CSS) statically; change it to dynamically import the
heavy "swiper/react" module for SwiperComponent (and SwiperSlide if needed)
using next/dynamic with ssr: false and a loading fallback (e.g.,
SkeletonPlaceholder) to reduce initial bundle size, while keeping the Navigation
module import as-is or loading it client-side; ensure CSS is loaded client-side
or moved to a dynamic import strategy so styles apply when the component mounts;
update references to SwiperComponent and SwiperSlide inside ProductCarousel to
use the dynamically imported components.
- Around line 47-58: The loading skeleton in ProductCarousel.tsx always renders
4 placeholders regardless of viewport causing excessive height on mobile; update
the loading branch (the code that checks the loading variable inside the
ProductCarousel component) to compute a responsive skeletonCount (e.g., derive
from the carousel's slidesPerView prop or current breakpoint via a small helper
like getSlidesPerViewFromBreakpoints or a useWindowWidth hook) and render
[...Array(skeletonCount)].map(...) instead of Array(4), ensuring mobile returns
1–2 skeletons while desktop can keep 3–4.
In `@src/hooks/useCarouselProducts.ts`:
- Around line 26-62: The hook useCarouselProducts uses useEffect to fetch data
(fetchProducts calling getProducts/getTaxonProducts) on changes to
currency/locale/taxonId/limit which violates the guideline to avoid
useEffect-driven fetching; refactor by moving product fetching out of the client
hook—either fetch products in a Server Component and pass them into the carousel
hook as props, or replace this effect with React 19's use() + Suspense in a
client wrapper so you can await getProducts/getTaxonProducts directly; remove
the cancelled/loading/error effect logic from useCarouselProducts and instead
rely on server-provided props or use() for pending/error states.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/hooks/useCarouselProducts.ts (1)
26-62:useEffectfor data fetching triggered by state changes violates the coding guidelines.The effect fires whenever
currency,locale,storeLoading,taxonId, orlimitchange — all state/prop-driven triggers. Per the project's guidelines, the preferred pattern is either a Server Action / Server Component with the React 19use()hook and<Suspense>, or passing a pre-resolved Promise from a parent.Concretely,
ProductCarouselcould be restructured as:
- A Server Component that calls
getProducts/getTaxonProductsdirectly (readingcurrency/localefrom cookies/headers server-side).- Returns a
<Suspense>boundary wrapping an inner"use client"component that receivesproductsas a prop and handles the Swiper rendering.This would eliminate the hook entirely and remove the loading/error state management from the client bundle. Given
ProductCarouselmust be a client component for Swiper, the split would look like:// Server Component export async function ProductCarousel({ taxonId, limit, basePath }) { const products = await getProducts({ per_page: limit }, { currency, locale }); return ( <Suspense fallback={<CarouselSkeleton count={limit} />}> <ProductCarouselClient products={products.data} basePath={basePath} /> </Suspense> ); }Based on learnings: "Applies to
**/*.{ts,tsx}: Avoid using useEffect for data fetching triggered by state changes; use event handlers or Server Actions instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCarouselProducts.ts` around lines 26 - 62, The current useEffect in useCarouselProducts triggers client-side data fetching on prop/state changes (currency, locale, storeLoading, taxonId, limit) which violates the guideline; refactor by moving data fetching to a Server Component (e.g., a new ProductCarousel server export) that calls getProducts/getTaxonProducts server-side (reading currency/locale from cookies/headers) and returns a Suspense wrapper that renders a client component (e.g., ProductCarouselClient) with products passed as a prop; remove useCarouselProducts/useEffect and its loading/error state, ensure ProductCarouselClient (the "use client" component) only receives products and basePath and handles Swiper rendering.src/components/products/ProductCarousel.tsx (1)
47-58: Skeleton placeholder count is hardcoded to4, ignoring thelimitprop.When
limitdiffers from 4 (e.g.,limit={6}orlimit={2}), the skeleton layout won't match the eventual carousel content.♻️ Proposed fix
- {[...Array(4)].map((_, i) => ( + {Array.from({ length: Math.min(limit, 4) }).map((_, i) => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/products/ProductCarousel.tsx` around lines 47 - 58, The skeleton placeholder count in the ProductCarousel loading branch is hardcoded to 4; update the JSX inside the loading conditional in ProductCarousel to derive the placeholder array length from the component's limit prop (falling back to a sensible default like 4 if limit is undefined or invalid) so the skeleton matches the eventual number of items rendered; modify the [...Array(4)].map(...) to use Array.from or [...Array(Math.max(1, limit || 4))] (or equivalent) so keys and counts align with limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/products/ProductCarousel.tsx`:
- Around line 47-58: The skeleton placeholder count in the ProductCarousel
loading branch is hardcoded to 4; update the JSX inside the loading conditional
in ProductCarousel to derive the placeholder array length from the component's
limit prop (falling back to a sensible default like 4 if limit is undefined or
invalid) so the skeleton matches the eventual number of items rendered; modify
the [...Array(4)].map(...) to use Array.from or [...Array(Math.max(1, limit ||
4))] (or equivalent) so keys and counts align with limit.
In `@src/hooks/useCarouselProducts.ts`:
- Around line 26-62: The current useEffect in useCarouselProducts triggers
client-side data fetching on prop/state changes (currency, locale, storeLoading,
taxonId, limit) which violates the guideline; refactor by moving data fetching
to a Server Component (e.g., a new ProductCarousel server export) that calls
getProducts/getTaxonProducts server-side (reading currency/locale from
cookies/headers) and returns a Suspense wrapper that renders a client component
(e.g., ProductCarouselClient) with products passed as a prop; remove
useCarouselProducts/useEffect and its loading/error state, ensure
ProductCarouselClient (the "use client" component) only receives products and
basePath and handles Swiper rendering.
- Add disabled attribute to carousel nav buttons when at beginning/end so they are removed from tab order and accessibility tree - Hoist cancelled flag in useCarouselProducts useEffect so cleanup always runs, preventing stale setState from in-flight fetches Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cichorek
force-pushed
the
product-carousel
branch
from
f4a26df to
50092c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/useCarouselProducts.ts (1)
26-62:useEffectfor data fetching driven by context state changes violates the coding guideline.The effect re-fires whenever
currency,locale, orstoreLoadingchange — all are context/state values, not external system events. The guideline requires using event handlers or Server Actions for this scenario rather thanuseEffect.A cleaner architecture would pass
currencyandlocaleas explicit props from the parent server component (both are already derivable from route params and store config) and promote the fetch to a Server Component or a React 19use()-based pattern with a Suspense boundary, eliminating the need for the effect entirely.As per coding guidelines,
**/*.{ts,tsx}: "Avoid using useEffect for data fetching triggered by state changes; use event handlers or Server Actions instead."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCarouselProducts.ts` around lines 26 - 62, The hook useCarouselProducts currently fetches on context-driven changes (currency, locale, storeLoading) inside a useEffect; per guidelines, remove that effect and stop driving fetches from context/state changes—instead accept currency and locale as explicit props to the parent server component (derived from route/store) and move the product fetching logic (the fetchProducts behavior that calls getProducts/getTaxonProducts and sets products/loading/error) into a Server Component or a Server Action / React 19 use() pattern with a Suspense boundary; update useCarouselProducts to be a pure client hook that receives already-fetched products (and loading/error) or exposes event handlers to refetch, and delete the useEffect and cancelled logic from useCarouselProducts.
🤖 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/ProductCarousel.tsx`:
- Around line 35-40: The Swiper navigation is being passed as a boolean which
leaves swiper.params.navigation as a boolean when handleBeforeInit runs, so
prevRef/nextRef never get wired; update the navigation prop on the Swiper
instance to be an explicit object that uses the refs (e.g., navigation={{
prevEl: prevRef.current, nextEl: nextRef.current }} or navigation={{ enabled:
true, prevEl: prevRef.current, nextEl: nextRef.current }}) and keep the
handleBeforeInit callback (handleBeforeInit) to assign prevEl/nextEl as a
fallback; if refs may be null on first render, ensure you also initialize/update
navigation after mount (e.g., in an effect that calls
swiper.navigation.init()/update() when prevRef.current/nextRef.current become
available) so the prevRef and nextRef in ProductCarousel are properly wired to
the Swiper instance.
---
Nitpick comments:
In `@src/hooks/useCarouselProducts.ts`:
- Around line 26-62: The hook useCarouselProducts currently fetches on
context-driven changes (currency, locale, storeLoading) inside a useEffect; per
guidelines, remove that effect and stop driving fetches from context/state
changes—instead accept currency and locale as explicit props to the parent
server component (derived from route/store) and move the product fetching logic
(the fetchProducts behavior that calls getProducts/getTaxonProducts and sets
products/loading/error) into a Server Component or a Server Action / React 19
use() pattern with a Suspense boundary; update useCarouselProducts to be a pure
client hook that receives already-fetched products (and loading/error) or
exposes event handlers to refetch, and delete the useEffect and cancelled logic
from useCarouselProducts.
| const handleBeforeInit = useCallback((swiper: Swiper) => { | ||
| if (typeof swiper.params.navigation === "object") { | ||
| swiper.params.navigation.prevEl = prevRef.current; | ||
| swiper.params.navigation.nextEl = nextRef.current; | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the ProductCarousel.tsx file to see the context
fd -t f ProductCarousel.tsxRepository: spree/nextjs-starter-spree
Length of output: 113
🏁 Script executed:
# Read the ProductCarousel.tsx file to see the actual code and context
cat -n src/components/products/ProductCarousel.tsxRepository: spree/nextjs-starter-spree
Length of output: 4752
🏁 Script executed:
# Check the Swiper version in package.json
cat -n package.json | grep -A 5 -B 5 "swiper"Repository: spree/nextjs-starter-spree
Length of output: 398
🏁 Script executed:
# Check Swiper v12 source to understand navigation prop handling
# Look for the swiper-react wrapper to see how navigation prop is processed
find node_modules/swiper -name "*.js" -o -name "*.ts" | grep -E "(index|swiper-react)" | head -20Repository: spree/nextjs-starter-spree
Length of output: 124
🏁 Script executed:
# Check if there's a lock file that shows Swiper installation
ls -la | grep -E "package-lock|yarn.lock|pnpm-lock"
# Also check TypeScript definitions or documentation files
fd -t f "swiper" --extension d.ts 2>/dev/null | head -10Repository: spree/nextjs-starter-spree
Length of output: 138
🌐 Web query:
Swiper v12 navigation prop beforeInit event timing when navigation={true}
💡 Result:
In Swiper v12, the Navigation module doesn’t initialize on beforeInit.
beforeInitfires right before Swiper initialization. At this point,swiper.navigation.nextEl/prevElare typically not set yet, because Navigation hasn’t run its setup. [1]- The Navigation module hooks into Swiper’s
initevent to resolvenextEl/prevEl, attach listeners, inject icons, and callupdate(). So the navigation elements are wired up duringinit, and are definitely ready byafterInit. [2]
What this means when navigation={true}:
- Use
onBeforeInitonly to mutate params (e.g., set customprevEl/nextElrefs before Navigation’sinitruns). - Use
onInit/onAfterInitif you need to read or manipulate the actual navigation elements (swiper.navigation.nextEl/prevEl) or callswiper.navigation.update().
Sources:
[1] Swiper API – events (beforeInit, init, afterInit) https://swiperjs.com/swiper-api
[2] Swiper Navigation module lifecycle (initializes on init) https://deepwiki.com/nolimits4web/swiper/3.1-navigation-and-pagination
🌐 Web query:
Swiper v12 "params.navigation" boolean "beforeInit" conversion initialization
💡 Result:
In Swiper v12, params.navigation can legally be boolean (true) or an options object. If you set navigation: true, then swiper.params.navigation is a boolean, so code that does swiper.params.navigation.nextEl = … will fail unless you convert it to an object first. Swiper’s docs explicitly describe navigation as “object … or boolean true to enable with default settings.” [1] (and the typed options live in NavigationOptions [2]).
Fix (recommended): don’t use true; pass an object
const swiper = new Swiper('.swiper', {
navigation: {
nextEl: '.swiper-button-next',
prevEl: '.swiper-button-prev',
},
});This keeps swiper.params.navigation as an object from the start. [1][2]
If you need refs (React) and must patch in beforeInit / onBeforeInit
Convert boolean → object inside onBeforeInit (pattern commonly used with refs):
<Swiper
navigation={true} // or omit and set it fully in onBeforeInit
onBeforeInit={(swiper) => {
if (swiper.params.navigation === true) swiper.params.navigation = {};
swiper.params.navigation.prevEl = prevRef.current;
swiper.params.navigation.nextEl = nextRef.current;
}}
modules={[Navigation]}
/>This mirrors the established workaround: check for boolean before mutating. [3]
If you’re changing navigation after initialization (e.g., toggling from false/true to elements), you generally must (re)initialize navigation (e.g., swiper.navigation.init() then swiper.navigation.update()), not only swiper.update() (because update() won’t create the module instances for you).
Sources: Swiper v12 API (Navigation parameters) [1], Swiper v12 type defs for NavigationOptions [2], example pattern using onBeforeInit boolean-guard [3].
🌐 Web query:
Swiper React v12 Navigation module "onBeforeInit" timing params initialization
💡 Result:
In Swiper v12 (React), the onBeforeInit(swiper) handler runs right before Swiper’s initialization (“beforeInit” event). That’s the last moment you can safely mutate swiper.params (or module state) before the Navigation module reads/uses it during init. [1]
What this means for Navigation refs
- If you pass React refs directly in
navigation={{ prevEl: prevRef.current, nextEl: nextRef.current }}, they’re oftennullon the first render, so Navigation initializes withnulland buttons won’t work. - The canonical workaround is: set navigation elements inside
onBeforeInit, then let Swiper continue initializing. (This pattern is widely used for React + Swiper navigation refs.) [2]
If refs aren’t ready by onBeforeInit
If your refs are still null at onBeforeInit time (e.g., buttons mount later), you must initialize navigation after refs exist:
- set
swiper.params.navigation.prevEl/nextEl, - then call
swiper.navigation.init()andswiper.navigation.update()(update alone won’t create navigation). The Navigation module exposesinit()andupdate()for exactly this. [3]
Practical patterns (choose one)
Pattern A (preferred): refs exist before init)
- Render the prev/next elements in the same render pass, and assign them in
onBeforeInit.
Pattern B (defer Swiper init until refs exist)
- Create Swiper with
init={false}, then when refs are set, assignswiper.params.navigation.*and callswiper.init()(so Navigation initializes with real elements). The Swiper API notes init can be manually triggered wheninit: false. [1]
Pattern C (post-init attach)
- After mount (e.g.,
useEffectonce refs are non-null): set params, thenswiper.navigation.init()+swiper.navigation.update()(because Navigation needsinit()to be created). [3]
Sources: Swiper events docs for beforeInit timing and manual init behavior [1]; React Swiper usage pattern with onBeforeInit for custom navigation refs [2]; Navigation module methods (init, update) [3].
[1] (swiperjs.com)
[2] (stackoverflow.com)
[3] (swiperjs.com)
Citations:
- 1: https://swiperjs.com/swiper-api?utm_source=openai
- 2: https://stackoverflow.com/questions/64099383/swiper-react-how-to-create-custom-navigation-pagination-components-using-react?utm_source=openai
- 3: https://swiperjs.com/types/interfaces/types_modules_navigation.NavigationMethods?utm_source=openai
Pass an explicit navigation options object instead of the shorthand boolean to ensure refs are wired correctly.
At line 101, navigation={true} leaves swiper.params.navigation as a boolean when onBeforeInit fires at line 35. The guard check typeof swiper.params.navigation === "object" then evaluates to false, and prevRef/nextRef are never assigned to prevEl/nextEl. The buttons render and show correct enabled/disabled states, but clicking them does nothing because they're disconnected from the Swiper instance.
🔧 Proposed fix
- navigation
+ navigation={{}}
onBeforeInit={handleBeforeInit}Also applies to: 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/products/ProductCarousel.tsx` around lines 35 - 40, The Swiper
navigation is being passed as a boolean which leaves swiper.params.navigation as
a boolean when handleBeforeInit runs, so prevRef/nextRef never get wired; update
the navigation prop on the Swiper instance to be an explicit object that uses
the refs (e.g., navigation={{ prevEl: prevRef.current, nextEl: nextRef.current
}} or navigation={{ enabled: true, prevEl: prevRef.current, nextEl:
nextRef.current }}) and keep the handleBeforeInit callback (handleBeforeInit) to
assign prevEl/nextEl as a fallback; if refs may be null on first render, ensure
you also initialize/update navigation after mount (e.g., in an effect that calls
swiper.navigation.init()/update() when prevRef.current/nextRef.current become
available) so the prevRef and nextRef in ProductCarousel are properly wired to
the Swiper instance.
2 participants
Changes
swiper@^12.1.2) for carousel functionalitySummary by CodeRabbit
New Features
Chores
Chore / Cleanup