Skip to content

Commit ede2802

Browse files
Cichorekclaude
andcommitted
Address PR #13 review feedback
- Fix race condition in useProductListing with loadIdRef guard for both loadProducts and loadMore to prevent stale results - Add .catch handlers to fetchStates in AddressEditModal and PaymentStep to prevent unhandled promise rejections - Simplify handleUpdateSavedAddress with early-return guard clauses - Throw instead of silently returning in handleSaveEditedAddress - Fix PriceFilter inputs not resetting on clearFilters via key prop - Add error logging to withFallback for production observability - Use neutral fallback message in applyCouponCode - Encode cookie values in setStoreCookies - Spread emptyAddress to avoid shared mutable reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 573f1cb commit ede2802

File tree

10 files changed

+41
-22
lines changed

10 files changed

+41
-22
lines changed

src/app/[country]/[locale]/(checkout)/checkout/[id]/page.tsx

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -369,16 +369,21 @@ export default function CheckoutPage({ params }: CheckoutPageProps) {
369369
// Update a saved address
370370
const handleUpdateSavedAddress = async (id: string, data: AddressParams) => {
371371
const result = await updateAddress(id, data);
372+
372373
if (!result.success) {
373374
throw new Error(result.error || "Failed to update address");
374375
}
375-
if (result.address) {
376-
setSavedAddresses((prev) =>
377-
prev.map((addr) => (addr.id === id ? result.address! : addr)),
378-
);
379-
return result.address;
376+
377+
if (!result.address) {
378+
throw new Error("Update succeeded but address payload is missing");
380379
}
381-
throw new Error("Failed to update address");
380+
381+
const updatedAddress = result.address;
382+
setSavedAddresses((prev) =>
383+
prev.map((addr) => (addr.id === id ? updatedAddress : addr)),
384+
);
385+
386+
return updatedAddress;
382387
};
383388

384389
// Navigate back to a previous step

src/components/checkout/AddressEditModal.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export function AddressEditModal({
4444
title,
4545
}: AddressEditModalProps) {
4646
const [formData, setFormData] = useState<AddressFormData>(
47-
address ? addressToFormData(address) : emptyAddress,
47+
address ? addressToFormData(address) : { ...emptyAddress },
4848
);
4949
const [states, setStates] = useState<StoreState[]>([]);
5050
const [loadingStates, setLoadingStates] = useState(false);
@@ -78,6 +78,9 @@ export function AddressEditModal({
7878
.then((result) => {
7979
if (!cancelled) setStates(result);
8080
})
81+
.catch(() => {
82+
if (!cancelled) setStates([]);
83+
})
8184
.finally(() => {
8285
if (!cancelled) setLoadingStates(false);
8386
});

src/components/checkout/AddressStep.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ export function AddressStep({
107107
};
108108

109109
const handleSaveEditedAddress = async (data: AddressParams, id?: string) => {
110-
if (!id || !onUpdateSavedAddress) return;
110+
if (!id || !onUpdateSavedAddress) {
111+
throw new Error("Cannot update address");
112+
}
111113

112114
const updatedAddress = await onUpdateSavedAddress(id, data);
113115
if (updatedAddress) {

src/components/checkout/PaymentStep.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,13 @@ export function PaymentStep({
5959
let cancelled = false;
6060

6161
startTransitionBill(() => {
62-
fetchStates(billAddress.country_iso).then((states) => {
63-
if (!cancelled) {
64-
setBillStates(states);
65-
}
66-
});
62+
fetchStates(billAddress.country_iso)
63+
.then((states) => {
64+
if (!cancelled) setBillStates(states);
65+
})
66+
.catch(() => {
67+
if (!cancelled) setBillStates([]);
68+
});
6769
});
6870

6971
return () => {

src/components/products/ProductFilters.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ export function ProductFilters({
145145
onToggle={() => toggleSection(filter.id)}
146146
>
147147
<PriceFilter
148+
key={`${activeFilters.priceMin}-${activeFilters.priceMax}`}
148149
filter={filter as PriceRangeFilter}
149150
minValue={activeFilters.priceMin}
150151
maxValue={activeFilters.priceMax}

src/hooks/useProductListing.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export function useProductListing({
4747
const pageRef = useRef(1);
4848
const hasMoreRef = useRef(false);
4949
const filtersRef = useRef<ActiveFilters>({ optionValues: [] });
50+
const loadIdRef = useRef(0);
5051

5152
const fetchProducts = useCallback(
5253
async (page: number, filters: ActiveFilters, query: string) => {
@@ -68,18 +69,21 @@ export function useProductListing({
6869
async (filters: ActiveFilters, query: string) => {
6970
setLoading(true);
7071
pageRef.current = 1;
72+
const currentLoadId = ++loadIdRef.current;
7173

7274
const response = await fetchProducts(1, filters, query);
7375

74-
if (response) {
76+
if (response && loadIdRef.current === currentLoadId) {
7577
setProducts(response.data);
7678
setTotalCount(response.meta.count);
7779
const moreAvailable = 1 < response.meta.pages;
7880
setHasMore(moreAvailable);
7981
hasMoreRef.current = moreAvailable;
8082
}
8183

82-
setLoading(false);
84+
if (loadIdRef.current === currentLoadId) {
85+
setLoading(false);
86+
}
8387
},
8488
[fetchProducts],
8589
);
@@ -140,11 +144,12 @@ export function useProductListing({
140144
if (loadingMore || !hasMoreRef.current) return;
141145

142146
setLoadingMore(true);
147+
const currentLoadId = loadIdRef.current;
143148
const nextPage = pageRef.current + 1;
144149

145150
const response = await fetchProducts(nextPage, activeFilters, searchQuery);
146151

147-
if (response) {
152+
if (response && loadIdRef.current === currentLoadId) {
148153
setProducts((prev) => [...prev, ...response.data]);
149154
const moreAvailable = nextPage < response.meta.pages;
150155
setHasMore(moreAvailable);

src/lib/data/__tests__/checkout.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,14 @@ describe("checkout server actions", () => {
192192
});
193193
});
194194

195-
it("returns 'Invalid coupon code' for non-Error throws", async () => {
195+
it("returns fallback message for non-Error throws", async () => {
196196
mockApplyCoupon.mockRejectedValue("unexpected");
197197

198198
const result = await applyCouponCode("order-1", "BAD");
199199

200200
expect(result).toEqual({
201201
success: false,
202-
error: "Invalid coupon code",
202+
error: "Failed to apply coupon code",
203203
});
204204
});
205205
});

src/lib/data/checkout.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ export async function applyCouponCode(orderId: string, couponCode: string) {
6262
return actionResult(async () => {
6363
const order = await applyCoupon(orderId, couponCode);
6464
return { order };
65-
}, "Invalid coupon code");
65+
}, "Failed to apply coupon code");
6666
}
6767

6868
export async function removeCouponCode(orderId: string, promotionId: string) {

src/lib/data/utils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ export async function withFallback<T>(
2626
): Promise<T> {
2727
try {
2828
return await fn();
29-
} catch {
29+
} catch (error) {
30+
console.error(error);
3031
return fallback;
3132
}
3233
}

src/lib/utils/cookies.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ const ONE_YEAR = 60 * 60 * 24 * 365;
44
* Persist the selected country and locale in client-side cookies.
55
*/
66
export function setStoreCookies(country: string, locale: string): void {
7-
document.cookie = `spree_country=${country}; path=/; max-age=${ONE_YEAR}`;
8-
document.cookie = `spree_locale=${locale}; path=/; max-age=${ONE_YEAR}`;
7+
document.cookie = `spree_country=${encodeURIComponent(country)}; path=/; max-age=${ONE_YEAR}`;
8+
document.cookie = `spree_locale=${encodeURIComponent(locale)}; path=/; max-age=${ONE_YEAR}`;
99
}

0 commit comments

Comments
 (0)