Use new locale middleware and improved list filtering/sorting - #39
Merged
Conversation
Contributor
|
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 ignored due to path filters (1)
📒 Files selected for processing (8)
WalkthroughThis pull request systematically refactors data-fetching functions and hooks across the codebase to remove optional locale and currency parameters, replacing generic parameter types with specific types from the Spree SDK (ProductListParams, TaxonListParams, OrderListParams). Store context dependencies are eliminated from hooks, and query parameter handling is standardized to use flattened keys instead of nested structures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useProductListing.ts (1)
39-43: 🛠️ Refactor suggestion | 🟠 MajorAdd an explicit return type to the exported hook.
useProductListingis exported but relies on type inference for its return shape. Define a concrete return type to lock the public API and comply with strict TypeScript requirements.The function should declare a return type interface containing:
products,loading,loadingMore,hasMore,totalCount,filtersData,filtersLoading,showMobileFilters,setShowMobileFilters,handleFilterChange, andloadMoreRef.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useProductListing.ts` around lines 39 - 43, Add an explicit return type for the exported hook by defining an interface (e.g., ProductListingResult) that includes products, loading, loadingMore, hasMore, totalCount, filtersData, filtersLoading, showMobileFilters, setShowMobileFilters, handleFilterChange, and loadMoreRef, then annotate the hook signature export function useProductListing({ fetchFn, filterParams = {}, searchQuery = "" }: UseProductListingOptions): ProductListingResult; update any internal returns to conform to this interface and export the interface if needed for external typing.
🧹 Nitpick comments (3)
src/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetailsWrapper.tsx (1)
25-56: Consider removingcurrencyfrom the effect dependency array.The
currencyvalue is only used for analytics tracking (trackViewItem(data, currency)) and doesn't affect the product data fetched. Including it in the dependency array causes the product to be re-fetched whenever the currency changes, which is unnecessary sincegetProductdoesn't use currency.Consider moving the tracking call outside the fetch effect or using a separate effect for analytics that depends on the product and currency.
♻️ Proposed refactor
useEffect(() => { let cancelled = false; const fetchProduct = async () => { setLoading(true); try { const data = await getProduct(slug, { includes: "variants,images,option_types", }); if (!cancelled) { setProduct(data); setError(false); - trackViewItem(data, currency); } } catch (err) { console.error("Failed to fetch product:", err); if (!cancelled) { setError(true); } } finally { if (!cancelled) { setLoading(false); } } }; fetchProduct(); return () => { cancelled = true; }; - }, [slug, currency]); + }, [slug]); + + // Track view_item when product loads + useEffect(() => { + if (product) { + trackViewItem(product, currency); + } + }, [product, currency]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetailsWrapper.tsx around lines 25 - 56, The current useEffect that fetches product data (fetchProduct / getProduct) should not depend on currency because currency is only used for analytics; remove currency from the dependency array and keep [slug] only so product is not re-fetched on currency changes, and keep setProduct/setLoading/setError handling as-is; then add a separate useEffect that watches product and currency and calls trackViewItem(product, currency) (guarding for non-null product) so analytics run when either product or currency changes without triggering the network fetch.src/lib/data/orders.ts (1)
14-15: Consider using a typed params object forgetOrderas well.For consistency with the
getOrdersrefactor usingOrderListParams, consider whethergetOrdershould also use a typed params object from the SDK instead ofRecord<string, unknown>. This would align with the PR's goal of standardizing on SDK types across data-fetching functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/orders.ts` around lines 14 - 15, Change getOrder’s loose params type to the SDK’s typed params used by getOrders (OrderListParams) or the SDK’s specific order params type if one exists: update the function signature of getOrder(id: string, params?: OrderListParams) (or OrderParams), adjust the call to _getOrder(id, params) accordingly, import the SDK type, and update any callers/tests to satisfy the new typed params so the function aligns with the standardized SDK types.src/lib/data/products.ts (1)
22-24: UseProductListParamsfor theparamsparameter to align with other product functions and enable compile-time validation.The function should match the type signature of
getProductsandgetTaxonProducts, which already useProductListParams. This type is already imported and ensures type safety across the data layer.Suggested change
-export async function getProductFilters(params?: Record<string, unknown>) { +export async function getProductFilters(params?: ProductListParams) { return _getProductFilters(params); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/products.ts` around lines 22 - 24, Update getProductFilters to accept ProductListParams instead of Record<string, unknown> so it matches the signatures used by getProducts and getTaxonProducts and enables compile-time validation; change the params parameter type in the getProductFilters function declaration to ProductListParams and pass it through to _getProductFilters unchanged, ensuring ProductListParams is imported where getProductFilters is defined and referenced.
🤖 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/`[country]/[locale]/(storefront)/products/ProductsContent.tsx:
- Around line 10-13: Remove the stray blank line after the import block to fix
formatting: ensure the import "getProducts" and the following interface
declaration "interface ProductsContentProps" are adjacent with no empty line
between them so the file starts with the import immediately followed by the
interface declaration.
In
`@src/app/`[country]/[locale]/(storefront)/t/[...permalink]/CategoryProductsContent.tsx:
- Around line 9-12: Remove the extra blank line after the import block to
satisfy formatting rules: ensure the import statement "import { getTaxonProducts
} from "@/lib/data/products";" is immediately followed by the "interface
CategoryProductsContentProps" declaration with no intervening blank line so the
file compiles with CI formatting checks.
In `@src/components/products/ProductFilters.tsx`:
- Around line 12-26: The hardcoded English maps SORT_LABELS and
AVAILABILITY_LABELS override localized labels from the API; remove these static
maps and use the label provided by the API (e.g., option.label /
sortOption.label) or the existing localization utility (e.g., t(...) or
props.localizedLabels) as the primary source, falling back to a short default
only if the API label is absent; update any usages in ProductFilters (references
to SORT_LABELS and AVAILABILITY_LABELS) to read the dynamic label from the
sort/filter option object or translation helper instead.
In `@src/lib/data/orders.ts`:
- Around line 3-4: The imports in orders.ts are misordered causing the CI Biome
rule to fail; move the value imports (getOrder as _getOrder, listOrders from
"@spree/next") before the type-only import and alphabetize groups, so the
non-type import group appears first and the type import (OrderListParams from
"@spree/sdk") appears after; after reordering, run the repo formatter/linter to
ensure the import sorting rule passes.
In `@src/lib/data/products.ts`:
- Around line 3-9: Imports in this file are unsorted causing CI to fail; reorder
them to satisfy the project's organize-imports rules by grouping and
alphabetizing: keep the type-only import for ProductListParams separate (at the
top), then import from `@spree/next` as a single block with the specifier sorted
and the named imports alphabetized (getProduct as _getProduct, getProductFilters
as _getProductFilters, listProducts, listTaxonProducts), or simply run the
project's import organizer to apply the correct ordering.
In `@src/lib/data/store.ts`:
- Around line 5-6: Add an explicit return type to the exported async function
getStore by annotating its signature instead of relying on inference; use a
precise type (avoid any) such as ReturnType<typeof _getStore> or the concrete
Promise<Store> type if you know the store interface, i.e., update the getStore
declaration to include that return type while keeping the body returning
_getStore().
In `@src/lib/data/taxonomies.ts`:
- Around line 3-9: The type import is out of order and causes CI failures; move
the type-only import "TaxonListParams" so it comes after the regular runtime
imports (getTaxon as _getTaxon, getTaxonomy as _getTaxonomy, listTaxonomies,
listTaxons) in src/lib/data/taxonomies.ts, keeping the same import names and
aliases (_getTaxon, _getTaxonomy) and ensuring only type-only syntax remains for
TaxonListParams.
In `@src/proxy.ts`:
- Around line 11-15: The config.matcher array in export const config is not
formatted per the project's formatter; open src/proxy.ts, locate export const
config and replace the existing matcher value (the array containing the regex
string) with the formatter-produced version (i.e., reformat the
array/whitespace/quoting to match the project's style) so the matcher definition
(config.matcher) matches the CI formatter output; save and run the formatter to
confirm no further changes.
- Line 1: The build fails because src/proxy.ts imports createSpreeMiddleware
from a non-existent subpath "@spree/next/middleware"; either update the
dependency to a version of `@spree/next` that actually exports
createSpreeMiddleware under that subpath or change the import to the correct
export path (e.g., import createSpreeMiddleware from the top-level export) and
then reinstall/update packages; locate the import of createSpreeMiddleware in
src/proxy.ts, verify the correct export in the installed `@spree/next` package (or
bump `@spree/next` to a version that includes the middleware export), update
package.json and run your package manager to restore a valid import.
---
Outside diff comments:
In `@src/hooks/useProductListing.ts`:
- Around line 39-43: Add an explicit return type for the exported hook by
defining an interface (e.g., ProductListingResult) that includes products,
loading, loadingMore, hasMore, totalCount, filtersData, filtersLoading,
showMobileFilters, setShowMobileFilters, handleFilterChange, and loadMoreRef,
then annotate the hook signature export function useProductListing({ fetchFn,
filterParams = {}, searchQuery = "" }: UseProductListingOptions):
ProductListingResult; update any internal returns to conform to this interface
and export the interface if needed for external typing.
---
Nitpick comments:
In
`@src/app/`[country]/[locale]/(storefront)/products/[slug]/ProductDetailsWrapper.tsx:
- Around line 25-56: The current useEffect that fetches product data
(fetchProduct / getProduct) should not depend on currency because currency is
only used for analytics; remove currency from the dependency array and keep
[slug] only so product is not re-fetched on currency changes, and keep
setProduct/setLoading/setError handling as-is; then add a separate useEffect
that watches product and currency and calls trackViewItem(product, currency)
(guarding for non-null product) so analytics run when either product or currency
changes without triggering the network fetch.
In `@src/lib/data/orders.ts`:
- Around line 14-15: Change getOrder’s loose params type to the SDK’s typed
params used by getOrders (OrderListParams) or the SDK’s specific order params
type if one exists: update the function signature of getOrder(id: string,
params?: OrderListParams) (or OrderParams), adjust the call to _getOrder(id,
params) accordingly, import the SDK type, and update any callers/tests to
satisfy the new typed params so the function aligns with the standardized SDK
types.
In `@src/lib/data/products.ts`:
- Around line 22-24: Update getProductFilters to accept ProductListParams
instead of Record<string, unknown> so it matches the signatures used by
getProducts and getTaxonProducts and enables compile-time validation; change the
params parameter type in the getProductFilters function declaration to
ProductListParams and pass it through to _getProductFilters unchanged, ensuring
ProductListParams is imported where getProductFilters is defined and referenced.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
src/app/[country]/[locale]/(storefront)/products/ProductsContent.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/ProductDetailsWrapper.tsxsrc/app/[country]/[locale]/(storefront)/t/[...permalink]/CategoryProductsContent.tsxsrc/components/products/ProductFilters.tsxsrc/components/search/SearchBar.tsxsrc/hooks/useCarouselProducts.tssrc/hooks/useProductListing.tssrc/lib/data/countries.tssrc/lib/data/orders.tssrc/lib/data/products.tssrc/lib/data/store.tssrc/lib/data/taxonomies.tssrc/lib/utils/product-query.tssrc/proxy.ts
Comment on lines
+12
to
+26
| const SORT_LABELS: Record<string, string> = { | ||
| manual: "Manual", | ||
| best_selling: "Best Selling", | ||
| "price asc": "Price (low-high)", | ||
| "price desc": "Price (high-low)", | ||
| "available_on desc": "Newest", | ||
| "available_on asc": "Oldest", | ||
| "name asc": "Name (A-Z)", | ||
| "name desc": "Name (Z-A)", | ||
| }; | ||
|
|
||
| const AVAILABILITY_LABELS: Record<string, string> = { | ||
| in_stock: "In Stock", | ||
| out_of_stock: "Out of Stock", | ||
| }; |
Contributor
There was a problem hiding this comment.
Avoid overriding localized labels with hardcoded English maps.
Line 137 and Line 324 now prioritize static labels keyed by IDs, which can bypass localized labels from the API and regress non-English storefronts.
💡 Suggested adjustment
- {SORT_LABELS[option.id] || option.id}
+ {option.label || SORT_LABELS[option.id] || option.id}
- {AVAILABILITY_LABELS[option.id] || option.id}
+ {option.label || AVAILABILITY_LABELS[option.id] || option.id}Also applies to: 137-137, 323-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/products/ProductFilters.tsx` around lines 12 - 26, The
hardcoded English maps SORT_LABELS and AVAILABILITY_LABELS override localized
labels from the API; remove these static maps and use the label provided by the
API (e.g., option.label / sortOption.label) or the existing localization utility
(e.g., t(...) or props.localizedLabels) as the primary source, falling back to a
short default only if the API label is absent; update any usages in
ProductFilters (references to SORT_LABELS and AVAILABILITY_LABELS) to read the
dynamic label from the sort/filter option object or translation helper instead.
Comment on lines
+5
to
+6
| export async function getStore() { | ||
| return _getStore(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the full file content
cat -n src/lib/data/store.tsRepository: spree/nextjs-starter-spree
Length of output: 246
🏁 Script executed:
# Search for the _getStore import/definition
rg -n "_getStore" src/lib/data/Repository: spree/nextjs-starter-spree
Length of output: 192
🏁 Script executed:
# Check package imports to understand what Spree SDK types are available
rg -n "from.*@spree" src/lib/data/store.ts -A 2 -B 2Repository: spree/nextjs-starter-spree
Length of output: 183
🏁 Script executed:
# Check tsconfig for type settings
cat -n tsconfig.json | head -40Repository: spree/nextjs-starter-spree
Length of output: 1017
🏁 Script executed:
# Look for Store-related types or type exports in the codebase
rg -n "type.*Store|interface.*Store" src/ --type=ts -A 2 | head -50Repository: spree/nextjs-starter-spree
Length of output: 3170
🏁 Script executed:
# Check if there are any type definitions or references for the Spree SDK
rg -n "@spree/next" src/ --type=ts -A 1 -B 1 | head -40Repository: spree/nextjs-starter-spree
Length of output: 1927
🏁 Script executed:
# Look at other exported functions in src/lib/data/ to see return type patterns
rg -n "export\s+(async\s+)?function" src/lib/data/ --type=ts -A 1 | head -60Repository: spree/nextjs-starter-spree
Length of output: 2971
🏁 Script executed:
# Check if there are any return type annotations in the data functions
rg -n "export\s+(async\s+)?function.*:" src/lib/data/ --type=ts -A 1Repository: spree/nextjs-starter-spree
Length of output: 4416
🏁 Script executed:
# Verify the actual return type by checking what _getStore returns
rg -n "export.*getStore" --type=ts -B 5 -A 3 src/lib/data/store.tsRepository: spree/nextjs-starter-spree
Length of output: 211
🏁 Script executed:
# Check if StoreStore type exists which would be appropriate return type
rg -n "StoreStore" src/ --type=ts -A 1 -B 1 | head -20Repository: spree/nextjs-starter-spree
Length of output: 1202
Add explicit return type to getStore().
The function relies on type inference; TypeScript strict mode requires explicit return type annotations.
Proposed change
-export async function getStore() {
+export async function getStore(): Promise<Awaited<ReturnType<typeof _getStore>>> {
return _getStore();
}Per coding guideline: **/*.ts{,x}: Use strict TypeScript type checking; define explicit return types for functions and avoid 'any' type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/data/store.ts` around lines 5 - 6, Add an explicit return type to the
exported async function getStore by annotating its signature instead of relying
on inference; use a precise type (avoid any) such as ReturnType<typeof
_getStore> or the concrete Promise<Store> type if you know the store interface,
i.e., update the getStore declaration to include that return type while keeping
the body returning _getStore().
v0.6.0 did not include the ./middleware subpath export, causing tsc --noEmit to fail in CI with TS2307. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 3, 2026
Merged
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
1 participant
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 by CodeRabbit
New Features
Improvements