-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Adds Flagsmith example #1144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@tiagoapolo is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
return ( | ||
<button | ||
type="button" | ||
className={`${colorMap[color]} cursor-pointer w-full rounded-full border border-transparent px-4 py-3 text-base font-medium text-white shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:ring-offset-gray-50`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
className={`${colorMap[color]} cursor-pointer w-full rounded-full border border-transparent px-4 py-3 text-base font-medium text-white shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:ring-offset-gray-50`} | |
className={`${colorMap[color] || colorMap.blue} cursor-pointer w-full rounded-full border border-transparent px-4 py-3 text-base font-medium text-white shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:ring-offset-gray-50`} |
The ProceedToCheckoutButton
uses colorMap[color]
without handling cases where color
is not in the map, which could result in undefined
CSS classes and broken styling.
View Details
Analysis
The ProceedToCheckoutButton
component accesses colorMap[color]
directly without checking if the color exists in the map. The proceedToCheckoutColorFlag
can have a default value of 'blue' and options of ['blue', 'green', 'red'], but if an invalid color value is somehow passed or if the flag system returns an unexpected value, colorMap[color]
will be undefined
.
When undefined
is interpolated into the className string, it becomes the literal text "undefined" in the CSS class, which is not a valid Tailwind class and will result in no styling being applied to the button.
Recommendation
Add a fallback for missing colors in the color map:
className={`${colorMap[color] || colorMap.blue} cursor-pointer w-full rounded-full border border-transparent px-4 py-3 text-base font-medium text-white shadow-sm focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 focus:ring-offset-gray-50`}
Or add a default case to the colorMap and use nullish coalescing:
const colorMap: Record<string, string> = {
blue: 'bg-blue-600 hover:bg-blue-700',
red: 'bg-red-600 hover:bg-red-700',
green: 'bg-green-600 hover:bg-green-700',
default: 'bg-blue-600 hover:bg-blue-700',
}
// Then use: colorMap[color] || colorMap.default
return fetch(`${BACKEND_URL}/api/cart/${cartId.value}`).then((res) => | ||
res.json() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getCart
function doesn't handle network errors or non-successful HTTP responses, which could cause the shopping cart to fail silently or throw unhandled exceptions.
View Details
📝 Patch Details
diff --git a/flags-sdk/flagsmith/lib/actions.ts b/flags-sdk/flagsmith/lib/actions.ts
index deb694df..4821d513 100644
--- a/flags-sdk/flagsmith/lib/actions.ts
+++ b/flags-sdk/flagsmith/lib/actions.ts
@@ -18,9 +18,13 @@ export async function getCart(): Promise<Cart> {
const cartId = await getCartId()
const delayMs = await delayFlag()
await delay(delayMs)
- return fetch(`${BACKEND_URL}/api/cart/${cartId.value}`).then((res) =>
- res.json()
- )
+
+ const response = await fetch(`${BACKEND_URL}/api/cart/${cartId.value}`)
+ if (!response.ok) {
+ throw new Error(`Failed to fetch cart: ${response.status}`)
+ }
+
+ return response.json()
}
export async function addToCart(item: CartItem) {
Analysis
The getCart
function makes a fetch request and directly calls res.json()
without checking if the response was successful (status 200-299). If the API returns an error status code (404, 500, etc.) or if there's a network failure, this will either throw an unhandled exception or return malformed data.
The same issue exists in addToCart
and removeFromCart
functions, but they're fire-and-forget operations where the lack of error handling is less critical. However, getCart
is used to render the shopping cart UI, so failures here directly impact the user interface.
Recommendation
Add response status checking and error handling:
export async function getCart(): Promise<Cart> {
const cartId = await getCartId()
const delayMs = await delayFlag()
await delay(delayMs)
const response = await fetch(`${BACKEND_URL}/api/cart/${cartId.value}`)
if (!response.ok) {
throw new Error(`Failed to fetch cart: ${response.status}`)
}
return response.json()
}
onClick={async () => { | ||
setIsLoading(true) | ||
track('add_to_cart:clicked') | ||
await addToCart({ id: 'shirt', color, size, quantity: 1 }) | ||
router.push('/cart') | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AddToCart
component sets loading state to true
but never resets it to false
if the addToCart
API call fails, leaving the button permanently in a loading state.
View Details
📝 Patch Details
diff --git a/flags-sdk/flagsmith/app/[code]/add-to-cart.tsx b/flags-sdk/flagsmith/app/[code]/add-to-cart.tsx
index c4d7c18b..f71625a7 100644
--- a/flags-sdk/flagsmith/app/[code]/add-to-cart.tsx
+++ b/flags-sdk/flagsmith/app/[code]/add-to-cart.tsx
@@ -21,9 +21,16 @@ export function AddToCart() {
isLoading={isLoading}
onClick={async () => {
setIsLoading(true)
- track('add_to_cart:clicked')
- await addToCart({ id: 'shirt', color, size, quantity: 1 })
- router.push('/cart')
+ try {
+ track('add_to_cart:clicked')
+ await addToCart({ id: 'shirt', color, size, quantity: 1 })
+ router.push('/cart')
+ } catch (error) {
+ console.error('Failed to add item to cart:', error)
+ // Optionally show error feedback to user
+ } finally {
+ setIsLoading(false)
+ }
}}
/>
)
Analysis
In the AddToCart
component, the onClick
handler sets isLoading
to true
at the start but has no error handling to reset it if the addToCart
API call fails. If addToCart
throws an error (network failure, server error, etc.), the component will remain in the loading state indefinitely, making the button unusable.
This creates a poor user experience where users can't retry the action after a network failure, and the UI provides no feedback about what went wrong.
Recommendation
Wrap the API call in a try-catch block and ensure setIsLoading(false)
is called in the catch block:
onClick={async () => {
setIsLoading(true)
try {
track('add_to_cart:clicked')
await addToCart({ id: 'shirt', color, size, quantity: 1 })
router.push('/cart')
} catch (error) {
console.error('Failed to add item to cart:', error)
// Optionally show error feedback to user
} finally {
setIsLoading(false)
}
}}
final work
Description
This PR adds an usage example for Flagsmith Adapter implement in vercel/flags#103
Demo URL
https://flagsmith-six.vercel.app/
Type of Change
New Example Checklist
npm run new-example
was used to create the example