vercel / commerce

Next.js Commerce
https://demo.vercel.store
MIT License
10.53k stars 3.87k forks source link

[Bug] Selecting variant with different price point doesn't update price tag. #508

Closed ceiphr closed 1 year ago

ceiphr commented 2 years ago

The price will be updated upon adding to cart, but the change in price is not shown in the product view.

lfades commented 2 years ago

@ceiphr Is the bug happening in our demo with BigCommerce or with another provider, or in general?

ceiphr commented 2 years ago

It is occurring with Shopify. I'm unfamiliar with how other providers handle price for variants. I haven't tested this with any of the demos.

The following change to the price object's amount in /components/product/ProductSidebar resolved the issue:

const { price } = usePrice({
    amount: (variant ? variant.price : product.variants[0].price), // instead of product.price.value
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
})

Should I make a PR for this, @lfades?

lfades commented 2 years ago

@ceiphr it may have broke for other providers with that change, in this case we need to make sure the price of the product matches the variant 🤔

ceiphr commented 2 years ago

Would it make more sense to do this?

variant ? variant.price : product.price.value

So, variant price is only applied if a variant is being shown. As I understand from the logic, the product object is consistent between providers, correct?

geniti commented 2 years ago

it seems that it doesn't handle price switch in producttag.

@lfades this code you shared above isnt in file

The following change to the price object's amount in /components/product/ProductSidebar resolved the issue:

