search

Shopify / buy-button-js

BuyButton.js is a highly customizable UI library for adding ecommerce functionality to any website.
http://shopify.github.io/buy-button-js/
MIT License
242 stars 114 forks source link

Remove unnecessary call to the js-buy-sdk in cart's init #499

Open spencercanner opened 6 years ago

spencercanner commented 6 years ago

In #498 a bug was fixed to prevent always fetching the moneyFormat from the js-buy-sdk in the cart component. It should now be taking the moneyFormat passed in via createComponent, if available, and resorting to the sdk's value otherwise. However, after looking into the globalConfig's moneyFormat it should always fall back to a default value if nothing is passed in via createComponent (https://github.com/Shopify/buy-button-js/blob/master/src/component.js#L10-L31).

Thus, it seems like the entire call to the sdk could be removed from the cart component's init(...) (https://github.com/Shopify/buy-button-js/blob/master/src/components/cart.js#L185-L189)

spencercanner commented 6 years ago

Upon further investigation, it seems like it might be beneficial to use the sdk for the fallback value, if nothing is specified in the config. This would allow merchants to update the admin money format setting and have this change propagate to all their buy buttons. However, currently the product component is falling back to a hardcoded value specified in this repo (https://github.com/Shopify/buy-button-js/blob/master/src/defaults/money-format.js). Ideally, this component should also be falling back to the sdk value. Since multiple components will be falling back to the same globalConfig value, it would be worth abstracting the call to the sdk up to a higher level, either in component.js or ui.js. This would aid in ensuring a single call to the sdk and prevent lower level components from mutating the globalConfig object.

melissaluu commented 5 years ago

Update: Since multi-currency will be changing how prices will be displayed in Buy Buttons, we'll hold off on this ticket until we decide on how multi-currency will be added to StorefrontAPI.