const { price } = usePrice({ amount: (variant ? variant.price : product.variants[0].price), // instead of product.price.value baseAmount: product.price.retailPrice, currencyCode: product.price.currencyCode!, })

found a solution already? im using Shopify also and before working with BigCommerce same issue

ceiphr commented 2 years ago

@geniti, I'm a bit confused by your comment. The snippet I commented is the solution I've made which seems to work for Shopify. Specifically, amount: (variant ? variant.price : product.variants[0].price). This way, variant price is used, which will update when a variant is selected. I hope that helps!

geniti commented 2 years ago

@ceiphr sorry but my file looks like this components/product/productsidebar

import s from './ProductSidebar.module.css' import { useAddItem } from '@framework/cart' import { FC, useEffect, useState } from 'react' import { ProductOptions } from '@components/product' import type { Product } from '@commerce/types/product' import { Button, Text, Rating, Collapse, useUI } from '@components/ui' import { getProductVariant, selectDefaultOptionFromProduct, SelectedOptions, } from '../helpers'

interface ProductSidebarProps { product: Product className?: string }

const ProductSidebar: FC = ({ product, className }) => { const addItem = useAddItem() const { openSidebar } = useUI() const [loading, setLoading] = useState(false) const [selectedOptions, setSelectedOptions] = useState({})

useEffect(() => { selectDefaultOptionFromProduct(product, setSelectedOptions) }, [product])

const variant = getProductVariant(product, selectedOptions) const addToCart = async () => { setLoading(true) try { await addItem({ productId: String(product.id), variantId: String(variant ? variant.id : product.variants[0].id), }) openSidebar() setLoading(false) } catch (err) { setLoading(false) } }

return (

36 reviews
{process.env.COMMERCE_CART_ENABLED && ( )}
This is a limited edition production run. Printing starts when the drop ends. This is a limited edition production run. Printing starts when the drop ends. Reminder: Bad Boys For Life. Shipping may take 10+ days due to COVID-19.

) }

export default ProductSidebar

Where should I add your solution?

ceiphr commented 2 years ago

@geniti the version of commerce starter kit I'm using has this before the return:

const { price } = usePrice({
    amount: product.price.value,
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
})

Mine also has a Text component for displaying the price:

<Text
    className='text-cyan pb-4 break-words w-full max-w-xl'
    variant='sectionHeading'
    html={`${price} ${product.price?.currencyCode}`}
/>

My solution is to rewrite amount for the object passed to usePrice, like so:

variant ? variant.price : product.price.value

It seems like your file is missing these parts of the starter. Maybe you're using an older version?

gnacoding commented 2 years ago

EDIT Found the error - would be great if someone could verify that I don't break something else with this solution. Open file:

framework\commerce\types\product.ts

Add the line price: number to export type ProductVariant ={...

export type ProductVariant = {
  id: string | number
  options: ProductOption[]
  availableForSale?: boolean
  price: number
}

/EDIT

variant ? variant.price : product.price.value

@ceiphr did you have to make other adjustments to product/ProductSidebar/ProductSidebar.tsx as well? I work on the latest vercel/commerce pull and by changing the amount with your line throws the error, that the variable variant is used before its declaration. My solution was to add const variant = getProductVariant(product, selectedOptions) above const { price } = usePrice({... like this:


const ProductSidebar: FC<ProductSidebarProps> = ({ product, className }) => {
  const addItem = useAddItem()
  const { openSidebar } = useUI()
  const [loading, setLoading] = useState(false)
  const [selectedOptions, setSelectedOptions] = useState<SelectedOptions>({})
  const variant = getProductVariant(product, selectedOptions)
  const { price } = usePrice({
    amount: (variant ? variant.price : product.price.value),
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
  })

  useEffect(() => {
    selectDefaultOptionFromProduct(product, setSelectedOptions)
  }, [product])

  const addToCart = async () => {
    setLoading(true)
    try {
      await addItem({
        productId: String(product.id),
        variantId: String(variant ? variant.id : product.variants[0].id),
      })
      openSidebar()
      setLoading(false)
    } catch (err) {
      setLoading(false)
    }
  }

It works but variant.price is giving me an error:

Property 'price' does not exist on type 'ProductVariant'

Did you change something else as well?

ceiphr commented 2 years ago

@gnacoding nice catch! I completely forgot I updated the ProductVariant type.

Also, to answer your question, my version of vercel/commerce is two months old. So, either, the ordering of price and variant was different before, or that was another thing I forgot about.

I'm going to make a fork and write a PR for this.

Side note: @lfades, just curious, are you willing to add this project to Hacktoberfest?

geniti commented 2 years ago

im trying to get this to work for like 4 months now. But seems that nothing will work.

Did anyone @lfades @ceiphr @gnacoding find a solution to this updating price variant with Shopify as provider?

please don't go use BigCommerce, as there infrastructure is a hell

gnacoding commented 2 years ago

im trying to get this to work for like 4 months now. But seems that nothing will work.

Did anyone @lfades @ceiphr @gnacoding find a solution to this updating price variant with Shopify as provider?

please don't go use BigCommerce, as there infrastructure is a hell

@geniti

The solution provided by @ceiphr and my addition to update ProductVariant in product.ts will update the price when selecting a variant. I am using Shopify as the provider as well.

I don't want to derail this issue but has anybody been able to achieve, that the image is changed as well when selecting a variant?

geniti commented 2 years ago

im trying to get this to work for like 4 months now. But seems that nothing will work. Did anyone @lfades @ceiphr @gnacoding find a solution to this updating price variant with Shopify as provider? please don't go use BigCommerce, as there infrastructure is a hell

@geniti

The solution provided by @ceiphr and my addition to update ProductVariant in product.ts will update the price when selecting a variant. I am using Shopify as the provider as well.

I don't want to derail this issue but has anybody been able to achieve, that the image is changed as well when selecting a variant?

Could you maybe share your version of the kit? im using the latest pull version but files does not seem the same @gnacoding thanks in advance

gnacoding commented 2 years ago

@geniti

This is what I have before return()

import s from './ProductSidebar.module.css'
import { useAddItem } from '@framework/cart'
import { FC, useEffect, useState } from 'react'
import { ProductOptions } from '@components/product'
import type { Product } from '@commerce/types/product'
import ProductTag from '../ProductTag'
import usePrice from '@framework/product/use-price'
import { WishlistButton } from '@components/wishlist'
import { Button, Text, Rating, Collapse, useUI } from '@components/ui'
import {
  getProductVariant,
  selectDefaultOptionFromProduct,
  SelectedOptions,
} from '../helpers'

interface ProductSidebarProps {
  product: Product
  className?: string
}

const ProductSidebar: FC<ProductSidebarProps> = ({ product, className }) => {
  const addItem = useAddItem()
  const { openSidebar } = useUI()
  const [loading, setLoading] = useState(false)
  const [selectedOptions, setSelectedOptions] = useState<SelectedOptions>({})
  const variant = getProductVariant(product, selectedOptions)
  const { price } = usePrice({
    amount: (variant ? variant.price : product.price.value),
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
  })

  useEffect(() => {
    selectDefaultOptionFromProduct(product, setSelectedOptions)
  }, [product])

  const addToCart = async () => {
    setLoading(true)
    try {
      await addItem({
        productId: String(product.id),
        variantId: String(variant ? variant.id : product.variants[0].id),
      })
      openSidebar()
      setLoading(false)
    } catch (err) {
      setLoading(false)
    }
  }

NOTE: I rearranged some variables

I pulled the main repo when this commit was made: f9644fecefa73229a5384ee70f86499be48ff3fa The funny thing is, when I compare the history of ProductSidebar.tsx I don't see the code:

 const { price } = usePrice({
    amount: (variant ? variant.price : product.price.value),
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
  })

I am not sure if this is somehow injected after the provider is initialised - I hope someone can explain this.

EDIT:

I just saw that the function usePrice have been copied from ProductView.tsx since I changed the Price from showing above the Image to the Sidebar. My bad. Just adjust the function there and the prices will update after selecting a variant.

/EDIT

The price is then displayed and updated on click with the ProductTag-Component

<ProductTag 
          variant='onlyprice'
          name={product.name}
          price={`${price}`}
          fontSize={32}
        />

NOTE#2: Please check what variants you have in the ProductTag-Component since I have added some variants by myself.

Hope this helps!

philsmithies commented 2 years ago

Having the same issue with Shopify as of now

gnacoding commented 1 year ago

I'm resurrecting this topic due to the recent update to next13.

The code:

    amount: (variant ? variant.price : product.price.value),
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
  })

still works but TypeScript is now having a problem with 'amount':

Type 'number | ProductPrice | undefined' is not assignable to type 'number'.
  Type 'undefined' is not assignable to type 'number'.ts(2322)

ProductPrice.value ( is a defined as 'number' in product.ts - so I'm not quite sure what I'm missing. By using:

    amount: (variant ? variant.price?.value : product.price.value),

TypeScript still says it's not assignable to type 'number'

Could someone please give me a hint?

EDIT: in product.ts > export interface ProductVariant > price? is now optional

gnacoding commented 1 year ago

I'm resurrecting this topic due to the recent update to next13.

The code:

    amount: (variant ? variant.price : product.price.value),
    baseAmount: product.price.retailPrice,
    currencyCode: product.price.currencyCode!,
  })

still works but TypeScript is now having a problem with 'amount':

Type 'number | ProductPrice | undefined' is not assignable to type 'number'.
  Type 'undefined' is not assignable to type 'number'.ts(2322)

ProductPrice.value ( is a defined as 'number' in product.ts - so I'm not quite sure what I'm missing. By using:

    amount: (variant ? variant.price?.value : product.price.value),

TypeScript still says it's not assignable to type 'number'

Could someone please give me a hint?

EDIT: in product.ts > export interface ProductVariant > price? is now optional

For anyone interested - I ended up modifying the accepted types of the usePrice.tsx hook:

packages/commerce/src/product/use-price.tsx

export default function usePrice(
  data?: {
    amount: number | any  /*<--- Add any */
    baseAmount?: number
    currencyCode: string
  } | null
) {
  const { amount, baseAmount, currencyCode } = data ?? {}
  const { locale } = useCommerce()
  const value = useMemo(() => {
    if (typeof amount !== 'number' || !currencyCode) return ''

    return baseAmount
      ? formatVariantPrice({ amount, baseAmount, currencyCode, locale })
      : formatPrice({ amount, currencyCode, locale })
  }, [amount, baseAmount, currencyCode])

  return typeof value === 'string' ? { price: value } : value
}
leerob commented 1 year ago

Hey there! Thank you for opening this issue. We have decided to take Next.js Commerce in a new direction and will be closing out current PRs and issues due to this change. Please see this PR for more details: https://github.com/vercel/commerce/pull/966